[clang] 85f5d12 - [ASTImporter] Corrected import of repeated friend declarations.
Balázs Kéri via cfe-commits
cfe-commits at lists.llvm.org
Tue Jul 7 07:22:12 PDT 2020
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) {
More information about the cfe-commits
mailing list