r310776 - PR34163: Don't cache an incorrect key function for a class if queried between
Hans Wennborg via cfe-commits
cfe-commits at lists.llvm.org
Mon Aug 14 10:07:38 PDT 2017
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 <richard at 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 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
>
>
More information about the cfe-commits
mailing list