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

8 files changed, 33 insertions, 30 deletions

diff --git a/include/clang/AST/Decl.h b/include/clang/AST/Decl.h
index 08b34a75aa..54e6ebcd8a 100644
--- a/include/clang/AST/Decl.h
+++ b/include/clang/AST/Decl.h
@@ -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.
diff --git a/lib/AST/DeclCXX.cpp b/lib/AST/DeclCXX.cpp
index 5cab488822..1caceab85e 100644
--- a/lib/AST/DeclCXX.cpp
+++ b/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 {
diff --git a/lib/Parse/ParseCXXInlineMethods.cpp b/lib/Parse/ParseCXXInlineMethods.cpp
index c9a895d7d5..2b3d4ba85b 100644
--- a/lib/Parse/ParseCXXInlineMethods.cpp
+++ b/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();
 
diff --git a/lib/Sema/SemaDecl.cpp b/lib/Sema/SemaDecl.cpp
index 86c367097f..0879e78a9a 100644
--- a/lib/Sema/SemaDecl.cpp
+++ b/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.
diff --git a/lib/Sema/SemaTemplateInstantiateDecl.cpp b/lib/Sema/SemaTemplateInstantiateDecl.cpp
index abe912fb54..6fee23aa8b 100644
--- a/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/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
diff --git a/test/CodeGenCXX/pr34163.cpp b/test/CodeGenCXX/pr34163.cpp
new file mode 100644
index 0000000000..a200a0f509
--- /dev/null
+++ b/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() {}
diff --git a/test/SemaCUDA/function-overload.cu b/test/SemaCUDA/function-overload.cu
index 3d4c29c42c..adf488b5ee 100644
--- a/test/SemaCUDA/function-overload.cu
+++ b/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}}
 };
 
diff --git a/test/SemaCUDA/no-destructor-overload.cu b/test/SemaCUDA/no-destructor-overload.cu
index aa6971ee8c..32dbb8db76 100644
--- a/test/SemaCUDA/no-destructor-overload.cu
+++ b/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}}
 };