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.

llvm-svn: 146883

clang/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.
+  enum RedeclKind { FirstDeclaration, PointsToPrevious };
+  RedeclKind Kind = (RedeclKind)Record[Idx++];
+  
+  // Determine the first declaration ID.
+  DeclID FirstDeclID;
+  switch (Kind) {
+  case FirstDeclaration: {
+    FirstDeclID = ThisDeclID;
 
-  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());
+    // 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);
 }
 
 //===----------------------------------------------------------------------===//

clang/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 (!Prev) {
+    Record.push_back(FirstDeclaration);
+  } else {  
+    Record.push_back(PointsToPrevious);
+    Writer.AddDeclRef(First, Record);
+    Writer.AddDeclRef(D->getPreviousDeclaration(), Record);
   }
   
-  T *First = D->getFirstDeclaration();
-  if (!D->getPreviousDeclaration() ||
-      D->getPreviousDeclaration()->isFromASTFile()) {
-    Record.push_back(FirstInFile);
-    Writer.AddDeclRef(First, 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);
   }
 }
 

clang/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
+

clang/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

clang/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

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

@@ -5,3 +5,7 @@
 @class A;
 
 @class B;
+
+#ifdef __cplusplus
+template<typename T> class Vector;
+#endif

clang/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