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( _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits