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.


git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@310776 91177308-0d34-0410-b5e6-96231b3b80d8

Author: zygoloid

include/clang/AST/Decl.h

@@ -1666,8 +1666,7 @@ class FunctionDecl : public DeclaratorDecl, public DeclContext,
   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.

lib/AST/DeclCXX.cpp

@@ -1837,9 +1837,10 @@ bool CXXMethodDecl::hasInlineBody() const {
   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 {

lib/Parse/ParseCXXInlineMethods.cpp

@@ -166,20 +166,11 @@ NamedDecl *Parser::ParseCXXInlineMethodDef(AccessSpecifier AS,
   }
 
   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(LexedMethod &LM) {
 
   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();
 

lib/Sema/SemaDecl.cpp

@@ -12090,8 +12090,9 @@ Decl *Sema::ActOnStartOfFunctionDef(Scope *FnBodyScope, Decl *D,
     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.

lib/Sema/SemaTemplateInstantiateDecl.cpp

@@ -3771,6 +3771,8 @@ void Sema::InstantiateFunctionDefinition(SourceLocation PointOfInstantiation,
   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

test/CodeGenCXX/pr34163.cpp

@@ -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() {}

test/SemaCUDA/function-overload.cu

@@ -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}}
 };
 

test/SemaCUDA/no-destructor-overload.cu

@@ -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}}
 };