Merge pending instantiations instead of overwriting existing ones.

Check whether a pending instantiation needs to be instantiated (or whether an instantiation already exists).
Verify the size of the PendingInstantiations record (was only checking size of existing PendingInstantiations).

Migrate Obj-C++ part of redecl-merge into separate test, now that this is growing.
templates.mm: test that CodeGen has seen exactly one definition of template instantiations.
redecl-merge.m: use "@" specifier for expected-diagnostics.

llvm-svn: 164993

clang/include/clang/Serialization/ASTReader.h

@@ -687,7 +687,7 @@ private:
   /// Objective-C protocols.
   std::deque<Decl *> InterestingDecls;
 
-  /// \brief The set of redeclarable declaraations that have been deserialized
+  /// \brief The set of redeclarable declarations that have been deserialized
   /// since the last time the declaration chains were linked.
   llvm::SmallPtrSet<Decl *, 16> RedeclsDeserialized;
   
@@ -854,6 +854,10 @@ private:
 
   void finishPendingActions();
 
+  /// \brief Whether D needs to be instantiated, i.e. whether an instantiation
+  /// for D does not exist yet.
+  bool needPendingInstantiation(ValueDecl* D) const;
+
   /// \brief Produce an error diagnostic and return true.
   ///
   /// This routine should only be used for fatal errors that have to

clang/lib/Serialization/ASTReader.cpp

@@ -2223,13 +2223,15 @@ ASTReader::ReadASTBlock(ModuleFile &F) {
 
     case PENDING_IMPLICIT_INSTANTIATIONS:
       if (PendingInstantiations.size() % 2 != 0) {
+        Error("Invalid existing PendingInstantiations");
+        return Failure;
+      }
+
+      if (Record.size() % 2 != 0) {
         Error("Invalid PENDING_IMPLICIT_INSTANTIATIONS block");
         return Failure;
       }
-        
-      // Later lists of pending instantiations overwrite earlier ones.
-      // FIXME: This is most certainly wrong for modules.
-      PendingInstantiations.clear();
+
       for (unsigned I = 0, N = Record.size(); I != N; /* in loop */) {
         PendingInstantiations.push_back(getGlobalDeclID(F, Record[I++]));
         PendingInstantiations.push_back(
@@ -5592,7 +5594,11 @@ void ASTReader::ReadPendingInstantiations(
     ValueDecl *D = cast<ValueDecl>(GetDecl(PendingInstantiations[Idx++]));
     SourceLocation Loc
       = SourceLocation::getFromRawEncoding(PendingInstantiations[Idx++]);
-    Pending.push_back(std::make_pair(D, Loc));
+
+    // For modules, find out whether an instantiation already exists
+    if (!getContext().getLangOpts().Modules
+        || needPendingInstantiation(D))
+      Pending.push_back(std::make_pair(D, Loc));
   }  
   PendingInstantiations.clear();
 }

clang/lib/Serialization/ASTReaderDecl.cpp

@@ -2156,7 +2156,7 @@ Decl *ASTReader::ReadDeclRecord(DeclID ID) {
   // loading, and some declarations may still be initializing.
   if (isConsumerInterestedIn(D))
     InterestingDecls.push_back(D);
-  
+
   return D;
 }
 
@@ -2504,3 +2504,60 @@ void ASTDeclReader::UpdateDecl(Decl *D, ModuleFile &ModuleFile,
     }
   }
 }
+
+/// \brief Return a template specialization of ND (should be a TemplateDecl)
+///  that matches FD or TD.
+static NamedDecl* findMatchingSpecialization(FunctionDecl* FD,
+                                             ClassTemplateSpecializationDecl*TD,
+                                             NamedDecl* ND) {
+  TemplateDecl* Templt = dyn_cast<TemplateDecl>(ND);
+  if (!Templt) return 0;
+  if (FD) {
+    FunctionTemplateDecl* FTD = dyn_cast<FunctionTemplateDecl>(Templt);
+    if (!FTD) return 0;
+    const TemplateArgumentList* TmpltArgs = FD->getTemplateSpecializationArgs();
+    assert(TmpltArgs || "Template without arguments");
+    void* InsertionPoint;
+    return FTD->findSpecialization(TmpltArgs->data(), TmpltArgs->size(),
+                                   InsertionPoint);
+  } else {
+    ClassTemplateDecl* CTD = dyn_cast<ClassTemplateDecl>(Templt);
+    if (!CTD) return 0;
+    const TemplateArgumentList& TmpltArgs = TD->getTemplateArgs();
+    void* InsertionPoint;
+    return CTD->findSpecialization(TmpltArgs.data(), TmpltArgs.size(),
+                                   InsertionPoint);
+  }
+  return 0;
+}
+
+/// \brief Find out whether an instantiation (outside the module) already exists
+bool ASTReader::needPendingInstantiation(ValueDecl* D) const {
+  DeclContext *DC = D->getDeclContext()->getRedeclContext();
+  DeclarationName Name = D->getDeclName();
+  assert(Name && "unnamed template");
+
+  FunctionDecl* FD = dyn_cast<FunctionDecl>(D);
+  ClassTemplateSpecializationDecl* CD
+    = FD ? 0 : dyn_cast<ClassTemplateSpecializationDecl>(D);
+
+  NamedDecl* FoundSpecialization = 0;
+  if (DC->isTranslationUnit() && SemaObj) {
+    IdentifierResolver &IdResolver = SemaObj->IdResolver;
+    for (IdentifierResolver::iterator I = IdResolver.begin(Name), 
+           IEnd = IdResolver.end();
+         I != IEnd && !FoundSpecialization; ++I)
+      FoundSpecialization = findMatchingSpecialization(FD, CD, *I);
+  } else {
+    // templates are redeclarables, i.e. they must have been merged into
+    // the primary context. Use localUncachedLookup to not pick up template
+    // decls from modules again.
+    llvm::SmallVector<NamedDecl*, 6> Results;
+    DC->getPrimaryContext()->localUncachedLookup(Name, Results);
+    for (llvm::SmallVector<NamedDecl *, 6>::const_iterator
+           I = Results.begin(), E = Results.end();
+         I != E && FoundSpecialization; ++I)
+      FoundSpecialization = findMatchingSpecialization(FD, CD, *I);
+  }
+  return FoundSpecialization && isSameEntity(FoundSpecialization, D);
+}

clang/test/Modules/Inputs/module.map

@@ -78,6 +78,18 @@ module namespaces_right {
   header "namespaces-right.h"
   export *
 }
+module templates_top { 
+  header "templates-top.h"
+  export *
+}
+module templates_left { 
+  header "templates-left.h"
+  export *
+}
+module templates_right { 
+  header "templates-right.h"
+  export *
+}
 module MethodPoolA {
   header "MethodPoolA.h"
 }

clang/test/Modules/Inputs/redecl-merge-bottom.h

@@ -18,11 +18,3 @@ struct S3;
 
 void refers_to_C4(C4*);
 
-#ifdef __cplusplus
-template<typename T> class Vector;
-
-template<typename T> class Vector;
-
-template<typename T> class Vector;
-#endif
-

clang/test/Modules/Inputs/redecl-merge-left.h

@@ -78,27 +78,6 @@ extern float var2;
 
 extern double var3;
 
-#ifdef __cplusplus
-template<typename T> class Vector;
-
-template<typename T> class Vector;
-
-template<typename T> class List;
-template<> class List<bool> {
-public:
-  void push_back(int);
-};
-namespace N {
-  template<typename T> class Set;
-}
-namespace N {
-  template<typename T> class Set {
-  public:
-    void insert(T);
-  };
-}
-#endif
-
 // Make sure this doesn't introduce an ambiguity-creating 'id' at the
 // top level.
 typedef void funcptr_with_id(int id);

clang/test/Modules/Inputs/redecl-merge-right.h

@@ -78,26 +78,6 @@ extern int var2;
 
 static double var3;
 
-#ifdef __cplusplus
-template<typename T> class Vector { 
-public:
-  void push_back(const T&);
-};
-
-template<typename T> class List;
-template<> class List<bool> {
-public:
-  void push_back(int);
-};
-
-namespace N {
-  template<typename T> class Set {
-  public:
-    void insert(T);
-  };
-}
-#endif
-
 int ONE;
 @__experimental_modules_import redecl_merge_top.Explicit;
 const int one = ONE;

clang/test/Modules/Inputs/redecl-merge-top.h

@@ -14,12 +14,3 @@
 struct S1;
 struct S2;
 struct S2;
-
-#ifdef __cplusplus
-template<typename T> class Vector;
-
-template<typename T> class List {
-public:
-  void push_back(T);
-};
-#endif

clang/test/Modules/Inputs/templates-left.h

@@ -0,0 +1,27 @@
+@__experimental_modules_import templates_top;
+
+template<typename T> class Vector;
+
+template<typename T> class Vector;
+
+template<typename T> class List;
+template<> class List<bool> {
+public:
+  void push_back(int);
+};
+namespace N {
+  template<typename T> class Set;
+}
+namespace N {
+  template<typename T> class Set {
+  public:
+    void insert(T);
+  };
+}
+
+template <typename T>
+void pendingInstantiation(T) {}
+void triggerPendingInstantiation() {
+  pendingInstantiation(12);
+  pendingInstantiation(42.);
+}

clang/test/Modules/Inputs/templates-right.h

@@ -0,0 +1,25 @@
+@__experimental_modules_import templates_top;
+
+template<typename T> class Vector { 
+public:
+  void push_back(const T&);
+};
+
+template<typename T> class List;
+template<> class List<bool> {
+public:
+  void push_back(int);
+};
+
+namespace N {
+  template<typename T> class Set {
+  public:
+    void insert(T);
+  };
+}
+
+template <typename T>
+void pendingInstantiation(T) {}
+void triggerPendingInstantiationToo() {
+  pendingInstantiation(12);
+}

clang/test/Modules/Inputs/templates-top.h

@@ -0,0 +1,6 @@
+template<typename T> class Vector;
+
+template<typename T> class List {
+public:
+  void push_back(T);
+};

clang/test/Modules/redecl-merge.m

@@ -1,6 +1,5 @@
 // RUN: rm -rf %t
 // RUN: %clang_cc1 -fmodules -fmodule-cache-path %t -I %S/Inputs %s -verify -Wno-objc-root-class
-// RUN: %clang_cc1 -x objective-c++ -fmodules -fmodule-cache-path %t -I %S/Inputs %s -verify -Wno-objc-root-class
 @class C2;
 @class C3;
 @class C3;
@@ -57,26 +56,26 @@ void testTagMerge() {
 
 void testTypedefMerge(int i, double d) {
   T1 *ip = &i;
-  // in other file: expected-note{{candidate found by name lookup is 'T2'}}
   // FIXME: Typedefs aren't actually merged in the sense of other merges, because
   // we should only merge them when the types are identical.
-  // in other file: expected-note{{candidate found by name lookup is 'T2'}}
-  // in other file: expected-note{{candidate function}}
+  // in other file: expected-note@60{{candidate found by name lookup is 'T2'}}
+  // in other file: expected-note@63{{candidate found by name lookup is 'T2'}}
   T2 *dp = &d; // expected-error{{reference to 'T2' is ambiguous}}
 }
 
 void testFuncMerge(int i) {
   func0(i);
-  // in other file: expected-note{{candidate function}}
   func1(i);
+  // in other file: expected-note@64{{candidate function}}
+  // in other file: expected-note@70{{candidate function}}
   func2(i); // expected-error{{call to 'func2' is ambiguous}}
 }
 
 void testVarMerge(int i) {
   var1 = i;
-  // in other files: expected-note 2{{candidate found by name lookup is 'var2'}}
+  // in other files: expected-note@77 2{{candidate found by name lookup is 'var2'}}
   var2 = i; // expected-error{{reference to 'var2' is ambiguous}}
-  // in other files: expected-note 2{{candidate found by name lookup is 'var3'}}
+  // in other files: expected-note@79 2{{candidate found by name lookup is 'var3'}}
   var3 = i; // expected-error{{reference to 'var3' is ambiguous}}
 }
 
@@ -146,19 +145,6 @@ void g(A *a) {
 id<P4> p4;
 id<P3> p3;
 
-#ifdef __cplusplus
-void testVector() {
-  Vector<int> vec_int;
-  vec_int.push_back(0);
-
-  List<bool> list_bool;
-  list_bool.push_back(false);
-
-  N::Set<char> set_char;
-  set_char.insert('A');
-}
-#endif
-
 // Make sure we don't get conflicts with 'id'.
 funcptr_with_id fid;
 id id_global;

clang/test/Modules/templates.mm

@@ -0,0 +1,28 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -x objective-c++ -fmodules -fmodule-cache-path %t -I %S/Inputs -verify %s -Wno-objc-root-class
+// RUN: %clang_cc1 -x objective-c++ -fmodules -fmodule-cache-path %t -I %S/Inputs -emit-llvm %s -o - -Wno-objc-root-class | grep pendingInstantiation | FileCheck %s
+
+@__experimental_modules_import templates_left;
+@__experimental_modules_import templates_right;
+
+
+void testTemplateClasses() {
+  Vector<int> vec_int;
+  vec_int.push_back(0);
+
+  List<bool> list_bool;
+  list_bool.push_back(false);
+
+  N::Set<char> set_char;
+  set_char.insert('A');
+}
+
+void testPendingInstantiations() {
+  // CHECK: call
+  // CHECK: call
+  // CHECK: {{define .*pendingInstantiation.*[(]i}}
+  // CHECK: {{define .*pendingInstantiation.*[(]double}}
+  // CHECK: call
+  triggerPendingInstantiation();
+  triggerPendingInstantiationToo();
+}