Whenever we instantiate a static data member, make sure to define any new

vtables! Fixes PR10020

This also allows us to revert the part of r130023 which added a big loop around
the template instantiation.

llvm-svn: 132331

clang/include/clang/Sema/Sema.h

@@ -4658,7 +4658,7 @@ public:
   /// types, static variables, enumerators, etc.
   std::deque<PendingImplicitInstantiation> PendingLocalImplicitInstantiations;
 
-  bool PerformPendingInstantiations(bool LocalOnly = false);
+  void PerformPendingInstantiations(bool LocalOnly = false);
 
   TypeSourceInfo *SubstType(TypeSourceInfo *T,
                             const MultiLevelTemplateArgumentList &TemplateArgs,

clang/lib/Sema/Sema.cpp

@@ -378,30 +378,22 @@ void Sema::ActOnEndOfTranslationUnit() {
       }
     }
 
-    bool SomethingChanged;
-    do {
-      SomethingChanged = false;
+    // 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 we then need
-      // to instantiate.
-      if (DefineUsedVTables())
-        SomethingChanged = true;
-
-      // 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.
-      if (PerformPendingInstantiations())
-        SomethingChanged = true;
-      
-    } while (SomethingChanged);
+    // 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.

clang/lib/Sema/SemaTemplateInstantiateDecl.cpp

@@ -2481,9 +2481,13 @@ void Sema::InstantiateFunctionDefinition(SourceLocation PointOfInstantiation,
     PerformPendingInstantiations();
 
     // Restore the set of pending vtables.
+    assert(VTableUses.empty() &&
+           "VTableUses should be empty before it is discarded.");
     VTableUses.swap(SavedVTableUses);
 
     // Restore the set of pending implicit instantiations.
+    assert(PendingInstantiations.empty() &&
+           "PendingInstantiations should be empty before it is discarded.");
     PendingInstantiations.swap(SavedPendingInstantiations);
   }
 }
@@ -2557,9 +2561,12 @@ void Sema::InstantiateStaticDataMemberDefinition(
   // 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);
+  }
 
   // Enter the scope of this instantiation. We don't use
   // PushDeclContext because we don't have a scope.
@@ -2581,11 +2588,23 @@ void Sema::InstantiateStaticDataMemberDefinition(
   }
 
   if (Recursive) {
+    // Define any newly required vtables.
+    DefineUsedVTables();
+
     // Instantiate any pending implicit instantiations found during the
     // instantiation of this template.
     PerformPendingInstantiations();
 
+    // Restore the set of pending vtables.
+    assert(VTableUses.empty() &&
+           "VTableUses should be empty before it is discarded, "
+           "while instantiating static data member.");
+    VTableUses.swap(SavedVTableUses);
+
     // Restore the set of pending implicit instantiations.
+    assert(PendingInstantiations.empty() &&
+           "PendingInstantiations should be empty before it is discarded, "
+           "while instantiating static data member.");
     PendingInstantiations.swap(SavedPendingInstantiations);
   }
 }
@@ -3181,10 +3200,7 @@ NamedDecl *Sema::FindInstantiatedDecl(SourceLocation Loc, NamedDecl *D,
 
 /// \brief Performs template instantiation for all implicit template
 /// instantiations we have seen until this point.
-///
-/// \returns true if anything was instantiated.
-bool Sema::PerformPendingInstantiations(bool LocalOnly) {
-  bool InstantiatedAnything = false;
+void Sema::PerformPendingInstantiations(bool LocalOnly) {
   while (!PendingLocalImplicitInstantiations.empty() ||
          (!LocalOnly && !PendingInstantiations.empty())) {
     PendingImplicitInstantiation Inst;
@@ -3205,7 +3221,6 @@ bool Sema::PerformPendingInstantiations(bool LocalOnly) {
                                 TSK_ExplicitInstantiationDefinition;
       InstantiateFunctionDefinition(/*FIXME:*/Inst.second, Function, true,
                                     DefinitionRequired);
-      InstantiatedAnything = true;
       continue;
     }
 
@@ -3242,10 +3257,7 @@ bool Sema::PerformPendingInstantiations(bool LocalOnly) {
                               TSK_ExplicitInstantiationDefinition;
     InstantiateStaticDataMemberDefinition(/*FIXME:*/Inst.second, Var, true,
                                           DefinitionRequired);
-    InstantiatedAnything = true;
   }
-  
-  return InstantiatedAnything;
 }
 
 void Sema::PerformDependentDiagnostics(const DeclContext *Pattern,

clang/test/SemaCXX/vtable-instantiation.cc

@@ -1,21 +1,22 @@
 // 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}}
+namespace 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}}
   }
-};
-
-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}}
 }
 
 namespace PR9325 {
@@ -42,5 +43,26 @@ namespace PR9325 {
   {
     Target<int*>* traits = &Provider<int*>::Instance;
   }
-
+}
+
+namespace PR10020 {
+  struct MG {
+    virtual void Accept(int) = 0;
+  };
+
+  template <typename Type>
+  struct GMG : MG {
+    void Accept(int i) {
+      static_cast<Type *>(0)->Accept(i); // expected-error{{member reference base}}
+    }
+    static GMG* Method() { return &singleton; } // expected-note{{in instantiation of}}
+    static GMG singleton;
+  };
+
+  template <typename Type>
+  GMG<Type> GMG<Type>::singleton;
+
+  void test(void) {
+    GMG<int>::Method(); // expected-note{{in instantiation of}}
+  }
 }