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@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@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits