Re-implement (de-)serialization of redeclaration chains for

redeclaration templates (RedeclarableTemplateDecl), similarly to the
way (de-)serialization is implemented for Redeclarable<T>. In the
process, found a simpler formulation for handling redeclaration
chains and implemented that in both places.

The new test establishes that we're building the redeclaration chains
properly. However, the FIXME indicates where we're tickling a
different bug that has to do with us not setting the DefinitionData
pointer properly in redeclarations that we detected after the
definition itself was deserialized. The (separable) fix for that bug
is forthcoming.


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

7 files changed, 123 insertions, 106 deletions

diff --git a/lib/Serialization/ASTReaderDecl.cpp b/lib/Serialization/ASTReaderDecl.cpp
index 3e08d99ae6..35cd8478d5 100644
--- a/lib/Serialization/ASTReaderDecl.cpp
+++ b/lib/Serialization/ASTReaderDecl.cpp
@@ -1162,53 +1162,55 @@ void ASTDeclReader::VisitTemplateDecl(TemplateDecl *D) {
 void ASTDeclReader::VisitRedeclarableTemplateDecl(RedeclarableTemplateDecl *D) {
   // Initialize CommonOrPrev before VisitTemplateDecl so that getCommonPtr()
   // can be used while this is still initializing.
-
-  assert(D->CommonOrPrev.isNull() && "getCommonPtr was called earlier on this");
-  DeclID PreviousDeclID = ReadDeclID(Record, Idx);
-  DeclID FirstDeclID =  PreviousDeclID ? ReadDeclID(Record, Idx) : 0;
-  // We delay loading of the redeclaration chain to avoid deeply nested calls.
-  // We temporarily set the first (canonical) declaration as the previous one
-  // which is the one that matters and mark the real previous DeclID to be
-  // loaded & attached later on.
-  RedeclarableTemplateDecl *FirstDecl =
-      cast_or_null<RedeclarableTemplateDecl>(Reader.GetDecl(FirstDeclID));
-  assert((FirstDecl == 0 || FirstDecl->getKind() == D->getKind()) &&
-         "FirstDecl kind mismatch");
-  if (FirstDecl) {
-    D->CommonOrPrev = FirstDecl;
-    // Mark the real previous DeclID to be loaded & attached later on.
-    if (PreviousDeclID != FirstDeclID)
-      Reader.PendingPreviousDecls.push_back(std::make_pair(D, PreviousDeclID));
-  } else {
-    D->CommonOrPrev = D->newCommon(Reader.getContext());
+  enum RedeclKind { FirstDeclaration, PointsToPrevious };
+  RedeclKind Kind = (RedeclKind)Record[Idx++];
+  
+  // Determine the first declaration ID.
+  DeclID FirstDeclID;
+  switch (Kind) {
+  case FirstDeclaration: {
+    FirstDeclID = ThisDeclID;
+
+    // Since this is the first declaration of the template, fill in the 
+    // information for the 'common' pointer.
+    if (D->CommonOrPrev.isNull()) {
+      RedeclarableTemplateDecl::CommonBase *Common
+        = D->newCommon(Reader.getContext());
+      Common->Latest = D;
+      D->CommonOrPrev = Common;
+    }
+    
     if (RedeclarableTemplateDecl *RTD
-          = ReadDeclAs<RedeclarableTemplateDecl>(Record, Idx)) {
+        = ReadDeclAs<RedeclarableTemplateDecl>(Record, Idx)) {
       assert(RTD->getKind() == D->getKind() &&
              "InstantiatedFromMemberTemplate kind mismatch");
       D->setInstantiatedFromMemberTemplateImpl(RTD);
       if (Record[Idx++])
         D->setMemberSpecialization();
     }
-
-    RedeclarableTemplateDecl *LatestDecl
-      = ReadDeclAs<RedeclarableTemplateDecl>(Record, Idx);
-  
-    // This decl is a first one and the latest declaration that it points to is
-    // in the same AST file. However, if this actually needs to point to a
-    // redeclaration in another AST file, we need to update it by checking
-    // the FirstLatestDeclIDs map which tracks this kind of decls.
-    assert(Reader.GetDecl(ThisDeclID) == D && "Invalid ThisDeclID ?");
-    ASTReader::FirstLatestDeclIDMap::iterator I
-        = Reader.FirstLatestDeclIDs.find(ThisDeclID);
-    if (I != Reader.FirstLatestDeclIDs.end()) {
-      if (Decl *NewLatest = Reader.GetDecl(I->second))
-        LatestDecl = cast<RedeclarableTemplateDecl>(NewLatest);
-    }
-
-    assert(LatestDecl->getKind() == D->getKind() && "Latest kind mismatch");
-    D->getCommonPtr()->Latest = LatestDecl;
+    break;
   }
-
+      
+  case PointsToPrevious: {
+    FirstDeclID = ReadDeclID(Record, Idx);
+    DeclID PrevDeclID = ReadDeclID(Record, Idx);
+    
+    RedeclarableTemplateDecl *FirstDecl
+      = cast_or_null<RedeclarableTemplateDecl>(Reader.GetDecl(FirstDeclID));
+    
+    // We delay loading of the redeclaration chain to avoid deeply nested calls.
+    // We temporarily set the first (canonical) declaration as the previous one
+    // which is the one that matters and mark the real previous DeclID to be
+    // loaded and attached later on.
+    D->CommonOrPrev = FirstDecl;
+    
+    // Make a note that we need to wire up this declaration to its
+    // previous declaration, later.
+    Reader.PendingPreviousDecls.push_back(std::make_pair(D, PrevDeclID));
+    break;
+  }
+  }
+  
   VisitTemplateDecl(D);
   D->IdentifierNamespace = Record[Idx++];
 }
@@ -1410,37 +1412,20 @@ ASTDeclReader::VisitDeclContext(DeclContext *DC) {
 
 template <typename T>
 void ASTDeclReader::VisitRedeclarable(Redeclarable<T> *D) {
-  enum RedeclKind { OnlyDeclaration = 0, FirstInFile, PointsToPrevious };
+  enum RedeclKind { FirstDeclaration = 0, PointsToPrevious };
   RedeclKind Kind = (RedeclKind)Record[Idx++];
   
-  // If this is the only known declaration of this entity, this module file
-  // has no additional redeclaration information. However, other module
-  // files might have redeclarations.
-  if (Kind == OnlyDeclaration) {
-    if (Reader.PendingDeclChainsKnown.insert(ThisDeclID))
-      Reader.PendingDeclChains.push_back(ThisDeclID);
-    return;
-  }
-  
-  // Read the first declaration ID, and note that we need to reconstruct
-  // the redeclaration chain once we hit the top level.
-  DeclID FirstDeclID = ReadDeclID(Record, Idx);
-  if (Reader.PendingDeclChainsKnown.insert(FirstDeclID))
-    Reader.PendingDeclChains.push_back(FirstDeclID);
-
-  T *FirstDecl = cast_or_null<T>(Reader.GetDecl(FirstDeclID));
-
+  DeclID FirstDeclID;
   switch (Kind) {
-  case OnlyDeclaration:
-    llvm_unreachable("only declaration handled above");
-      
-  case FirstInFile:
-    if (FirstDecl != D)
-      D->RedeclLink = typename Redeclarable<T>::PreviousDeclLink(FirstDecl);
+  case FirstDeclaration:
+    FirstDeclID = ThisDeclID;
     break;
       
   case PointsToPrevious: {
-    DeclID PreviousDeclID = ReadDeclID(Record, Idx);
+    FirstDeclID = ReadDeclID(Record, Idx);
+    DeclID PrevDeclID = ReadDeclID(Record, Idx);
+    
+    T *FirstDecl = cast_or_null<T>(Reader.GetDecl(FirstDeclID));
     
     // We delay loading of the redeclaration chain to avoid deeply nested calls.
     // We temporarily set the first (canonical) declaration as the previous one
@@ -1451,10 +1436,14 @@ void ASTDeclReader::VisitRedeclarable(Redeclarable<T> *D) {
     // Make a note that we need to wire up this declaration to its
     // previous declaration, later.
     Reader.PendingPreviousDecls.push_back(std::make_pair(static_cast<T*>(D),
-                                                         PreviousDeclID));
+                                                         PrevDeclID));
     break;
   }
   }
+
+  // Note that we need to load the other declaration chains for this ID.
+  if (Reader.PendingDeclChainsKnown.insert(ThisDeclID))
+    Reader.PendingDeclChains.push_back(ThisDeclID);
 }
 
 //===----------------------------------------------------------------------===//
diff --git a/lib/Serialization/ASTWriterDecl.cpp b/lib/Serialization/ASTWriterDecl.cpp
index bb4359a54b..cb1c6f0476 100644
--- a/lib/Serialization/ASTWriterDecl.cpp
+++ b/lib/Serialization/ASTWriterDecl.cpp
@@ -188,7 +188,7 @@ void ASTDeclWriter::VisitTypedefDecl(TypedefDecl *D) {
   if (!D->hasAttrs() &&
       !D->isImplicit() &&
       !D->isUsed(false) &&
-      D->RedeclLink.getPointer() == D &&
+      !D->getPreviousDeclaration() &&
       !D->isInvalidDecl() &&
       !D->isReferenced() &&
       !D->isTopLevelDeclInObjCContainer() &&
@@ -238,7 +238,7 @@ void ASTDeclWriter::VisitEnumDecl(EnumDecl *D) {
       !D->isImplicit() &&
       !D->isUsed(false) &&
       !D->hasExtInfo() &&
-      D->RedeclLink.getPointer() == D &&
+      !D->getPreviousDeclaration() &&
       !D->isInvalidDecl() &&
       !D->isReferenced() &&
       !D->isTopLevelDeclInObjCContainer() &&
@@ -262,7 +262,7 @@ void ASTDeclWriter::VisitRecordDecl(RecordDecl *D) {
       !D->isImplicit() &&
       !D->isUsed(false) &&
       !D->hasExtInfo() &&
-      D->RedeclLink.getPointer() == D &&
+      !D->getPreviousDeclaration() &&
       !D->isInvalidDecl() &&
       !D->isReferenced() &&
       !D->isTopLevelDeclInObjCContainer() &&
@@ -708,7 +708,7 @@ void ASTDeclWriter::VisitVarDecl(VarDecl *D) {
       !D->isModulePrivate() &&
       D->getDeclName().getNameKind() == DeclarationName::Identifier &&
       !D->hasExtInfo() &&
-      D->RedeclLink.getPointer() == D &&
+      !D->getPreviousDeclaration() &&
       !D->hasCXXDirectInitializer() &&
       D->getInit() == 0 &&
       !isa<ParmVarDecl>(D) &&
@@ -1051,32 +1051,34 @@ void ASTDeclWriter::VisitTemplateDecl(TemplateDecl *D) {
 void ASTDeclWriter::VisitRedeclarableTemplateDecl(RedeclarableTemplateDecl *D) {
   // Emit data to initialize CommonOrPrev before VisitTemplateDecl so that
   // getCommonPtr() can be used while this is still initializing.
-
-  Writer.AddDeclRef(D->getPreviousDeclaration(), Record);
-  if (D->getPreviousDeclaration())
-    Writer.AddDeclRef(D->getFirstDeclaration(), Record);
-
-  if (D->getPreviousDeclaration() == 0) {
-    // This TemplateDecl owns the CommonPtr; write it.
-    assert(D->isCanonicalDecl());
-
+  enum { FirstDeclaration, PointsToPrevious };
+  RedeclarableTemplateDecl *Prev = D->getPreviousDeclaration();
+  RedeclarableTemplateDecl *First = 0;
+  if (!Prev) {
+    Record.push_back(FirstDeclaration);
+    
+    // This declaration owns the 'common' pointer, so serialize that data now.
     Writer.AddDeclRef(D->getInstantiatedFromMemberTemplate(), Record);
     if (D->getInstantiatedFromMemberTemplate())
       Record.push_back(D->isMemberSpecialization());
-
-    Writer.AddDeclRef(D->getCommonPtr()->Latest, Record);
   } else {
-    RedeclarableTemplateDecl *First = D->getFirstDeclaration();
-    assert(First != D);
-    // If this is a most recent redeclaration that is pointed to by a first decl
-    // in a chained PCH, keep track of the association with the map so we can
-    // update the first decl during AST reading.
-    if (First->getMostRecentDeclaration() == D &&
-        First->isFromASTFile() && !D->isFromASTFile()) {
-      assert(Writer.FirstLatestDecls.find(First)==Writer.FirstLatestDecls.end()
-             && "The latest is already set");
-      Writer.FirstLatestDecls[First] = D;
-    }
+    First = D->getFirstDeclaration();
+    Record.push_back(PointsToPrevious);
+    Writer.AddDeclRef(First, Record);
+    Writer.AddDeclRef(Prev, Record);    
+  }
+  
+  if (D->getMostRecentDeclaration() != D && (!Prev || Prev->isFromASTFile())) {
+    if (!First)
+      First = D->getFirstDeclaration();
+    
+    // Capture the set of redeclarations in this file.
+    LocalRedeclarationsInfo LocalInfo = {
+      Writer.GetDeclRef(First),
+      Writer.GetDeclRef(D),
+      Writer.GetDeclRef(D->getMostRecentDeclaration())
+    };
+    Writer.LocalRedeclarations.push_back(LocalInfo);
   }
 
   VisitTemplateDecl(D);
@@ -1274,19 +1276,19 @@ void ASTDeclWriter::VisitDeclContext(DeclContext *DC, uint64_t LexicalOffset,
 
 template <typename T>
 void ASTDeclWriter::VisitRedeclarable(Redeclarable<T> *D) {
-  enum { OnlyDeclaration = 0, FirstInFile, PointsToPrevious };
-  if (D->RedeclLink.getPointer() == D) {
-    // This is the only declaration.
-    Record.push_back(OnlyDeclaration);
-    return;
-  }
-  
+  enum { FirstDeclaration = 0, PointsToPrevious };
+  T *Prev = D->getPreviousDeclaration();
   T *First = D->getFirstDeclaration();
-  if (!D->getPreviousDeclaration() ||
-      D->getPreviousDeclaration()->isFromASTFile()) {
-    Record.push_back(FirstInFile);
+  
+  if (!Prev) {
+    Record.push_back(FirstDeclaration);
+  } else {  
+    Record.push_back(PointsToPrevious);
     Writer.AddDeclRef(First, Record);
-
+    Writer.AddDeclRef(D->getPreviousDeclaration(), Record);
+  }
+  
+  if (D->RedeclLink.getPointer() != D && (!Prev || Prev->isFromASTFile())) {
     // Capture the set of redeclarations in this file.
     LocalRedeclarationsInfo LocalInfo = {
       Writer.GetDeclRef(First),
@@ -1294,10 +1296,6 @@ void ASTDeclWriter::VisitRedeclarable(Redeclarable<T> *D) {
       Writer.GetDeclRef(D->getMostRecentDeclaration())
     };
     Writer.LocalRedeclarations.push_back(LocalInfo);    
-  } else {
-    Record.push_back(PointsToPrevious);
-    Writer.AddDeclRef(First, Record);
-    Writer.AddDeclRef(D->getPreviousDeclaration(), Record);
   }
 }
 
diff --git a/test/Modules/Inputs/redecl-merge-bottom.h b/test/Modules/Inputs/redecl-merge-bottom.h
index 198bde3e5f..1c59c1f223 100644
--- a/test/Modules/Inputs/redecl-merge-bottom.h
+++ b/test/Modules/Inputs/redecl-merge-bottom.h
@@ -5,3 +5,11 @@ __import_module__ redecl_merge_right;
 
 @class A;
 
+#ifdef __cplusplus
+template<typename T> class Vector;
+
+template<typename T> class Vector;
+
+template<typename T> class Vector;
+#endif
+
diff --git a/test/Modules/Inputs/redecl-merge-left.h b/test/Modules/Inputs/redecl-merge-left.h
index d97fb7b25c..8e2fa53ae1 100644
--- a/test/Modules/Inputs/redecl-merge-left.h
+++ b/test/Modules/Inputs/redecl-merge-left.h
@@ -10,3 +10,8 @@ __import_module__ redecl_merge_top;
 
 @class A;
 
+#ifdef __cplusplus
+template<typename T> class Vector;
+
+template<typename T> class Vector;
+#endif
diff --git a/test/Modules/Inputs/redecl-merge-right.h b/test/Modules/Inputs/redecl-merge-right.h
index d8fd45f2b9..6d69734711 100644
--- a/test/Modules/Inputs/redecl-merge-right.h
+++ b/test/Modules/Inputs/redecl-merge-right.h
@@ -9,3 +9,9 @@ __import_module__ redecl_merge_top;
 
 @class B;
 
+#ifdef __cplusplus
+template<typename T> class Vector { 
+public:
+  void push_back(const T&);
+};
+#endif
diff --git a/test/Modules/Inputs/redecl-merge-top.h b/test/Modules/Inputs/redecl-merge-top.h
index 68dae5058e..64e2cc58d9 100644
--- a/test/Modules/Inputs/redecl-merge-top.h
+++ b/test/Modules/Inputs/redecl-merge-top.h
@@ -5,3 +5,7 @@
 @class A;
 
 @class B;
+
+#ifdef __cplusplus
+template<typename T> class Vector;
+#endif
diff --git a/test/Modules/redecl-merge.m b/test/Modules/redecl-merge.m
index 23524af3a5..c5da754dfe 100644
--- a/test/Modules/redecl-merge.m
+++ b/test/Modules/redecl-merge.m
@@ -29,3 +29,10 @@ __import_module__ redecl_merge_bottom;
 void g(A *a) {
   [a init];
 }
+
+#ifdef __cplusplus
+void testVector() {
+  Vector<int> *vec_int;
+  // FIXME:  vec_int.push_back(0);
+}
+#endif