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