Fix duplicate class template definitions problem

Summary:
We fail to import a `ClassTemplateDecl` if the "To" context already
contains a definition and then a forward decl.  This is because
`localUncachedLookup` does not find the definition.  This is not a
lookup error, the parser behaves differently than assumed in the
importer code.  A `DeclContext` contains one DenseMap (`LookupPtr`)
which maps names to lists.  The list is a special list `StoredDeclsList`
which is optimized to have one element.  During building the initial
AST, the parser first adds the definition to the `DeclContext`.  Then
during parsing the second declaration (the forward decl) the parser
again calls `DeclContext::addDecl` but that will not add a new element
to the `StoredDeclsList` rarther it simply overwrites the old element
with the most recent one.  This patch fixes the error by finding the
definition in the redecl chain.  Added tests for the same issue with
`CXXRecordDecl` and with `ClassTemplateSpecializationDecl`.  These tests
pass and they pass because in `VisitRecordDecl` and in
`VisitClassTemplateSpecializationDecl` we already use
`D->getDefinition()` after the lookup.

Reviewers: a.sidorin, xazax.hun, szepet

Subscribers: rnkovacs, dkrupp, cfe-commits

Differential Revision: https://reviews.llvm.org/D46950

llvm-svn: 333082

1 files changed, 31 insertions, 15 deletions

diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp
index 6fa4617cf06..3c698974364 100644
--- a/clang/lib/AST/ASTImporter.cpp
+++ b/clang/lib/AST/ASTImporter.cpp
@@ -4070,6 +4070,17 @@ ASTNodeImporter::VisitTemplateTemplateParmDecl(TemplateTemplateParmDecl *D) {
                                           TemplateParams);
 }
 
+// Returns the definition for a (forward) declaration of a ClassTemplateDecl, if
+// it has any definition in the redecl chain.
+static ClassTemplateDecl *getDefinition(ClassTemplateDecl *D) {
+  CXXRecordDecl *ToTemplatedDef = D->getTemplatedDecl()->getDefinition();
+  if (!ToTemplatedDef)
+    return nullptr;
+  ClassTemplateDecl *TemplateWithDef =
+      ToTemplatedDef->getDescribedClassTemplate();
+  return TemplateWithDef;
+}
+
 Decl *ASTNodeImporter::VisitClassTemplateDecl(ClassTemplateDecl *D) {
   // If this record has a definition in the translation unit we're coming from,
   // but this particular declaration is not that definition, import the
@@ -4084,7 +4095,7 @@ Decl *ASTNodeImporter::VisitClassTemplateDecl(ClassTemplateDecl *D) {
 
     return Importer.Imported(D, ImportedDef);
   }
-  
+
   // Import the major distinguishing characteristics of this class template.
   DeclContext *DC, *LexicalDC;
   DeclarationName Name;
@@ -4103,34 +4114,39 @@ Decl *ASTNodeImporter::VisitClassTemplateDecl(ClassTemplateDecl *D) {
     for (auto *FoundDecl : FoundDecls) {
       if (!FoundDecl->isInIdentifierNamespace(Decl::IDNS_Ordinary))
         continue;
-      
+
       Decl *Found = FoundDecl;
       if (auto *FoundTemplate = dyn_cast<ClassTemplateDecl>(Found)) {
-        if (IsStructuralMatch(D, FoundTemplate)) {
-          // The class templates structurally match; call it the same template.
 
-          // We found a forward declaration but the class to be imported has a
-          // definition.
-          // FIXME Add this forward declaration to the redeclaration chain.
-          if (D->isThisDeclarationADefinition() &&
-              !FoundTemplate->isThisDeclarationADefinition())
+        // The class to be imported is a definition.
+        if (D->isThisDeclarationADefinition()) {
+          // Lookup will find the fwd decl only if that is more recent than the
+          // definition. So, try to get the definition if that is available in
+          // the redecl chain.
+          ClassTemplateDecl *TemplateWithDef = getDefinition(FoundTemplate);
+          if (!TemplateWithDef)
             continue;
+          FoundTemplate = TemplateWithDef; // Continue with the definition.
+        }
+
+        if (IsStructuralMatch(D, FoundTemplate)) {
+          // The class templates structurally match; call it the same template.
 
-          Importer.Imported(D->getTemplatedDecl(), 
+          Importer.Imported(D->getTemplatedDecl(),
                             FoundTemplate->getTemplatedDecl());
           return Importer.Imported(D, FoundTemplate);
-        }         
+        }
       }
-      
+
       ConflictingDecls.push_back(FoundDecl);
     }
-    
+
     if (!ConflictingDecls.empty()) {
       Name = Importer.HandleNameConflict(Name, DC, Decl::IDNS_Ordinary,
-                                         ConflictingDecls.data(), 
+                                         ConflictingDecls.data(),
                                          ConflictingDecls.size());
     }
-    
+
     if (!Name)
       return nullptr;
   }