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
Thu Aug 17 10:27:33 PDT 2017
Merged in r311105.
On Mon, Aug 14, 2017 at 10:07 AM, Hans Wennborg <hans at chromium.org> wrote:
> 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