Sounds good to me. Let's give it another day and see how it goes.
On Fri, Aug 11, 2017 at 6:48 PM, Richard Smith <rich...@metafoo.co.uk> wrote: > 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
