Author: Raphael Isemann Date: 2020-11-24T23:46:18+01:00 New Revision: 0c926e6d245bec176bf2554a9f0bd48ef2276678
URL: https://github.com/llvm/llvm-project/commit/0c926e6d245bec176bf2554a9f0bd48ef2276678 DIFF: https://github.com/llvm/llvm-project/commit/0c926e6d245bec176bf2554a9f0bd48ef2276678.diff LOG: [ASTImporter] Make the Import() return value consistent with the map of imported decls when merging ClassTemplateSpecializationDecls When importing a `ClassTemplateSpecializationDecl` definition into a TU with a matching `ClassTemplateSpecializationDecl` definition and a more recent forward decl, the ASTImporter currently will call `MapImported()` for the definitions, but will return the forward declaration from the `ASTImporter::Import()` call. This is triggering some assertions in LLDB when we try to fully import some DeclContexts before we delete the 'From' AST. The returned 'To' Decl before this patch is just the most recent forward decl but that's not the Decl with the definition to which the ASTImporter will import the child declarations. This patch just changes that the ASTImporter returns the definition that the imported Decl was merged with instead of the found forward declaration. Reviewed By: martong Differential Revision: https://reviews.llvm.org/D92016 Added: Modified: clang/lib/AST/ASTImporter.cpp clang/unittests/AST/ASTImporterTest.cpp Removed: ################################################################################ diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp index de1f13edf3ca..0886980fe905 100644 --- a/clang/lib/AST/ASTImporter.cpp +++ b/clang/lib/AST/ASTImporter.cpp @@ -5423,8 +5423,9 @@ ExpectedDecl ASTNodeImporter::VisitClassTemplateSpecializationDecl( if (PrevDecl) { if (IsStructuralMatch(D, PrevDecl)) { - if (D->isThisDeclarationADefinition() && PrevDecl->getDefinition()) { - Importer.MapImported(D, PrevDecl->getDefinition()); + CXXRecordDecl *PrevDefinition = PrevDecl->getDefinition(); + if (D->isThisDeclarationADefinition() && PrevDefinition) { + Importer.MapImported(D, PrevDefinition); // Import those default field initializers which have been // instantiated in the "From" context, but not in the "To" context. for (auto *FromField : D->fields()) { @@ -5446,7 +5447,7 @@ ExpectedDecl ASTNodeImporter::VisitClassTemplateSpecializationDecl( // // Generally, ASTCommon.h/DeclUpdateKind enum gives a very good hint // what else could be fused during an AST merge. - return PrevDecl; + return PrevDefinition; } } else { // ODR violation. // FIXME HandleNameConflict diff --git a/clang/unittests/AST/ASTImporterTest.cpp b/clang/unittests/AST/ASTImporterTest.cpp index f869e492c90a..97a18a76622b 100644 --- a/clang/unittests/AST/ASTImporterTest.cpp +++ b/clang/unittests/AST/ASTImporterTest.cpp @@ -3104,6 +3104,25 @@ TEST_P(ASTImporterOptionSpecificTestBase, EXPECT_TRUE(ToFun->hasBody()); } +TEST_P(ASTImporterOptionSpecificTestBase, MergeTemplateSpecWithForwardDecl) { + std::string ClassTemplate = + R"( + template<typename T> + struct X { int m; }; + template<> + struct X<int> { int m; }; + )"; + // Append a forward decl for our template specialization. + getToTuDecl(ClassTemplate + "template<> struct X<int>;", Lang_CXX11); + Decl *FromTU = getTuDecl(ClassTemplate, Lang_CXX11); + auto *FromSpec = FirstDeclMatcher<ClassTemplateSpecializationDecl>().match( + FromTU, classTemplateSpecializationDecl(hasName("X"), isDefinition())); + auto *ImportedSpec = Import(FromSpec, Lang_CXX11); + // Check that our definition got merged with the existing definition. + EXPECT_TRUE(FromSpec->isThisDeclarationADefinition()); + EXPECT_TRUE(ImportedSpec->isThisDeclarationADefinition()); +} + TEST_P(ASTImporterOptionSpecificTestBase, ODRViolationOfClassTemplateSpecializationsShouldBeReported) { std::string ClassTemplate = _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
