Tie DefineVTablesUsed() in with recursive function instantiation so that we emit

a useful template instantiation stack. Fixes PR8640.

This also causes a slight change to where the "instantianted from" note shows up
in truly esoteric cases (see the change to test/SemaCXX/destructor.cpp), but
that isn't directly the fault of this patch.

llvm-svn: 120135

clang/include/clang/Sema/Sema.h

@@ -2557,8 +2557,11 @@ public:
   /// \brief The list of classes whose vtables have been used within
   /// this translation unit, and the source locations at which the
   /// first use occurred.
-  llvm::SmallVector<std::pair<CXXRecordDecl *, SourceLocation>, 16> 
-    VTableUses;
+  typedef std::pair<CXXRecordDecl*, SourceLocation> VTableUse;
+
+  /// \brief The list of vtables that are required but have not yet been
+  /// materialized.
+  llvm::SmallVector<VTableUse, 16> VTableUses;
 
   /// \brief The set of classes whose vtables have been used within
   /// this translation unit, and a bit that will be true if the vtable is

clang/lib/Sema/Sema.cpp

@@ -284,26 +284,24 @@ static bool ShouldRemoveFromUnused(Sema *SemaRef, const DeclaratorDecl *D) {
 void Sema::ActOnEndOfTranslationUnit() {
   // At PCH writing, implicit instantiations and VTable handling info are
   // stored and performed when the PCH is included.
-  if (CompleteTranslationUnit)
-    while (1) {
-      // C++: Perform implicit template instantiations.
-      //
-      // FIXME: When we perform these implicit instantiations, we do not
-      // carefully keep track of the point of instantiation (C++ [temp.point]).
-      // This means that name lookup that occurs within the template
-      // instantiation will always happen at the end of the translation unit,
-      // so it will find some names that should not be found. Although this is
-      // common behavior for C++ compilers, it is technically wrong. In the
-      // future, we either need to be able to filter the results of name lookup
-      // or we need to perform template instantiations earlier.
-      PerformPendingInstantiations();
+  if (CompleteTranslationUnit) {
+    // If DefinedUsedVTables ends up marking any virtual member functions it
+    // might lead to more pending template instantiations, which we then need
+    // to instantiate.
+    DefineUsedVTables();
 
-      /// If DefinedUsedVTables ends up marking any virtual member
-      /// functions it might lead to more pending template
-      /// instantiations, which is why we need to loop here.
-      if (!DefineUsedVTables())
-        break;
-    }
+    // C++: Perform implicit template instantiations.
+    //
+    // FIXME: When we perform these implicit instantiations, we do not
+    // carefully keep track of the point of instantiation (C++ [temp.point]).
+    // This means that name lookup that occurs within the template
+    // instantiation will always happen at the end of the translation unit,
+    // so it will find some names that should not be found. Although this is
+    // common behavior for C++ compilers, it is technically wrong. In the
+    // future, we either need to be able to filter the results of name lookup
+    // or we need to perform template instantiations earlier.
+    PerformPendingInstantiations();
+  }
   
   // Remove file scoped decls that turned out to be used.
   UnusedFileScopedDecls.erase(std::remove_if(UnusedFileScopedDecls.begin(),

clang/lib/Sema/SemaTemplateInstantiateDecl.cpp

@@ -2108,9 +2108,12 @@ void Sema::InstantiateFunctionDefinition(SourceLocation PointOfInstantiation,
   // If we're performing recursive template instantiation, create our own
   // queue of pending implicit instantiations that we will instantiate later,
   // while we're still within our own instantiation context.
+  llvm::SmallVector<VTableUse, 16> SavedVTableUses;
   std::deque<PendingImplicitInstantiation> SavedPendingInstantiations;
-  if (Recursive)
+  if (Recursive) {
+    VTableUses.swap(SavedVTableUses);
     PendingInstantiations.swap(SavedPendingInstantiations);
+  }
 
   EnterExpressionEvaluationContext EvalContext(*this, 
                                                Sema::PotentiallyEvaluated);
@@ -2173,10 +2176,16 @@ void Sema::InstantiateFunctionDefinition(SourceLocation PointOfInstantiation,
   Scope.Exit();
 
   if (Recursive) {
+    // Define any pending vtables.
+    DefineUsedVTables();
+
     // Instantiate any pending implicit instantiations found during the
     // instantiation of this template.
     PerformPendingInstantiations();
 
+    // Restore the set of pending vtables.
+    VTableUses.swap(SavedVTableUses);
+
     // Restore the set of pending implicit instantiations.
     PendingInstantiations.swap(SavedPendingInstantiations);
   }

clang/test/SemaCXX/destructor.cpp

@@ -100,11 +100,11 @@ namespace test6 {
       T::deleteIt(p); // expected-error {{type 'int' cannot be used prior to '::'}}
     }
 
-    virtual ~A() {} // expected-note {{in instantiation of member function 'test6::A<int>::operator delete' requested here}}
+    virtual ~A() {}
   };
 
-  class B : A<int> { B(); };
-  B::B() {} // expected-note {{in instantiation of member function 'test6::A<int>::~A' requested here}}
+  class B : A<int> { B(); }; // expected-note {{in instantiation of member function 'test6::A<int>::operator delete' requested here}}
+  B::B() {}
 }
 
 // Make sure classes are marked invalid when they have invalid

clang/test/SemaCXX/vtable-instantiation.cc

@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+// PR8640
+
+template<class T1> struct C1 {
+  virtual void c1() {
+    T1 t1 = 3;  // expected-error {{cannot initialize a variable}}
+  }
+};
+
+template<class T2> struct C2 {
+  void c2() {
+    new C1<T2>();  // expected-note {{in instantiation of member function}}
+  }
+};
+
+void f() {
+  C2<int*> c2;
+  c2.c2();  // expected-note {{in instantiation of member function}}
+}
+