From 0c23dc59fcf55491063e94b2f474e55112876c8e Mon Sep 17 00:00:00 2001 From: Balazs Keri <1.int32@gmail.com> Date: Mon, 13 Aug 2018 13:08:37 +0000 Subject: [PATCH] [ASTImporter] Improved import of friend templates. Summary: When importing a friend class template declaration, this declaration should not be merged with any other existing declaration for the same type. Otherwise the getFriendDecl of the FriendDecl can point to an other already referenced declaration, this case causes problems. Additionally the previous decl of class templates is set at import. Reviewers: a.sidorin, a_sidorin Reviewed By: a_sidorin Subscribers: a_sidorin, martong, cfe-commits Differential Revision: https://reviews.llvm.org/D50516 llvm-svn: 339560 --- clang/lib/AST/ASTImporter.cpp | 46 ++++++++++--- clang/unittests/AST/ASTImporterTest.cpp | 87 +++++++++++++++++++++++++ 2 files changed, 123 insertions(+), 10 deletions(-) diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp index 768b2e5bedb3..c1136c9f428c 100644 --- a/clang/lib/AST/ASTImporter.cpp +++ b/clang/lib/AST/ASTImporter.cpp @@ -2164,11 +2164,21 @@ Decl *ASTNodeImporter::VisitEnumDecl(EnumDecl *D) { } Decl *ASTNodeImporter::VisitRecordDecl(RecordDecl *D) { + bool IsFriendTemplate = false; + if (auto *DCXX = dyn_cast(D)) { + IsFriendTemplate = + DCXX->getDescribedClassTemplate() && + DCXX->getDescribedClassTemplate()->getFriendObjectKind() != + Decl::FOK_None; + } + // If this record has a definition in the translation unit we're coming from, // but this particular declaration is not that definition, import the // definition and map to that. TagDecl *Definition = D->getDefinition(); if (Definition && Definition != D && + // Friend template declaration must be imported on its own. + !IsFriendTemplate && // In contrast to a normal CXXRecordDecl, the implicit // CXXRecordDecl of ClassTemplateSpecializationDecl is its redeclaration. // The definition of the implicit CXXRecordDecl in this case is the @@ -2241,7 +2251,7 @@ Decl *ASTNodeImporter::VisitRecordDecl(RecordDecl *D) { PrevDecl = FoundRecord; if (RecordDecl *FoundDef = FoundRecord->getDefinition()) { - if ((SearchName && !D->isCompleteDefinition()) + if ((SearchName && !D->isCompleteDefinition() && !IsFriendTemplate) || (D->isCompleteDefinition() && D->isAnonymousStructOrUnion() == FoundDef->isAnonymousStructOrUnion() && @@ -2281,6 +2291,9 @@ Decl *ASTNodeImporter::VisitRecordDecl(RecordDecl *D) { !IsStructuralMatch(D, FoundRecord)) continue; + if (IsFriendTemplate) + continue; + AdoptDecl = FoundRecord; continue; } else if (!SearchName) { @@ -2348,7 +2361,7 @@ Decl *ASTNodeImporter::VisitRecordDecl(RecordDecl *D) { if (!ToDescribed) return nullptr; D2CXX->setDescribedClassTemplate(ToDescribed); - if (!DCXX->isInjectedClassName()) { + if (!DCXX->isInjectedClassName() && !IsFriendTemplate) { // In a record describing a template the type should be an // InjectedClassNameType (see Sema::CheckClassTemplate). Update the // previously set type to the correct value here (ToDescribed is not @@ -4371,12 +4384,14 @@ static ClassTemplateDecl *getDefinition(ClassTemplateDecl *D) { } Decl *ASTNodeImporter::VisitClassTemplateDecl(ClassTemplateDecl *D) { + bool IsFriend = D->getFriendObjectKind() != Decl::FOK_None; + // If this record has a definition in the translation unit we're coming from, // but this particular declaration is not that definition, import the // definition and map to that. auto *Definition = cast_or_null(D->getTemplatedDecl()->getDefinition()); - if (Definition && Definition != D->getTemplatedDecl()) { + if (Definition && Definition != D->getTemplatedDecl() && !IsFriend) { Decl *ImportedDef = Importer.Import(Definition->getDescribedClassTemplate()); if (!ImportedDef) @@ -4413,17 +4428,20 @@ Decl *ASTNodeImporter::VisitClassTemplateDecl(ClassTemplateDecl *D) { // definition. So, try to get the definition if that is available in // the redecl chain. ClassTemplateDecl *TemplateWithDef = getDefinition(FoundTemplate); - if (!TemplateWithDef) + if (TemplateWithDef) + FoundTemplate = TemplateWithDef; + else continue; - FoundTemplate = TemplateWithDef; // Continue with the definition. } if (IsStructuralMatch(D, FoundTemplate)) { - // The class templates structurally match; call it the same template. + if (!IsFriend) { + Importer.MapImported(D->getTemplatedDecl(), + FoundTemplate->getTemplatedDecl()); + return Importer.MapImported(D, FoundTemplate); + } - Importer.MapImported(D->getTemplatedDecl(), - FoundTemplate->getTemplatedDecl()); - return Importer.MapImported(D, FoundTemplate); + continue; } } @@ -4461,9 +4479,17 @@ Decl *ASTNodeImporter::VisitClassTemplateDecl(ClassTemplateDecl *D) { ToTemplated->setDescribedClassTemplate(D2); + if (ToTemplated->getPreviousDecl()) { + assert( + ToTemplated->getPreviousDecl()->getDescribedClassTemplate() && + "Missing described template"); + D2->setPreviousDecl( + ToTemplated->getPreviousDecl()->getDescribedClassTemplate()); + } D2->setAccess(D->getAccess()); D2->setLexicalDeclContext(LexicalDC); - LexicalDC->addDeclInternal(D2); + if (!IsFriend) + LexicalDC->addDeclInternal(D2); if (FromTemplated->isCompleteDefinition() && !ToTemplated->isCompleteDefinition()) { diff --git a/clang/unittests/AST/ASTImporterTest.cpp b/clang/unittests/AST/ASTImporterTest.cpp index 7d61e98be1a7..4e13f295aa34 100644 --- a/clang/unittests/AST/ASTImporterTest.cpp +++ b/clang/unittests/AST/ASTImporterTest.cpp @@ -2677,6 +2677,93 @@ TEST_P(ASTImporterTestBase, ImportUnnamedFieldsInCorrectOrder) { EXPECT_EQ(FromIndex, 3u); } +TEST_P( + ASTImporterTestBase, + ImportOfFriendRecordDoesNotMergeDefinition) { + Decl *FromTU = getTuDecl( + R"( + class A { + template class F {}; + class X { + template friend class F; + }; + }; + )", + Lang_CXX, "input0.cc"); + + auto *FromClass = FirstDeclMatcher().match( + FromTU, cxxRecordDecl(hasName("F"), isDefinition())); + auto *FromFriendClass = LastDeclMatcher().match( + FromTU, cxxRecordDecl(hasName("F"))); + + ASSERT_TRUE(FromClass); + ASSERT_TRUE(FromFriendClass); + ASSERT_NE(FromClass, FromFriendClass); + ASSERT_EQ(FromFriendClass->getDefinition(), FromClass); + ASSERT_EQ(FromFriendClass->getPreviousDecl(), FromClass); + ASSERT_EQ( + FromFriendClass->getDescribedClassTemplate()->getPreviousDecl(), + FromClass->getDescribedClassTemplate()); + + auto *ToClass = cast(Import(FromClass, Lang_CXX)); + auto *ToFriendClass = cast(Import(FromFriendClass, Lang_CXX)); + + EXPECT_TRUE(ToClass); + EXPECT_TRUE(ToFriendClass); + EXPECT_NE(ToClass, ToFriendClass); + EXPECT_EQ(ToFriendClass->getDefinition(), ToClass); + EXPECT_EQ(ToFriendClass->getPreviousDecl(), ToClass); + EXPECT_EQ( + ToFriendClass->getDescribedClassTemplate()->getPreviousDecl(), + ToClass->getDescribedClassTemplate()); +} + +TEST_P( + ASTImporterTestBase, + ImportOfRecursiveFriendClass) { + Decl *FromTu = getTuDecl( + R"( + class declToImport { + friend class declToImport; + }; + )", + Lang_CXX, "input.cc"); + + auto *FromD = FirstDeclMatcher().match( + FromTu, cxxRecordDecl(hasName("declToImport"))); + auto *ToD = Import(FromD, Lang_CXX); + auto Pattern = cxxRecordDecl(hasName("declToImport"), has(friendDecl())); + ASSERT_TRUE(MatchVerifier{}.match(FromD, Pattern)); + EXPECT_TRUE(MatchVerifier{}.match(ToD, Pattern)); +} + +TEST_P( + ASTImporterTestBase, + ImportOfRecursiveFriendClassTemplate) { + Decl *FromTu = getTuDecl( + R"( + template class declToImport { + template friend class declToImport; + }; + )", + Lang_CXX, "input.cc"); + + auto *FromD = FirstDeclMatcher().match( + FromTu, classTemplateDecl(hasName("declToImport"))); + auto *ToD = Import(FromD, Lang_CXX); + + auto Pattern = classTemplateDecl( + has(cxxRecordDecl(has(friendDecl(has(classTemplateDecl())))))); + ASSERT_TRUE(MatchVerifier{}.match(FromD, Pattern)); + EXPECT_TRUE(MatchVerifier{}.match(ToD, Pattern)); + + auto *Class = + FirstDeclMatcher().match(ToD, classTemplateDecl()); + auto *Friend = FirstDeclMatcher().match(ToD, friendDecl()); + EXPECT_NE(Friend->getFriendDecl(), Class); + EXPECT_EQ(Friend->getFriendDecl()->getPreviousDecl(), Class); +} + struct DeclContextTest : ASTImporterTestBase {}; TEST_P(DeclContextTest, removeDeclOfClassTemplateSpecialization) {