martong created this revision.
martong added reviewers: a_sidorin, shafik.
Herald added subscribers: cfe-commits, teemperor, gamesh411, Szelethus, dkrupp,
rnkovacs.
Herald added a reviewer: a.sidorin.
Herald added a project: clang.
Currently, we do not check the scope of friend classes. This can result in some
cases with missing nodes in the imported AST. E.g.:
class Container {
friend class X; // A friend class ::X
class X{ };
friend class X; // Friend class Container::X
};
... here either `::X` or `Container::X` is missing after importing `Container`.
This patch fixes the problem.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D75922
Files:
clang/lib/AST/ASTImporter.cpp
clang/unittests/AST/ASTImporterTest.cpp
Index: clang/unittests/AST/ASTImporterTest.cpp
===================================================================
--- clang/unittests/AST/ASTImporterTest.cpp
+++ clang/unittests/AST/ASTImporterTest.cpp
@@ -4023,6 +4023,35 @@
EXPECT_EQ(ImportedFoo, ToFoo);
}
+TEST_P(ImportFriendClasses, ImportOfInlineFriendClassAfterFwdWithSameName) {
+ auto Code =
+ R"(
+ class Container {
+ friend class X; // A friend class ::X
+ class X{ };
+ friend class X; // Friend class Container::X
+ };
+ )";
+ Decl *FromTu = getTuDecl(Code, Lang_CXX, "from.cc");
+
+ auto *To = Import(FirstDeclMatcher<CXXRecordDecl>().match(
+ FromTu, cxxRecordDecl(hasName("Container"))),
+ Lang_CXX);
+ ASSERT_TRUE(To);
+
+ Decl *ToTu = To->getTranslationUnitDecl();
+ auto *ToFriend1 = FirstDeclMatcher<FriendDecl>().match(ToTu, friendDecl());
+ auto *ToFriend2 = LastDeclMatcher<FriendDecl>().match(ToTu, friendDecl());
+ auto *ToX = FirstDeclMatcher<RecordDecl>().match(
+ ToTu, cxxRecordDecl(hasName("X"), unless(isImplicit())));
+
+ EXPECT_NE(ToFriend1, ToFriend2);
+ const RecordDecl *RecordOfFriend1 = getRecordDeclOfFriend(ToFriend1);
+ const RecordDecl *RecordOfFriend2 = getRecordDeclOfFriend(ToFriend2);
+ EXPECT_NE(RecordOfFriend1, ToX);
+ EXPECT_EQ(RecordOfFriend2, ToX);
+}
+
struct DeclContextTest : ASTImporterOptionSpecificTestBase {};
TEST_P(DeclContextTest, removeDeclOfClassTemplateSpecialization) {
Index: clang/lib/AST/ASTImporter.cpp
===================================================================
--- clang/lib/AST/ASTImporter.cpp
+++ clang/lib/AST/ASTImporter.cpp
@@ -3632,6 +3632,30 @@
return ToIndirectField;
}
+// Returns the DeclContext of the underlying friend declaration/type. If that
+// is a dependent type then the returned optional does not have a value.
+static Optional<DeclContext *> getDCOfUnderlyingDecl(FriendDecl *FrD) {
+ if (NamedDecl *ND = FrD->getFriendDecl())
+ return ND->getDeclContext();
+ if (FrD->getFriendType()) {
+ QualType Ty = FrD->getFriendType()->getType();
+ if (isa<ElaboratedType>(Ty))
+ Ty = cast<ElaboratedType>(Ty)->getNamedType();
+ if (!Ty->isDependentType()) {
+ if (const auto *RTy = dyn_cast<RecordType>(Ty))
+ return RTy->getAsCXXRecordDecl()->getDeclContext();
+ else if (const auto *SpecTy = dyn_cast<TemplateSpecializationType>(Ty))
+ return SpecTy->getAsCXXRecordDecl()->getDeclContext();
+ else if (const auto TypedefTy = dyn_cast<TypedefType>(Ty))
+ return TypedefTy->getDecl()->getDeclContext();
+ else
+ llvm_unreachable("Unhandled type of friend");
+ }
+ }
+ // DependentType
+ return Optional<DeclContext *>();
+}
+
ExpectedDecl ASTNodeImporter::VisitFriendDecl(FriendDecl *D) {
// Import the major distinguishing characteristics of a declaration.
DeclContext *DC, *LexicalDC;
@@ -3641,9 +3665,25 @@
// Determine whether we've already imported this decl.
// FriendDecl is not a NamedDecl so we cannot use lookup.
auto *RD = cast<CXXRecordDecl>(DC);
- FriendDecl *ImportedFriend = RD->getFirstFriend();
- while (ImportedFriend) {
+ for (FriendDecl *ImportedFriend = RD->getFirstFriend(); ImportedFriend;
+ ImportedFriend = ImportedFriend->getNextFriend()) {
+
+ // Compare the semantic DeclContext of the underlying declarations of the
+ // existing and the to be imported friend.
+ // Normally, lookup ensures this, but with friends we cannot use the lookup.
+ Optional<DeclContext *> ImportedFriendDC =
+ getDCOfUnderlyingDecl(ImportedFriend);
+ Optional<DeclContext *> FromFriendDC = getDCOfUnderlyingDecl(D);
+ if (FromFriendDC) { // The underlying friend type is not dependent.
+ ExpectedDecl FriendDCDeclOrErr = import(cast<Decl>(*FromFriendDC));
+ if (!FriendDCDeclOrErr)
+ return FriendDCDeclOrErr.takeError();
+ DeclContext *FriendDC = cast<DeclContext>(*FriendDCDeclOrErr);
+ if (ImportedFriendDC != FriendDC)
+ continue;
+ }
+
if (D->getFriendDecl() && ImportedFriend->getFriendDecl()) {
if (IsStructuralMatch(D->getFriendDecl(), ImportedFriend->getFriendDecl(),
/*Complain=*/false))
@@ -3655,7 +3695,6 @@
ImportedFriend->getFriendType()->getType(), true))
return Importer.MapImported(D, ImportedFriend);
}
- ImportedFriend = ImportedFriend->getNextFriend();
}
// Not found. Create it.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
