Skip to content

Commit

Permalink
[clang][ASTImporter] Fix partially import of repeated friend templates
Browse files Browse the repository at this point in the history 
Pointer comparison is not reliable in this case. Use ASTStructuralEquivalence
to compute FriendCountAndPosition.

Reviewed By: balazske

Differential Revision: https://reviews.llvm.org/D157684
  • Loading branch information
@danix800
danix800 committed Aug 24, 2023
1 parent 935ba83 commit 1654634
Show file tree
Hide file tree
Showing 2 changed files with 68 additions and 69 deletions.
66 changes: 25 additions & 41 deletions clang/lib/AST/ASTImporter.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4079,22 +4079,34 @@ struct FriendCountAndPosition {
unsigned int IndexOfDecl;
};

template <class T>
static FriendCountAndPosition getFriendCountAndPosition(
const FriendDecl *FD,
llvm::function_ref<T(const FriendDecl *)> GetCanTypeOrDecl) {
static bool IsEquivalentFriend(ASTImporter &Importer, FriendDecl *FD1,
FriendDecl *FD2) {
if ((!FD1->getFriendType()) != (!FD2->getFriendType()))
return false;

if (const TypeSourceInfo *TSI = FD1->getFriendType())
return Importer.IsStructurallyEquivalent(
TSI->getType(), FD2->getFriendType()->getType(), /*Complain=*/false);

ASTImporter::NonEquivalentDeclSet NonEquivalentDecls;
StructuralEquivalenceContext Ctx(
FD1->getASTContext(), FD2->getASTContext(), NonEquivalentDecls,
StructuralEquivalenceKind::Default,
/* StrictTypeSpelling = */ false, /* Complain = */ false);
return Ctx.IsEquivalent(FD1, FD2);
}

static FriendCountAndPosition getFriendCountAndPosition(ASTImporter &Importer,
FriendDecl *FD) {
unsigned int FriendCount = 0;
std::optional<unsigned int> FriendPosition;
const auto *RD = cast<CXXRecordDecl>(FD->getLexicalDeclContext());

T TypeOrDecl = GetCanTypeOrDecl(FD);

for (const FriendDecl *FoundFriend : RD->friends()) {
for (FriendDecl *FoundFriend : RD->friends()) {
if (FoundFriend == FD) {
FriendPosition = FriendCount;
++FriendCount;
} else if (!FoundFriend->getFriendDecl() == !FD->getFriendDecl() &&
GetCanTypeOrDecl(FoundFriend) == TypeOrDecl) {
} else if (IsEquivalentFriend(Importer, FD, FoundFriend)) {
++FriendCount;
}
}
Expand All @@ -4104,21 +4116,6 @@ static FriendCountAndPosition getFriendCountAndPosition(
return {FriendCount, *FriendPosition};
}

static 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;
Expand All @@ -4129,26 +4126,13 @@ ExpectedDecl ASTNodeImporter::VisitFriendDecl(FriendDecl *D) {
// FriendDecl is not a NamedDecl so we cannot use lookup.
// 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()) {
Match =
IsStructuralMatch(D->getFriendDecl(), ImportedFriend->getFriendDecl(),
/*Complain=*/false);
} else if (D->getFriendType() && ImportedFriend->getFriendType()) {
Match = Importer.IsStructurallyEquivalent(
D->getFriendType()->getType(),
ImportedFriend->getFriendType()->getType(), /*Complain=*/false);
}
if (Match)
for (FriendDecl *ImportedFriend : RD->friends())
if (IsEquivalentFriend(Importer, D, ImportedFriend))
ImportedEquivalentFriends.push_back(ImportedFriend);

ImportedFriend = ImportedFriend->getNextFriend();
}
FriendCountAndPosition CountAndPosition = getFriendCountAndPosition(D);
FriendCountAndPosition CountAndPosition =
getFriendCountAndPosition(Importer, D);

assert(ImportedEquivalentFriends.size() <= CountAndPosition.TotalCount &&
"Class with non-matching friends is imported, ODR check wrong?");
Expand Down
71 changes: 43 additions & 28 deletions clang/unittests/AST/ASTImporterTest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4054,6 +4054,25 @@ struct ImportFriendClasses : ASTImporterOptionSpecificTestBase {
->lookup(ToRecordOfFriend->getDeclName())
.empty());
}

void testRepeatedFriendImport(const char *Code) {
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, ImportOfFriendRecordDoesNotMergeDefinition) {
Expand Down Expand Up @@ -4395,21 +4414,7 @@ TEST_P(ImportFriendClasses, ImportOfRepeatedFriendType) {
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);
testRepeatedFriendImport(Code);
}

TEST_P(ImportFriendClasses, ImportOfRepeatedFriendDecl) {
Expand All @@ -4420,21 +4425,31 @@ TEST_P(ImportFriendClasses, ImportOfRepeatedFriendDecl) {
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());
testRepeatedFriendImport(Code);
}

FriendDecl *ToImportedFriend1 = Import(FromFriend1, Lang_CXX03);
FriendDecl *ToImportedFriend2 = Import(FromFriend2, Lang_CXX03);
TEST_P(ImportFriendClasses, ImportOfRepeatedFriendFunctionTemplateDecl) {
const char *Code =
R"(
template <class T>
class Container {
template <class U> friend void m();
template <class U> friend void m();
};
)";
testRepeatedFriendImport(Code);
}

EXPECT_NE(ToImportedFriend1, ToImportedFriend2);
EXPECT_EQ(ToFriend1, ToImportedFriend1);
EXPECT_EQ(ToFriend2, ToImportedFriend2);
TEST_P(ImportFriendClasses, ImportOfRepeatedFriendClassTemplateDecl) {
const char *Code =
R"(
template <class T>
class Container {
template <class U> friend class X;
template <class U> friend class X;
};
)";
testRepeatedFriendImport(Code);
}

TEST_P(ASTImporterOptionSpecificTestBase, FriendFunInClassTemplate) {
Expand Down

0 comments on commit 1654634

Please sign in to comment.