r310776 - PR34163: Don't cache an incorrect key function for a class if queried between
Richard Smith via cfe-commits
cfe-commits at lists.llvm.org
Fri Aug 11 18:48:17 PDT 2017
Hi Hans,
I'd like to get this into Clang 5, but it's not entirely risk-free. Perhaps
we could leave it in the tree for a little while and then merge if it seems
OK?
On 11 August 2017 at 18:46, Richard Smith via cfe-commits <
cfe-commits at lists.llvm.org> wrote:
> Author: rsmith
> Date: Fri Aug 11 18:46:03 2017
> New Revision: 310776
>
> URL: http://llvm.org/viewvc/llvm-project?rev=310776&view=rev
> Log:
> PR34163: Don't cache an incorrect key function for a class if queried
> between
> the class becoming complete and its inline methods being parsed.
>
> This replaces the hack of using the "late parsed template" flag to track
> member
> functions with bodies we've not parsed yet; instead we now use the "will
> have
> body" flag, which carries the desired implication that the function
> declaration
> *is* a definition, and that we've just not parsed its body yet.
>
> Added:
> cfe/trunk/test/CodeGenCXX/pr34163.cpp
> Modified:
> cfe/trunk/include/clang/AST/Decl.h
> cfe/trunk/lib/AST/DeclCXX.cpp
> cfe/trunk/lib/Parse/ParseCXXInlineMethods.cpp
> cfe/trunk/lib/Sema/SemaDecl.cpp
> cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
> cfe/trunk/test/SemaCUDA/function-overload.cu
> cfe/trunk/test/SemaCUDA/no-destructor-overload.cu
>
> Modified: cfe/trunk/include/clang/AST/Decl.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/
> clang/AST/Decl.h?rev=310776&r1=310775&r2=310776&view=diff
> ============================================================
> ==================
> --- cfe/trunk/include/clang/AST/Decl.h (original)
> +++ cfe/trunk/include/clang/AST/Decl.h Fri Aug 11 18:46:03 2017
> @@ -1666,8 +1666,7 @@ private:
> unsigned HasSkippedBody : 1;
>
> /// Indicates if the function declaration will have a body, once we're
> done
> - /// parsing it. (We don't set it to false when we're done parsing, in
> the
> - /// hopes this is simpler.)
> + /// parsing it.
> unsigned WillHaveBody : 1;
>
> /// \brief End part of this FunctionDecl's source range.
>
> Modified: cfe/trunk/lib/AST/DeclCXX.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/
> DeclCXX.cpp?rev=310776&r1=310775&r2=310776&view=diff
> ============================================================
> ==================
> --- cfe/trunk/lib/AST/DeclCXX.cpp (original)
> +++ cfe/trunk/lib/AST/DeclCXX.cpp Fri Aug 11 18:46:03 2017
> @@ -1837,9 +1837,10 @@ bool CXXMethodDecl::hasInlineBody() cons
> const FunctionDecl *CheckFn = getTemplateInstantiationPattern();
> if (!CheckFn)
> CheckFn = this;
> -
> +
> const FunctionDecl *fn;
> - return CheckFn->hasBody(fn) && !fn->isOutOfLine();
> + return CheckFn->isDefined(fn) && !fn->isOutOfLine() &&
> + (fn->doesThisDeclarationHaveABody() || fn->willHaveBody());
> }
>
> bool CXXMethodDecl::isLambdaStaticInvoker() const {
>
> Modified: cfe/trunk/lib/Parse/ParseCXXInlineMethods.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/
> ParseCXXInlineMethods.cpp?rev=310776&r1=310775&r2=310776&view=diff
> ============================================================
> ==================
> --- cfe/trunk/lib/Parse/ParseCXXInlineMethods.cpp (original)
> +++ cfe/trunk/lib/Parse/ParseCXXInlineMethods.cpp Fri Aug 11 18:46:03 2017
> @@ -166,20 +166,11 @@ NamedDecl *Parser::ParseCXXInlineMethodD
> }
>
> if (FnD) {
> - // If this is a friend function, mark that it's late-parsed so that
> - // it's still known to be a definition even before we attach the
> - // parsed body. Sema needs to treat friend function definitions
> - // differently during template instantiation, and it's possible for
> - // the containing class to be instantiated before all its member
> - // function definitions are parsed.
> - //
> - // If you remove this, you can remove the code that clears the flag
> - // after parsing the member.
> - if (D.getDeclSpec().isFriendSpecified()) {
> - FunctionDecl *FD = FnD->getAsFunction();
> - Actions.CheckForFunctionRedefinition(FD);
> - FD->setLateTemplateParsed(true);
> - }
> + FunctionDecl *FD = FnD->getAsFunction();
> + // Track that this function will eventually have a body; Sema needs
> + // to know this.
> + Actions.CheckForFunctionRedefinition(FD);
> + FD->setWillHaveBody(true);
> } else {
> // If semantic analysis could not build a function declaration,
> // just throw away the late-parsed declaration.
> @@ -559,10 +550,6 @@ void Parser::ParseLexedMethodDef(LexedMe
>
> ParseFunctionStatementBody(LM.D, FnScope);
>
> - // Clear the late-template-parsed bit if we set it before.
> - if (LM.D)
> - LM.D->getAsFunction()->setLateTemplateParsed(false);
> -
> while (Tok.isNot(tok::eof))
> ConsumeAnyToken();
>
>
> Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/
> SemaDecl.cpp?rev=310776&r1=310775&r2=310776&view=diff
> ============================================================
> ==================
> --- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaDecl.cpp Fri Aug 11 18:46:03 2017
> @@ -12090,8 +12090,9 @@ Decl *Sema::ActOnStartOfFunctionDef(Scop
> FD->setInvalidDecl();
> }
>
> - // See if this is a redefinition.
> - if (!FD->isLateTemplateParsed()) {
> + // See if this is a redefinition. If 'will have body' is already set,
> then
> + // these checks were already performed when it was set.
> + if (!FD->willHaveBody() && !FD->isLateTemplateParsed()) {
> CheckForFunctionRedefinition(FD, nullptr, SkipBody);
>
> // If we're skipping the body, we're done. Don't enter the scope.
>
> Modified: cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/
> SemaTemplateInstantiateDecl.cpp?rev=310776&r1=310775&r2=310776&view=diff
> ============================================================
> ==================
> --- cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp Fri Aug 11
> 18:46:03 2017
> @@ -3771,6 +3771,8 @@ void Sema::InstantiateFunctionDefinition
> if (PatternDef) {
> Pattern = PatternDef->getBody(PatternDef);
> PatternDecl = PatternDef;
> + if (PatternDef->willHaveBody())
> + PatternDef = nullptr;
> }
>
> // FIXME: We need to track the instantiation stack in order to know
> which
>
> Added: cfe/trunk/test/CodeGenCXX/pr34163.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/
> CodeGenCXX/pr34163.cpp?rev=310776&view=auto
> ============================================================
> ==================
> --- cfe/trunk/test/CodeGenCXX/pr34163.cpp (added)
> +++ cfe/trunk/test/CodeGenCXX/pr34163.cpp Fri Aug 11 18:46:03 2017
> @@ -0,0 +1,13 @@
> +// RUN: %clang_cc1 -emit-llvm -debug-info-kind=standalone -triple
> x86_64-linux-gnu -o - -x c++ %s | FileCheck %s
> +
> +void f(struct X *) {}
> +
> +// CHECK: @_ZTV1X =
> +struct X {
> + void a() { delete this; }
> + virtual ~X() {}
> + virtual void key_function();
> +};
> +
> +// CHECK: define {{.*}} @_ZN1X12key_functionEv(
> +void X::key_function() {}
>
> Modified: cfe/trunk/test/SemaCUDA/function-overload.cu
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/
> SemaCUDA/function-overload.cu?rev=310776&r1=310775&r2=310776&view=diff
> ============================================================
> ==================
> --- cfe/trunk/test/SemaCUDA/function-overload.cu (original)
> +++ cfe/trunk/test/SemaCUDA/function-overload.cu Fri Aug 11 18:46:03 2017
> @@ -222,7 +222,7 @@ GlobalFnPtr fp_g = g;
> // Test overloading of destructors
> // Can't mix H and unattributed destructors
> struct d_h {
> - ~d_h() {} // expected-note {{previous declaration is here}}
> + ~d_h() {} // expected-note {{previous definition is here}}
> __host__ ~d_h() {} // expected-error {{destructor cannot be redeclared}}
> };
>
>
> Modified: cfe/trunk/test/SemaCUDA/no-destructor-overload.cu
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/
> SemaCUDA/no-destructor-overload.cu?rev=310776&r1=
> 310775&r2=310776&view=diff
> ============================================================
> ==================
> --- cfe/trunk/test/SemaCUDA/no-destructor-overload.cu (original)
> +++ cfe/trunk/test/SemaCUDA/no-destructor-overload.cu Fri Aug 11 18:46:03
> 2017
> @@ -7,27 +7,27 @@
> // giant change to clang, and the use cases seem quite limited.
>
> struct A {
> - ~A() {} // expected-note {{previous declaration is here}}
> + ~A() {} // expected-note {{previous definition is here}}
> __device__ ~A() {} // expected-error {{destructor cannot be redeclared}}
> };
>
> struct B {
> - __host__ ~B() {} // expected-note {{previous declaration is here}}
> + __host__ ~B() {} // expected-note {{previous definition is here}}
> __host__ __device__ ~B() {} // expected-error {{destructor cannot be
> redeclared}}
> };
>
> struct C {
> - __host__ __device__ ~C() {} // expected-note {{previous declaration is
> here}}
> + __host__ __device__ ~C() {} // expected-note {{previous definition is
> here}}
> __host__ ~C() {} // expected-error {{destructor cannot be redeclared}}
> };
>
> struct D {
> - __device__ ~D() {} // expected-note {{previous declaration is here}}
> + __device__ ~D() {} // expected-note {{previous definition is here}}
> __host__ __device__ ~D() {} // expected-error {{destructor cannot be
> redeclared}}
> };
>
> struct E {
> - __host__ __device__ ~E() {} // expected-note {{previous declaration is
> here}}
> + __host__ __device__ ~E() {} // expected-note {{previous definition is
> here}}
> __device__ ~E() {} // expected-error {{destructor cannot be redeclared}}
> };
>
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170811/d9cbf12b/attachment.html>
More information about the cfe-commits
mailing list