martong created this revision.
martong added reviewers: balazske, a_sidorin.
Herald added subscribers: cfe-commits, teemperor, gamesh411, Szelethus, dkrupp,
rnkovacs.
Herald added a reviewer: a.sidorin.
Herald added a reviewer: shafik.
Herald added a project: clang.
In the past we had to use DeclContext::makeDeclVisibleInContext to make
friend declarations available for subsequent lookup calls and this way
we could chain (redecl) the structurally equivalent decls.
By doing this we created an AST that improperly made declarations
visible in some contexts, so the AST was malformed.
Since we use the importer specific lookup this is no longer necessary,
because with that we can find every previous nodes.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D71020
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
@@ -245,6 +245,25 @@
return cast<RecordType>(ET->getNamedType().getTypePtr())->getDecl();
}
+static QualType getUnderlyingType(const TypedefType *TDT) {
+ QualType T = TDT->getDecl()->getUnderlyingType();
+
+ if (const auto *Inner = dyn_cast<TypedefType>(T.getTypePtr()))
+ return getUnderlyingType(Inner);
+
+ return T;
+}
+
+static const RecordDecl *getRecordDeclOfFriend(FriendDecl *FD) {
+ QualType Ty = FD->getFriendType()->getType();
+ if (auto *Inner = dyn_cast<TypedefType>(Ty.getTypePtr())) {
+ Ty = getUnderlyingType(Inner);
+ }
+ if (isa<ElaboratedType>(Ty))
+ Ty = cast<ElaboratedType>(Ty)->getNamedType();
+ return cast<RecordType>(Ty)->getDecl();
+}
+
struct ImportExpr : TestImportBase {};
struct ImportType : TestImportBase {};
struct ImportDecl : TestImportBase {};
@@ -2705,7 +2724,7 @@
EXPECT_FALSE(To0->isInIdentifierNamespace(Decl::IDNS_Ordinary));
}
-TEST_P(ImportFriendFunctions, DISABLED_LookupWithProtoAfter) {
+TEST_P(ImportFriendFunctions, LookupWithProtoAfter) {
auto FunctionPattern = functionDecl(hasName("f"));
auto ClassPattern = cxxRecordDecl(hasName("X"));
@@ -3776,6 +3795,44 @@
EXPECT_TRUE(MatchVerifier<Decl>{}.match(ToD, Pattern));
}
+TEST_P(ImportFriendClasses, UndeclaredFriendClassShouldNotBeVisible) {
+ Decl *FromTu = getTuDecl("class X { friend class Y; };", Lang_CXX, "from.cc");
+ auto *FromX = FirstDeclMatcher<CXXRecordDecl>().match(
+ FromTu, cxxRecordDecl(hasName("X")));
+ auto *FromFriend = FirstDeclMatcher<FriendDecl>().match(FromTu, friendDecl());
+ RecordDecl *FromRecordOfFriend =
+ const_cast<RecordDecl *>(getRecordDeclOfFriend(FromFriend));
+
+ ASSERT_EQ(FromRecordOfFriend->getDeclContext(), cast<DeclContext>(FromTu));
+ ASSERT_EQ(FromRecordOfFriend->getLexicalDeclContext(),
+ cast<DeclContext>(FromX));
+ ASSERT_FALSE(
+ FromRecordOfFriend->getDeclContext()->containsDecl(FromRecordOfFriend));
+ ASSERT_FALSE(FromRecordOfFriend->getLexicalDeclContext()->containsDecl(
+ FromRecordOfFriend));
+ ASSERT_FALSE(FromRecordOfFriend->getLookupParent()
+ ->lookup(FromRecordOfFriend->getDeclName())
+ .empty());
+
+ auto *ToX = Import(FromX, Lang_CXX);
+ ASSERT_TRUE(ToX);
+
+ Decl *ToTu = ToX->getTranslationUnitDecl();
+ auto *ToFriend = FirstDeclMatcher<FriendDecl>().match(ToTu, friendDecl());
+ RecordDecl *ToRecordOfFriend =
+ const_cast<RecordDecl *>(getRecordDeclOfFriend(ToFriend));
+
+ ASSERT_EQ(ToRecordOfFriend->getDeclContext(), cast<DeclContext>(ToTu));
+ ASSERT_EQ(ToRecordOfFriend->getLexicalDeclContext(), cast<DeclContext>(ToX));
+ EXPECT_FALSE(
+ ToRecordOfFriend->getDeclContext()->containsDecl(ToRecordOfFriend));
+ EXPECT_FALSE(ToRecordOfFriend->getLexicalDeclContext()->containsDecl(
+ ToRecordOfFriend));
+ EXPECT_FALSE(ToRecordOfFriend->getLookupParent()
+ ->lookup(ToRecordOfFriend->getDeclName())
+ .empty());
+}
+
TEST_P(ImportFriendClasses, ImportOfRecursiveFriendClassTemplate) {
Decl *FromTu = getTuDecl(
R"(
@@ -4475,11 +4532,6 @@
ASSERT_EQ(Res.size(), 0u);
}
-static const RecordDecl *getRecordDeclOfFriend(FriendDecl *FD) {
- QualType Ty = FD->getFriendType()->getType().getCanonicalType();
- return cast<RecordType>(Ty)->getDecl();
-}
-
TEST_P(ASTImporterLookupTableTest,
LookupFindsFwdFriendClassDeclWithElaboratedType) {
TranslationUnitDecl *ToTU = getToTuDecl(
Index: clang/lib/AST/ASTImporter.cpp
===================================================================
--- clang/lib/AST/ASTImporter.cpp
+++ clang/lib/AST/ASTImporter.cpp
@@ -298,6 +298,46 @@
return nullptr;
}
+ void addDeclToContexts(Decl *FromD, Decl *ToD) {
+ if (Importer.isMinimalImport()) {
+ // In minimal import case the decl must be added even if it is not
+ // contained in original context, for LLDB compatibility.
+ // FIXME: Check if a better solution is possible.
+ if (!FromD->getDescribedTemplate() &&
+ FromD->getFriendObjectKind() == Decl::FOK_None)
+ ToD->getLexicalDeclContext()->addDeclInternal(ToD);
+ return;
+ }
+
+ DeclContext *FromDC = FromD->getDeclContext();
+ DeclContext *FromLexicalDC = FromD->getLexicalDeclContext();
+ DeclContext *ToDC = ToD->getDeclContext();
+ DeclContext *ToLexicalDC = ToD->getLexicalDeclContext();
+
+ bool Visible = false;
+ if (FromDC->containsDeclAndLoad(FromD)) {
+ ToDC->addDeclInternal(ToD);
+ Visible = true;
+ }
+ if (ToDC != ToLexicalDC && FromLexicalDC->containsDeclAndLoad(FromD)) {
+ ToLexicalDC->addDeclInternal(ToD);
+ Visible = true;
+ }
+
+ // If the Decl was added to any context, it was made already visible.
+ // Otherwise it is still possible that it should be visible.
+ if (!Visible) {
+ if (auto *FromNamed = dyn_cast<NamedDecl>(FromD)) {
+ auto *ToNamed = cast<NamedDecl>(ToD);
+ DeclContextLookupResult FromLookup =
+ FromDC->lookup(FromNamed->getDeclName());
+ if (std::find(FromLookup.begin(), FromLookup.end(), FromNamed) !=
+ FromLookup.end())
+ ToDC->makeDeclVisibleInContext(ToNamed);
+ }
+ }
+ }
+
public:
explicit ASTNodeImporter(ASTImporter &Importer) : Importer(Importer) {}
@@ -2720,11 +2760,7 @@
D2 = D2CXX;
D2->setAccess(D->getAccess());
D2->setLexicalDeclContext(LexicalDC);
- if (!DCXX->getDescribedClassTemplate() || DCXX->isImplicit())
- LexicalDC->addDeclInternal(D2);
-
- if (LexicalDC != DC && D->isInIdentifierNamespace(Decl::IDNS_TagFriend))
- DC->makeDeclVisibleInContext(D2);
+ addDeclToContexts(D, D2);
if (ClassTemplateDecl *FromDescribed =
DCXX->getDescribedClassTemplate()) {
@@ -2790,7 +2826,7 @@
Name.getAsIdentifierInfo(), PrevDecl))
return D2;
D2->setLexicalDeclContext(LexicalDC);
- LexicalDC->addDeclInternal(D2);
+ addDeclToContexts(D, D2);
}
if (auto QualifierLocOrErr = import(D->getQualifierLoc()))
@@ -3310,23 +3346,7 @@
if (Error Err = ImportTemplateInformation(D, ToFunction))
return std::move(Err);
- bool IsFriend = D->isInIdentifierNamespace(Decl::IDNS_OrdinaryFriend);
-
- // TODO Can we generalize this approach to other AST nodes as well?
- if (D->getDeclContext()->containsDeclAndLoad(D))
- DC->addDeclInternal(ToFunction);
- if (DC != LexicalDC && D->getLexicalDeclContext()->containsDeclAndLoad(D))
- LexicalDC->addDeclInternal(ToFunction);
-
- // Friend declaration's lexical context is the befriending class, but the
- // semantic context is the enclosing scope of the befriending class.
- // We want the friend functions to be found in the semantic context by lookup.
- // FIXME should we handle this generically in VisitFriendDecl?
- // In Other cases when LexicalDC != DC we don't want it to be added,
- // e.g out-of-class definitions like void B::f() {} .
- if (LexicalDC != DC && IsFriend) {
- DC->makeDeclVisibleInContext(ToFunction);
- }
+ addDeclToContexts(D, ToFunction);
if (auto *FromCXXMethod = dyn_cast<CXXMethodDecl>(D))
if (Error Err = ImportOverriddenMethods(cast<CXXMethodDecl>(ToFunction),
@@ -3774,10 +3794,7 @@
if (D->isConstexpr())
ToVar->setConstexpr(true);
- if (D->getDeclContext()->containsDeclAndLoad(D))
- DC->addDeclInternal(ToVar);
- if (DC != LexicalDC && D->getLexicalDeclContext()->containsDeclAndLoad(D))
- LexicalDC->addDeclInternal(ToVar);
+ addDeclToContexts(D, ToVar);
// Import the rest of the chain. I.e. import all subsequent declarations.
for (++RedeclIt; RedeclIt != Redecls.end(); ++RedeclIt) {
@@ -5049,7 +5066,6 @@
}
ExpectedDecl ASTNodeImporter::VisitClassTemplateDecl(ClassTemplateDecl *D) {
- bool IsFriend = D->getFriendObjectKind() != Decl::FOK_None;
// Import the major distinguishing characteristics of this class template.
DeclContext *DC, *LexicalDC;
@@ -5126,10 +5142,7 @@
D2->setAccess(D->getAccess());
D2->setLexicalDeclContext(LexicalDC);
- if (D->getDeclContext()->containsDeclAndLoad(D))
- DC->addDeclInternal(D2);
- if (DC != LexicalDC && D->getLexicalDeclContext()->containsDeclAndLoad(D))
- LexicalDC->addDeclInternal(D2);
+ addDeclToContexts(D, D2);
if (FoundByLookup) {
auto *Recent =
@@ -5155,9 +5168,6 @@
D2->setPreviousDecl(Recent);
}
- if (LexicalDC != DC && IsFriend)
- DC->makeDeclVisibleInContext(D2);
-
if (FromTemplated->isCompleteDefinition() &&
!ToTemplated->isCompleteDefinition()) {
// FIXME: Import definition!
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
