Index: lib/AST/ASTImporter.cpp =================================================================== --- lib/AST/ASTImporter.cpp +++ lib/AST/ASTImporter.cpp @@ -4070,6 +4070,17 @@ TemplateParams); } +// Returns the definition for a (forwad) 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 @@ return Importer.Imported(D, ImportedDef); } - + // Import the major distinguishing characteristics of this class template. DeclContext *DC, *LexicalDC; DeclarationName Name; @@ -4103,34 +4114,39 @@ for (auto *FoundDecl : FoundDecls) { if (!FoundDecl->isInIdentifierNamespace(Decl::IDNS_Ordinary)) continue; - + Decl *Found = FoundDecl; if (auto *FoundTemplate = dyn_cast(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 has a definition. + if (D->isThisDeclarationADefinition()) { + // Lookup will find the fwd decl only if that is more recent than the + // definition. So, lets 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; } Index: unittests/AST/ASTImporterTest.cpp =================================================================== --- unittests/AST/ASTImporterTest.cpp +++ unittests/AST/ASTImporterTest.cpp @@ -281,8 +281,9 @@ return std::make_tuple(*FoundDecls.begin(), Imported); } - // Creates a TU decl for the given source code. - // May be called several times in a given test. + // Creates a TU decl for the given source code which can be used as a From + // context. May be called several times in a given test (with different file + // name). TranslationUnitDecl *getTuDecl(StringRef SrcCode, Language Lang, StringRef FileName = "input.cc") { assert( @@ -297,6 +298,16 @@ return Tu.TUDecl; } + // Creates the To context with the given source code and returns the TU decl. + TranslationUnitDecl *getToTuDecl(StringRef ToSrcCode, Language ToLang) { + ArgVector ToArgs = getArgVectorForLanguage(ToLang); + ToCode = ToSrcCode; + assert(!ToAST); + ToAST = tooling::buildASTFromCodeWithArgs(ToCode, ToArgs, OutputFileName); + + return ToAST->getASTContext().getTranslationUnitDecl(); + } + // Import the given Decl into the ToCtx. // May be called several times in a given test. // The different instances of the param From may have different ASTContext. @@ -1464,6 +1475,120 @@ } } +TEST_P(ASTImporterTestBase, + ImportDefinitionOfClassTemplateIfThereIsAnExistingFwdDeclAndDefinition) { + Decl *ToTU = getToTuDecl( + R"( + template + struct B { + void f(); + }; + + template + struct B; + )", + Lang_CXX); + ASSERT_EQ(1u, DeclCounterWithPredicate( + [](const ClassTemplateDecl *T) { + return T->isThisDeclarationADefinition(); + }) + .match(ToTU, classTemplateDecl())); + + Decl *FromTU = getTuDecl( + R"( + template + struct B { + void f(); + }; + )", + Lang_CXX, "input1.cc"); + ClassTemplateDecl *FromD = FirstDeclMatcher().match( + FromTU, classTemplateDecl(hasName("B"))); + + Import(FromD, Lang_CXX); + + // We should have only one definition. + EXPECT_EQ(1u, DeclCounterWithPredicate( + [](const ClassTemplateDecl *T) { + return T->isThisDeclarationADefinition(); + }) + .match(ToTU, classTemplateDecl())); +} + +TEST_P(ASTImporterTestBase, + ImportDefinitionOfClassIfThereIsAnExistingFwdDeclAndDefinition) { + Decl *ToTU = getToTuDecl( + R"( + struct B { + void f(); + }; + + struct B; + )", + Lang_CXX); + ASSERT_EQ(2u, DeclCounter().match( + ToTU, cxxRecordDecl(hasParent(translationUnitDecl())))); + + Decl *FromTU = getTuDecl( + R"( + struct B { + void f(); + }; + )", + Lang_CXX, "input1.cc"); + auto *FromD = FirstDeclMatcher().match( + FromTU, cxxRecordDecl(hasName("B"))); + + Import(FromD, Lang_CXX); + + EXPECT_EQ(2u, DeclCounter().match( + ToTU, cxxRecordDecl(hasParent(translationUnitDecl())))); +} + +TEST_P( + ASTImporterTestBase, + ImportDefinitionOfClassTemplateSpecializationIfThereIsAnExistingFwdDeclAndDefinition) { + Decl *ToTU = getToTuDecl( + R"( + template + struct B; + + template <> + struct B {}; + + template <> + struct B; + )", + Lang_CXX); + // We should have only one definition. + ASSERT_EQ(1u, DeclCounterWithPredicate( + [](const ClassTemplateSpecializationDecl *T) { + return T->isThisDeclarationADefinition(); + }) + .match(ToTU, classTemplateSpecializationDecl())); + + Decl *FromTU = getTuDecl( + R"( + template + struct B; + + template <> + struct B {}; + )", + Lang_CXX, "input1.cc"); + auto *FromD = FirstDeclMatcher().match( + FromTU, classTemplateSpecializationDecl(hasName("B"))); + + Import(FromD, Lang_CXX); + + // We should have only one definition. + EXPECT_EQ(1u, DeclCounterWithPredicate( + [](const ClassTemplateSpecializationDecl *T) { + return T->isThisDeclarationADefinition(); + }) + .match(ToTU, classTemplateSpecializationDecl())); +} + INSTANTIATE_TEST_CASE_P( ParameterizedTests, ASTImporterTestBase, ::testing::Values(ArgVector(), ArgVector{"-fdelayed-template-parsing"}),); Index: unittests/AST/DeclMatcher.h =================================================================== --- unittests/AST/DeclMatcher.h +++ unittests/AST/DeclMatcher.h @@ -44,24 +44,35 @@ using FirstDeclMatcher = DeclMatcher; template -class DeclCounter : public MatchFinder::MatchCallback { - unsigned Count = 0; +class DeclCounterWithPredicate : public MatchFinder::MatchCallback { + using UnaryPredicate = std::function; + UnaryPredicate predicate; + unsigned count = 0; void run(const MatchFinder::MatchResult &Result) override { - if(Result.Nodes.getNodeAs("")) { - ++Count; - } + if (auto N = Result.Nodes.getNodeAs("")) { + if (predicate(N)) + ++count; + } } + public: - // Returns the number of matched nodes under the tree rooted in `D`. + DeclCounterWithPredicate() + : predicate([](const NodeType *) { return true; }) {} + DeclCounterWithPredicate(UnaryPredicate p) : predicate(p) {} + // Returns the number of matched nodes which satisfy the predicate under the + // tree rooted in `D`. template unsigned match(const Decl *D, const MatcherType &AMatcher) { MatchFinder Finder; Finder.addMatcher(AMatcher.bind(""), this); Finder.matchAST(D->getASTContext()); - return Count; + return count; } }; +template +using DeclCounter = DeclCounterWithPredicate; + } // end namespace ast_matchers } // end namespace clang