[modules] Add an assert for redeclarations that we never added to their redecl

chain and fix the cases where it fires.

 * Handle the __va_list_tag as a predefined decl. Previously we failed to merge
   sometimes it because it's not visible to name lookup. (In passing, remove
   redundant __va_list_tag typedefs that we were creating for some ABIs. These
   didn't affect the mangling or representation of the type.)

 * For Decls derived from Redeclarable that are not in fact redeclarable
   (implicit params, function params, ObjC type parameters), remove them from
   the list of expected redeclarable decls.

llvm-svn: 243259

1 files changed, 30 insertions, 17 deletions

diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp b/clang/lib/Serialization/ASTReaderDecl.cpp
index f6858b4a5b7..45a3e45bbcf 100644
--- a/clang/lib/Serialization/ASTReaderDecl.cpp
+++ b/clang/lib/Serialization/ASTReaderDecl.cpp
@@ -126,35 +126,43 @@ namespace clang {
     class RedeclarableResult {
       ASTReader &Reader;
       GlobalDeclID FirstID;
+      Decl *LoadedDecl;
       Decl *MergeWith;
       mutable bool Owning;
       bool IsKeyDecl;
-      Decl::Kind DeclKind;
 
       void operator=(RedeclarableResult &) = delete;
 
     public:
       RedeclarableResult(ASTReader &Reader, GlobalDeclID FirstID,
-                         Decl *MergeWith, Decl::Kind DeclKind,
-                         bool IsKeyDecl)
-        : Reader(Reader), FirstID(FirstID), MergeWith(MergeWith),
-          Owning(true), IsKeyDecl(IsKeyDecl), DeclKind(DeclKind) {}
+                         Decl *LoadedDecl, Decl *MergeWith, bool IsKeyDecl)
+          : Reader(Reader), FirstID(FirstID), LoadedDecl(LoadedDecl),
+            MergeWith(MergeWith), Owning(true), IsKeyDecl(IsKeyDecl) {}
 
       RedeclarableResult(RedeclarableResult &&Other)
-        : Reader(Other.Reader), FirstID(Other.FirstID),
-          MergeWith(Other.MergeWith), Owning(Other.Owning),
-          IsKeyDecl(Other.IsKeyDecl), DeclKind(Other.DeclKind) {
+          : Reader(Other.Reader), FirstID(Other.FirstID),
+            LoadedDecl(Other.LoadedDecl), MergeWith(Other.MergeWith),
+            Owning(Other.Owning), IsKeyDecl(Other.IsKeyDecl) {
         Other.Owning = false;
       }
 
       ~RedeclarableResult() {
-        if (FirstID && Owning && isRedeclarableDeclKind(DeclKind)) {
+        if (FirstID && Owning &&
+            isRedeclarableDeclKind(LoadedDecl->getKind())) {
           auto Canon = Reader.GetDecl(FirstID)->getCanonicalDecl();
           if (Reader.PendingDeclChainsKnown.insert(Canon).second)
             Reader.PendingDeclChains.push_back(Canon);
         }
       }
 
+      /// \brief Note that a RedeclarableDecl is not actually redeclarable.
+      void setNotRedeclarable() {
+        Owning = false;
+        Reader.RedeclsDeserialized.erase(LoadedDecl);
+        assert(FirstID == LoadedDecl->getGlobalID() && !MergeWith &&
+               "non-redeclarable declaration was redeclared?");
+      }
+
       /// \brief Retrieve the first ID.
       GlobalDeclID getFirstID() const { return FirstID; }
 
@@ -908,7 +916,9 @@ void ASTDeclReader::VisitObjCMethodDecl(ObjCMethodDecl *MD) {
 }
 
 void ASTDeclReader::VisitObjCTypeParamDecl(ObjCTypeParamDecl *D) {
-  VisitTypedefNameDecl(D);
+  RedeclarableResult Redecl = VisitTypedefNameDecl(D);
+  Redecl.setNotRedeclarable();
+
   D->Variance = Record[Idx++];
   D->Index = Record[Idx++];
   D->VarianceLoc = ReadSourceLocation(Record, Idx);
@@ -1208,9 +1218,11 @@ ASTDeclReader::RedeclarableResult ASTDeclReader::VisitVarDeclImpl(VarDecl *VD) {
   };
   switch ((VarKind)Record[Idx++]) {
   case VarNotTemplate:
-    // Only true variables (not parameters or implicit parameters) can be merged
-    if (VD->getKind() != Decl::ParmVar && VD->getKind() != Decl::ImplicitParam &&
-        !isa<VarTemplateSpecializationDecl>(VD))
+    // Only true variables (not parameters or implicit parameters) can be
+    // merged; the other kinds are not really redeclarable at all.
+    if (isa<ParmVarDecl>(VD) || isa<ImplicitParamDecl>(VD))
+      Redecl.setNotRedeclarable();
+    else if (!isa<VarTemplateSpecializationDecl>(VD))
       mergeRedeclarable(VD, Redecl);
     break;
   case VarTemplate:
@@ -2070,6 +2082,7 @@ ASTDeclReader::VisitVarTemplateSpecializationDeclImpl(
   if (writtenAsCanonicalDecl) {
     VarTemplateDecl *CanonPattern = ReadDeclAs<VarTemplateDecl>(Record, Idx);
     if (D->isCanonicalDecl()) { // It's kept in the folding set.
+      // FIXME: If it's already present, merge it.
       if (VarTemplatePartialSpecializationDecl *Partial =
               dyn_cast<VarTemplatePartialSpecializationDecl>(D)) {
         CanonPattern->getCommonPtr()->PartialSpecializations
@@ -2212,8 +2225,8 @@ ASTDeclReader::VisitRedeclarable(Redeclarable<T> *D) {
                              
   // The result structure takes care to note that we need to load the 
   // other declaration chains for this ID.
-  return RedeclarableResult(Reader, FirstDeclID, MergeWith,
-                            static_cast<T *>(D)->getKind(), IsKeyDecl);
+  return RedeclarableResult(Reader, FirstDeclID, static_cast<T *>(D), MergeWith,
+                            IsKeyDecl);
 }
 
 /// \brief Attempts to merge the given declaration (D) with another declaration
@@ -2256,8 +2269,7 @@ void ASTDeclReader::mergeTemplatePattern(RedeclarableTemplateDecl *D,
   auto *DPattern = D->getTemplatedDecl();
   auto *ExistingPattern = Existing->getTemplatedDecl();
   RedeclarableResult Result(Reader, DPattern->getCanonicalDecl()->getGlobalID(),
-                            /*MergeWith*/ExistingPattern, DPattern->getKind(),
-                            IsKeyDecl);
+                            DPattern, /*MergeWith*/ ExistingPattern, IsKeyDecl);
 
   if (auto *DClass = dyn_cast<CXXRecordDecl>(DPattern)) {
     // Merge with any existing definition.
@@ -3540,6 +3552,7 @@ void ASTReader::loadPendingDeclChain(Decl *CanonDecl) {
   // Build up the list of redeclarations.
   RedeclChainVisitor Visitor(*this, SearchDecls, RedeclsDeserialized, CanonID);
   ModuleMgr.visit(Visitor);
+  RedeclsDeserialized.erase(CanonDecl);
 
   // Retrieve the chains.
   ArrayRef<Decl *> Chain = Visitor.getChain();