[clang] 55c55f8 - Revert "[ASTImporter] Friend class decl should not be visible in its context"
Nico Weber via cfe-commits
cfe-commits at lists.llvm.org
Tue Dec 17 10:44:28 PST 2019
Author: Nico Weber
Date: 2019-12-17T13:43:50-05:00
New Revision: 55c55f8eb86ba3e77fe73ccdf7c861e2c2c7ae92
URL: https://github.com/llvm/llvm-project/commit/55c55f8eb86ba3e77fe73ccdf7c861e2c2c7ae92
DIFF: https://github.com/llvm/llvm-project/commit/55c55f8eb86ba3e77fe73ccdf7c861e2c2c7ae92.diff
LOG: Revert "[ASTImporter] Friend class decl should not be visible in its context"
This reverts commit 4becf68c6f17fe143539ceac954b21175914e1c1.
Breaks building on Windows, see comments on D71020
Added:
Modified:
clang/lib/AST/ASTImporter.cpp
clang/unittests/AST/ASTImporterTest.cpp
Removed:
################################################################################
diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp
index 336ee88f994a..414092f33c47 100644
--- a/clang/lib/AST/ASTImporter.cpp
+++ b/clang/lib/AST/ASTImporter.cpp
@@ -298,45 +298,6 @@ namespace clang {
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 (llvm::is_contained(FromLookup, FromNamed))
- ToDC->makeDeclVisibleInContext(ToNamed);
- }
- }
- }
-
public:
explicit ASTNodeImporter(ASTImporter &Importer) : Importer(Importer) {}
@@ -2776,7 +2737,11 @@ ExpectedDecl ASTNodeImporter::VisitRecordDecl(RecordDecl *D) {
D2 = D2CXX;
D2->setAccess(D->getAccess());
D2->setLexicalDeclContext(LexicalDC);
- addDeclToContexts(D, D2);
+ if (!DCXX->getDescribedClassTemplate() || DCXX->isImplicit())
+ LexicalDC->addDeclInternal(D2);
+
+ if (LexicalDC != DC && D->isInIdentifierNamespace(Decl::IDNS_TagFriend))
+ DC->makeDeclVisibleInContext(D2);
if (ClassTemplateDecl *FromDescribed =
DCXX->getDescribedClassTemplate()) {
@@ -2842,7 +2807,7 @@ ExpectedDecl ASTNodeImporter::VisitRecordDecl(RecordDecl *D) {
Name.getAsIdentifierInfo(), PrevDecl))
return D2;
D2->setLexicalDeclContext(LexicalDC);
- addDeclToContexts(D, D2);
+ LexicalDC->addDeclInternal(D2);
}
if (auto BraceRangeOrErr = import(D->getBraceRange()))
@@ -3421,7 +3386,23 @@ ExpectedDecl ASTNodeImporter::VisitFunctionDecl(FunctionDecl *D) {
if (Error Err = ImportTemplateInformation(D, ToFunction))
return std::move(Err);
- addDeclToContexts(D, ToFunction);
+ 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);
+ }
if (auto *FromCXXMethod = dyn_cast<CXXMethodDecl>(D))
if (Error Err = ImportOverriddenMethods(cast<CXXMethodDecl>(ToFunction),
@@ -3869,7 +3850,10 @@ ExpectedDecl ASTNodeImporter::VisitVarDecl(VarDecl *D) {
if (D->isConstexpr())
ToVar->setConstexpr(true);
- addDeclToContexts(D, ToVar);
+ if (D->getDeclContext()->containsDeclAndLoad(D))
+ DC->addDeclInternal(ToVar);
+ if (DC != LexicalDC && D->getLexicalDeclContext()->containsDeclAndLoad(D))
+ LexicalDC->addDeclInternal(ToVar);
// Import the rest of the chain. I.e. import all subsequent declarations.
for (++RedeclIt; RedeclIt != Redecls.end(); ++RedeclIt) {
@@ -5149,6 +5133,7 @@ template <typename T> static auto getTemplateDefinition(T *D) -> T * {
}
ExpectedDecl ASTNodeImporter::VisitClassTemplateDecl(ClassTemplateDecl *D) {
+ bool IsFriend = D->getFriendObjectKind() != Decl::FOK_None;
// Import the major distinguishing characteristics of this class template.
DeclContext *DC, *LexicalDC;
@@ -5225,7 +5210,10 @@ ExpectedDecl ASTNodeImporter::VisitClassTemplateDecl(ClassTemplateDecl *D) {
D2->setAccess(D->getAccess());
D2->setLexicalDeclContext(LexicalDC);
- addDeclToContexts(D, D2);
+ if (D->getDeclContext()->containsDeclAndLoad(D))
+ DC->addDeclInternal(D2);
+ if (DC != LexicalDC && D->getLexicalDeclContext()->containsDeclAndLoad(D))
+ LexicalDC->addDeclInternal(D2);
if (FoundByLookup) {
auto *Recent =
@@ -5251,6 +5239,9 @@ ExpectedDecl ASTNodeImporter::VisitClassTemplateDecl(ClassTemplateDecl *D) {
D2->setPreviousDecl(Recent);
}
+ if (LexicalDC != DC && IsFriend)
+ DC->makeDeclVisibleInContext(D2);
+
if (FromTemplated->isCompleteDefinition() &&
!ToTemplated->isCompleteDefinition()) {
// FIXME: Import definition!
diff --git a/clang/unittests/AST/ASTImporterTest.cpp b/clang/unittests/AST/ASTImporterTest.cpp
index 3e8f804374f4..6652111fd48e 100644
--- a/clang/unittests/AST/ASTImporterTest.cpp
+++ b/clang/unittests/AST/ASTImporterTest.cpp
@@ -247,11 +247,6 @@ template <typename T> RecordDecl *getRecordDecl(T *D) {
return cast<RecordType>(ET->getNamedType().getTypePtr())->getDecl();
}
-static const RecordDecl *getRecordDeclOfFriend(FriendDecl *FD) {
- QualType Ty = FD->getFriendType()->getType().getCanonicalType();
- return cast<RecordType>(Ty)->getDecl();
-}
-
struct ImportExpr : TestImportBase {};
struct ImportType : TestImportBase {};
struct ImportDecl : TestImportBase {};
@@ -2712,7 +2707,7 @@ TEST_P(ImportFriendFunctions, Lookup) {
EXPECT_FALSE(To0->isInIdentifierNamespace(Decl::IDNS_Ordinary));
}
-TEST_P(ImportFriendFunctions, LookupWithProtoAfter) {
+TEST_P(ImportFriendFunctions, DISABLED_LookupWithProtoAfter) {
auto FunctionPattern = functionDecl(hasName("f"));
auto ClassPattern = cxxRecordDecl(hasName("X"));
@@ -3783,44 +3778,6 @@ TEST_P(ImportFriendClasses, ImportOfRecursiveFriendClass) {
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"(
@@ -4520,6 +4477,11 @@ TEST_P(ASTImporterLookupTableTest, LookupDeclNamesFromDifferentTUs) {
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(
More information about the cfe-commits
mailing list