Author: Balázs Kéri Date: 2020-07-07T16:24:24+02:00 New Revision: 85f5d1261c9a3f0abc4ae370005a1127174f2ce1
URL: https://github.com/llvm/llvm-project/commit/85f5d1261c9a3f0abc4ae370005a1127174f2ce1 DIFF: https://github.com/llvm/llvm-project/commit/85f5d1261c9a3f0abc4ae370005a1127174f2ce1.diff LOG: [ASTImporter] Corrected import of repeated friend declarations. Summary: Import declarations in correct order if a class contains multiple redundant friend (type or decl) declarations. If the order is incorrect this could cause false structural equivalences and wrong declaration chains after import. Reviewers: a.sidorin, shafik, a_sidorin Reviewed By: shafik Subscribers: dkrupp, Szelethus, gamesh411, teemperor, martong, cfe-commits Tags: #clang Differential Revision: https://reviews.llvm.org/D75740 Added: Modified: clang/lib/AST/ASTImporter.cpp clang/unittests/AST/ASTImporterTest.cpp clang/unittests/AST/StructuralEquivalenceTest.cpp Removed: ################################################################################ diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp index a47be2da4aab..8ec6db622f0a 100644 --- a/clang/lib/AST/ASTImporter.cpp +++ b/clang/lib/AST/ASTImporter.cpp @@ -3646,6 +3646,54 @@ ExpectedDecl ASTNodeImporter::VisitIndirectFieldDecl(IndirectFieldDecl *D) { return ToIndirectField; } +/// Used as return type of getFriendCountAndPosition. +struct FriendCountAndPosition { + /// Number of similar looking friends. + unsigned int TotalCount; + /// Index of the specific FriendDecl. + unsigned int IndexOfDecl; +}; + +template <class T> +FriendCountAndPosition getFriendCountAndPosition( + const FriendDecl *FD, + std::function<T(const FriendDecl *)> GetCanTypeOrDecl) { + unsigned int FriendCount = 0; + llvm::Optional<unsigned int> FriendPosition; + const auto *RD = cast<CXXRecordDecl>(FD->getLexicalDeclContext()); + + T TypeOrDecl = GetCanTypeOrDecl(FD); + + for (const FriendDecl *FoundFriend : RD->friends()) { + if (FoundFriend == FD) { + FriendPosition = FriendCount; + ++FriendCount; + } else if (!FoundFriend->getFriendDecl() == !FD->getFriendDecl() && + GetCanTypeOrDecl(FoundFriend) == TypeOrDecl) { + ++FriendCount; + } + } + + assert(FriendPosition && "Friend decl not found in own parent."); + + return {FriendCount, *FriendPosition}; +} + +FriendCountAndPosition getFriendCountAndPosition(const FriendDecl *FD) { + if (FD->getFriendType()) + return getFriendCountAndPosition<QualType>(FD, [](const FriendDecl *F) { + if (TypeSourceInfo *TSI = F->getFriendType()) + return TSI->getType().getCanonicalType(); + llvm_unreachable("Wrong friend object type."); + }); + else + return getFriendCountAndPosition<Decl *>(FD, [](const FriendDecl *F) { + if (Decl *D = F->getFriendDecl()) + return D->getCanonicalDecl(); + llvm_unreachable("Wrong friend object type."); + }); +} + ExpectedDecl ASTNodeImporter::VisitFriendDecl(FriendDecl *D) { // Import the major distinguishing characteristics of a declaration. DeclContext *DC, *LexicalDC; @@ -3654,25 +3702,37 @@ ExpectedDecl ASTNodeImporter::VisitFriendDecl(FriendDecl *D) { // Determine whether we've already imported this decl. // FriendDecl is not a NamedDecl so we cannot use lookup. - auto *RD = cast<CXXRecordDecl>(DC); + // We try to maintain order and count of redundant friend declarations. + const auto *RD = cast<CXXRecordDecl>(DC); FriendDecl *ImportedFriend = RD->getFirstFriend(); + SmallVector<FriendDecl *, 2> ImportedEquivalentFriends; while (ImportedFriend) { + bool Match = false; if (D->getFriendDecl() && ImportedFriend->getFriendDecl()) { - if (IsStructuralMatch(D->getFriendDecl(), ImportedFriend->getFriendDecl(), - /*Complain=*/false)) - return Importer.MapImported(D, ImportedFriend); - + Match = + IsStructuralMatch(D->getFriendDecl(), ImportedFriend->getFriendDecl(), + /*Complain=*/false); } else if (D->getFriendType() && ImportedFriend->getFriendType()) { - if (Importer.IsStructurallyEquivalent( - D->getFriendType()->getType(), - ImportedFriend->getFriendType()->getType(), true)) - return Importer.MapImported(D, ImportedFriend); + Match = Importer.IsStructurallyEquivalent( + D->getFriendType()->getType(), + ImportedFriend->getFriendType()->getType(), /*Complain=*/false); } + if (Match) + ImportedEquivalentFriends.push_back(ImportedFriend); + ImportedFriend = ImportedFriend->getNextFriend(); } + FriendCountAndPosition CountAndPosition = getFriendCountAndPosition(D); + + assert(ImportedEquivalentFriends.size() <= CountAndPosition.TotalCount && + "Class with non-matching friends is imported, ODR check wrong?"); + if (ImportedEquivalentFriends.size() == CountAndPosition.TotalCount) + return Importer.MapImported( + D, ImportedEquivalentFriends[CountAndPosition.IndexOfDecl]); // Not found. Create it. + // The declarations will be put into order later by ImportDeclContext. FriendDecl::FriendUnion ToFU; if (NamedDecl *FriendD = D->getFriendDecl()) { NamedDecl *ToFriendD; diff --git a/clang/unittests/AST/ASTImporterTest.cpp b/clang/unittests/AST/ASTImporterTest.cpp index d6a5afeeb489..f92ea8de1651 100644 --- a/clang/unittests/AST/ASTImporterTest.cpp +++ b/clang/unittests/AST/ASTImporterTest.cpp @@ -3974,6 +3974,56 @@ TEST_P(ImportFriendClasses, ImportOfClassDefinitionAndFwdFriendShouldBeLinked) { EXPECT_EQ(ImportedFwd, ImportedDef->getPreviousDecl()); } +TEST_P(ImportFriendClasses, ImportOfRepeatedFriendType) { + const char *Code = + R"( + class Container { + friend class X; + friend class X; + }; + )"; + Decl *ToTu = getToTuDecl(Code, Lang_CXX03); + Decl *FromTu = getTuDecl(Code, Lang_CXX03, "from.cc"); + + auto *ToFriend1 = FirstDeclMatcher<FriendDecl>().match(ToTu, friendDecl()); + auto *ToFriend2 = LastDeclMatcher<FriendDecl>().match(ToTu, friendDecl()); + auto *FromFriend1 = + FirstDeclMatcher<FriendDecl>().match(FromTu, friendDecl()); + auto *FromFriend2 = LastDeclMatcher<FriendDecl>().match(FromTu, friendDecl()); + + FriendDecl *ToImportedFriend1 = Import(FromFriend1, Lang_CXX03); + FriendDecl *ToImportedFriend2 = Import(FromFriend2, Lang_CXX03); + + EXPECT_NE(ToImportedFriend1, ToImportedFriend2); + EXPECT_EQ(ToFriend1, ToImportedFriend1); + EXPECT_EQ(ToFriend2, ToImportedFriend2); +} + +TEST_P(ImportFriendClasses, ImportOfRepeatedFriendDecl) { + const char *Code = + R"( + class Container { + friend void f(); + friend void f(); + }; + )"; + Decl *ToTu = getToTuDecl(Code, Lang_CXX03); + Decl *FromTu = getTuDecl(Code, Lang_CXX03, "from.cc"); + + auto *ToFriend1 = FirstDeclMatcher<FriendDecl>().match(ToTu, friendDecl()); + auto *ToFriend2 = LastDeclMatcher<FriendDecl>().match(ToTu, friendDecl()); + auto *FromFriend1 = + FirstDeclMatcher<FriendDecl>().match(FromTu, friendDecl()); + auto *FromFriend2 = LastDeclMatcher<FriendDecl>().match(FromTu, friendDecl()); + + FriendDecl *ToImportedFriend1 = Import(FromFriend1, Lang_CXX03); + FriendDecl *ToImportedFriend2 = Import(FromFriend2, Lang_CXX03); + + EXPECT_NE(ToImportedFriend1, ToImportedFriend2); + EXPECT_EQ(ToFriend1, ToImportedFriend1); + EXPECT_EQ(ToFriend2, ToImportedFriend2); +} + TEST_P(ASTImporterOptionSpecificTestBase, FriendFunInClassTemplate) { auto *Code = R"( template <class T> diff --git a/clang/unittests/AST/StructuralEquivalenceTest.cpp b/clang/unittests/AST/StructuralEquivalenceTest.cpp index 08512d606279..2b5ce0fed51d 100644 --- a/clang/unittests/AST/StructuralEquivalenceTest.cpp +++ b/clang/unittests/AST/StructuralEquivalenceTest.cpp @@ -759,6 +759,27 @@ TEST_F(StructuralEquivalenceRecordTest, RecordsWithDifferentBody) { EXPECT_FALSE(testStructuralMatch(t)); } +TEST_F(StructuralEquivalenceRecordTest, SameFriendMultipleTimes) { + auto t = makeNamedDecls("struct foo { friend class X; };", + "struct foo { friend class X; friend class X; };", + Lang_CXX11); + EXPECT_FALSE(testStructuralMatch(t)); +} + +TEST_F(StructuralEquivalenceRecordTest, SameFriendsDifferentOrder) { + auto t = makeNamedDecls("struct foo { friend class X; friend class Y; };", + "struct foo { friend class Y; friend class X; };", + Lang_CXX11); + EXPECT_FALSE(testStructuralMatch(t)); +} + +TEST_F(StructuralEquivalenceRecordTest, SameFriendsSameOrder) { + auto t = makeNamedDecls("struct foo { friend class X; friend class Y; };", + "struct foo { friend class X; friend class Y; };", + Lang_CXX11); + EXPECT_TRUE(testStructuralMatch(t)); +} + struct StructuralEquivalenceLambdaTest : StructuralEquivalenceTest {}; TEST_F(StructuralEquivalenceLambdaTest, LambdaClassesWithDifferentMethods) { _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
