Author: Gabor Marton Date: 2019-12-18T11:43:46+01:00 New Revision: bc5b7e21e32b23603f4d6148adeb88cd34dd287e
URL: https://github.com/llvm/llvm-project/commit/bc5b7e21e32b23603f4d6148adeb88cd34dd287e DIFF: https://github.com/llvm/llvm-project/commit/bc5b7e21e32b23603f4d6148adeb88cd34dd287e.diff LOG: recommit: [ASTImporter] Friend class decl should not be visible in its context Summary: 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. Reviewers: balazske, a_sidorin, a.sidorin, shafik Subscribers: rnkovacs, dkrupp, Szelethus, gamesh411, teemperor, cfe-commits Tags: #clang Differential Revision: https://reviews.llvm.org/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 414092f33c47..b75a689ec275 100644 --- a/clang/lib/AST/ASTImporter.cpp +++ b/clang/lib/AST/ASTImporter.cpp @@ -298,6 +298,48 @@ 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()); + for (NamedDecl *ND : FromLookup) + if (ND == FromNamed) { + ToDC->makeDeclVisibleInContext(ToNamed); + break; + } + } + } + } + public: explicit ASTNodeImporter(ASTImporter &Importer) : Importer(Importer) {} @@ -2737,11 +2779,7 @@ ExpectedDecl ASTNodeImporter::VisitRecordDecl(RecordDecl *D) { 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()) { @@ -2807,7 +2845,7 @@ ExpectedDecl ASTNodeImporter::VisitRecordDecl(RecordDecl *D) { Name.getAsIdentifierInfo(), PrevDecl)) return D2; D2->setLexicalDeclContext(LexicalDC); - LexicalDC->addDeclInternal(D2); + addDeclToContexts(D, D2); } if (auto BraceRangeOrErr = import(D->getBraceRange())) @@ -3386,23 +3424,7 @@ ExpectedDecl ASTNodeImporter::VisitFunctionDecl(FunctionDecl *D) { 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), @@ -3850,10 +3872,7 @@ ExpectedDecl ASTNodeImporter::VisitVarDecl(VarDecl *D) { 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) { @@ -5133,7 +5152,6 @@ 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; @@ -5210,10 +5228,7 @@ ExpectedDecl ASTNodeImporter::VisitClassTemplateDecl(ClassTemplateDecl *D) { 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 = @@ -5239,9 +5254,6 @@ 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 6652111fd48e..3e8f804374f4 100644 --- a/clang/unittests/AST/ASTImporterTest.cpp +++ b/clang/unittests/AST/ASTImporterTest.cpp @@ -247,6 +247,11 @@ 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 {}; @@ -2707,7 +2712,7 @@ TEST_P(ImportFriendFunctions, Lookup) { 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")); @@ -3778,6 +3783,44 @@ 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"( @@ -4477,11 +4520,6 @@ 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
