balazske updated this revision to Diff 249317. balazske marked an inline comment as done. balazske added a comment.
Removed code repetition. Using `const char *` instead of `auto`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75740/new/ https://reviews.llvm.org/D75740 Files: clang/lib/AST/ASTImporter.cpp clang/unittests/AST/ASTImporterTest.cpp clang/unittests/AST/StructuralEquivalenceTest.cpp
Index: clang/unittests/AST/StructuralEquivalenceTest.cpp =================================================================== --- clang/unittests/AST/StructuralEquivalenceTest.cpp +++ clang/unittests/AST/StructuralEquivalenceTest.cpp @@ -797,6 +797,27 @@ 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_CXX); + 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_CXX); + 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_CXX); + EXPECT_TRUE(testStructuralMatch(t)); +} + struct StructuralEquivalenceLambdaTest : StructuralEquivalenceTest {}; TEST_F(StructuralEquivalenceLambdaTest, LambdaClassesWithDifferentMethods) { Index: clang/unittests/AST/ASTImporterTest.cpp =================================================================== --- clang/unittests/AST/ASTImporterTest.cpp +++ clang/unittests/AST/ASTImporterTest.cpp @@ -4005,6 +4005,56 @@ 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_CXX); + Decl *FromTu = getTuDecl(Code, Lang_CXX, "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_CXX); + FriendDecl *ToImportedFriend2 = Import(FromFriend2, Lang_CXX); + + 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_CXX); + Decl *FromTu = getTuDecl(Code, Lang_CXX, "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_CXX); + FriendDecl *ToImportedFriend2 = Import(FromFriend2, Lang_CXX); + + EXPECT_NE(ToImportedFriend1, ToImportedFriend2); + EXPECT_EQ(ToFriend1, ToImportedFriend1); + EXPECT_EQ(ToFriend2, ToImportedFriend2); +} + TEST_P(ASTImporterOptionSpecificTestBase, FriendFunInClassTemplate) { auto *Code = R"( template <class T> Index: clang/lib/AST/ASTImporter.cpp =================================================================== --- clang/lib/AST/ASTImporter.cpp +++ clang/lib/AST/ASTImporter.cpp @@ -3632,6 +3632,54 @@ 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; @@ -3640,25 +3688,37 @@ // 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;
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits