Author: Balázs Kéri Date: 2025-01-13T09:46:45+01:00 New Revision: b270525f730be6e7196667925f5a9bfa153262e9
URL: https://github.com/llvm/llvm-project/commit/b270525f730be6e7196667925f5a9bfa153262e9 DIFF: https://github.com/llvm/llvm-project/commit/b270525f730be6e7196667925f5a9bfa153262e9.diff LOG: [clang][ASTImporter] Not using primary context in lookup table (#118466) `ASTImporterLookupTable` did use the `getPrimaryContext` function to get the declaration context of the inserted items. This is problematic because the primary context can change during import of AST items, most likely if a definition of a previously not defined class is imported. (For any record the primary context is the definition if there is one.) The use of primary context is really not important, only for namespaces because these can be re-opened and lookup in one namespace block is not enough. This special search is now moved into ASTImporter instead of relying on the lookup table. Added: Modified: clang/lib/AST/ASTImporter.cpp clang/lib/AST/ASTImporterLookupTable.cpp clang/unittests/AST/ASTImporterTest.cpp Removed: ################################################################################ diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp index 26d33b0d94795f..dec4c7221bc776 100644 --- a/clang/lib/AST/ASTImporter.cpp +++ b/clang/lib/AST/ASTImporter.cpp @@ -3165,6 +3165,7 @@ ExpectedDecl ASTNodeImporter::VisitRecordDecl(RecordDecl *D) { if (Error Err = ImportImplicitMethods(DCXX, FoundCXX)) return std::move(Err); } + // FIXME: We can return FoundDef here. } PrevDecl = FoundRecord->getMostRecentDecl(); break; @@ -9064,9 +9065,26 @@ ASTImporter::findDeclsInToCtx(DeclContext *DC, DeclarationName Name) { // We can diagnose this only if we search in the redecl context. DeclContext *ReDC = DC->getRedeclContext(); if (SharedState->getLookupTable()) { - ASTImporterLookupTable::LookupResult LookupResult = - SharedState->getLookupTable()->lookup(ReDC, Name); - return FoundDeclsTy(LookupResult.begin(), LookupResult.end()); + if (ReDC->isNamespace()) { + // Namespaces can be reopened. + // Lookup table does not handle this, we must search here in all linked + // namespaces. + FoundDeclsTy Result; + SmallVector<Decl *, 2> NSChain = + getCanonicalForwardRedeclChain<NamespaceDecl>( + dyn_cast<NamespaceDecl>(ReDC)); + for (auto *D : NSChain) { + ASTImporterLookupTable::LookupResult LookupResult = + SharedState->getLookupTable()->lookup(dyn_cast<NamespaceDecl>(D), + Name); + Result.append(LookupResult.begin(), LookupResult.end()); + } + return Result; + } else { + ASTImporterLookupTable::LookupResult LookupResult = + SharedState->getLookupTable()->lookup(ReDC, Name); + return FoundDeclsTy(LookupResult.begin(), LookupResult.end()); + } } else { DeclContext::lookup_result NoloadLookupResult = ReDC->noload_lookup(Name); FoundDeclsTy Result(NoloadLookupResult.begin(), NoloadLookupResult.end()); diff --git a/clang/lib/AST/ASTImporterLookupTable.cpp b/clang/lib/AST/ASTImporterLookupTable.cpp index 07d39dcee2583a..4ed3198d7ea62b 100644 --- a/clang/lib/AST/ASTImporterLookupTable.cpp +++ b/clang/lib/AST/ASTImporterLookupTable.cpp @@ -115,8 +115,9 @@ void ASTImporterLookupTable::remove(DeclContext *DC, NamedDecl *ND) { #ifndef NDEBUG if (!EraseResult) { std::string Message = - llvm::formatv("Trying to remove not contained Decl '{0}' of type {1}", - Name.getAsString(), DC->getDeclKindName()) + llvm::formatv( + "Trying to remove not contained Decl '{0}' of type {1} from a {2}", + Name.getAsString(), ND->getDeclKindName(), DC->getDeclKindName()) .str(); llvm_unreachable(Message.c_str()); } @@ -125,18 +126,18 @@ void ASTImporterLookupTable::remove(DeclContext *DC, NamedDecl *ND) { void ASTImporterLookupTable::add(NamedDecl *ND) { assert(ND); - DeclContext *DC = ND->getDeclContext()->getPrimaryContext(); + DeclContext *DC = ND->getDeclContext(); add(DC, ND); - DeclContext *ReDC = DC->getRedeclContext()->getPrimaryContext(); + DeclContext *ReDC = DC->getRedeclContext(); if (DC != ReDC) add(ReDC, ND); } void ASTImporterLookupTable::remove(NamedDecl *ND) { assert(ND); - DeclContext *DC = ND->getDeclContext()->getPrimaryContext(); + DeclContext *DC = ND->getDeclContext(); remove(DC, ND); - DeclContext *ReDC = DC->getRedeclContext()->getPrimaryContext(); + DeclContext *ReDC = DC->getRedeclContext(); if (DC != ReDC) remove(ReDC, ND); } @@ -161,7 +162,7 @@ void ASTImporterLookupTable::updateForced(NamedDecl *ND, DeclContext *OldDC) { ASTImporterLookupTable::LookupResult ASTImporterLookupTable::lookup(DeclContext *DC, DeclarationName Name) const { - auto DCI = LookupTable.find(DC->getPrimaryContext()); + auto DCI = LookupTable.find(DC); if (DCI == LookupTable.end()) return {}; @@ -178,7 +179,7 @@ bool ASTImporterLookupTable::contains(DeclContext *DC, NamedDecl *ND) const { } void ASTImporterLookupTable::dump(DeclContext *DC) const { - auto DCI = LookupTable.find(DC->getPrimaryContext()); + auto DCI = LookupTable.find(DC); if (DCI == LookupTable.end()) llvm::errs() << "empty\n"; const auto &FoundNameMap = DCI->second; @@ -196,8 +197,7 @@ void ASTImporterLookupTable::dump(DeclContext *DC) const { void ASTImporterLookupTable::dump() const { for (const auto &Entry : LookupTable) { DeclContext *DC = Entry.first; - StringRef Primary = DC->getPrimaryContext() ? " primary" : ""; - llvm::errs() << "== DC:" << cast<Decl>(DC) << Primary << "\n"; + llvm::errs() << "== DC:" << cast<Decl>(DC) << "\n"; dump(DC); } } diff --git a/clang/unittests/AST/ASTImporterTest.cpp b/clang/unittests/AST/ASTImporterTest.cpp index f2bfde9bed3725..a0aaad6082d8cb 100644 --- a/clang/unittests/AST/ASTImporterTest.cpp +++ b/clang/unittests/AST/ASTImporterTest.cpp @@ -6052,7 +6052,7 @@ TEST_P(ASTImporterLookupTableTest, EnumConstantDecl) { EXPECT_EQ(*Res.begin(), A); } -TEST_P(ASTImporterLookupTableTest, LookupSearchesInTheWholeRedeclChain) { +TEST_P(ASTImporterLookupTableTest, LookupSearchesInActualNamespaceOnly) { TranslationUnitDecl *ToTU = getToTuDecl( R"( namespace N { @@ -6062,7 +6062,9 @@ TEST_P(ASTImporterLookupTableTest, LookupSearchesInTheWholeRedeclChain) { } )", Lang_CXX03); - auto *N1 = + auto *N1 = FirstDeclMatcher<NamespaceDecl>().match( + ToTU, namespaceDecl(hasName("N"))); + auto *N2 = LastDeclMatcher<NamespaceDecl>().match(ToTU, namespaceDecl(hasName("N"))); auto *A = FirstDeclMatcher<VarDecl>().match(ToTU, varDecl(hasName("A"))); DeclarationName Name = A->getDeclName(); @@ -6071,6 +6073,7 @@ TEST_P(ASTImporterLookupTableTest, LookupSearchesInTheWholeRedeclChain) { auto Res = LT.lookup(N1, Name); ASSERT_EQ(Res.size(), 1u); EXPECT_EQ(*Res.begin(), A); + EXPECT_TRUE(LT.lookup(N2, Name).empty()); } TEST_P(ASTImporterOptionSpecificTestBase, @@ -10170,6 +10173,151 @@ TEST_P(ImportTemplateParmDeclDefaultValue, FromD, FromDInherited); } +TEST_P(ASTImporterOptionSpecificTestBase, ImportIntoReopenedNamespaceNoMatch1) { + const char *ToCode = + R"( + namespace a { + } + namespace a { + struct X { int A; }; + } + )"; + Decl *ToTU = getToTuDecl(ToCode, Lang_CXX11); + const char *Code = + R"( + namespace a { + struct X { char A; }; + } + )"; + Decl *FromTU = getTuDecl(Code, Lang_CXX11); + auto *FromX = FirstDeclMatcher<CXXRecordDecl>().match( + FromTU, cxxRecordDecl(hasName("X"))); + auto *ImportedX = Import(FromX, Lang_CXX11); + EXPECT_FALSE(ImportedX); +} + +TEST_P(ASTImporterOptionSpecificTestBase, ImportIntoReopenedNamespaceNoMatch2) { + const char *ToCode = + R"( + namespace a { + struct X { int A; }; + } + namespace a { + } + )"; + Decl *ToTU = getToTuDecl(ToCode, Lang_CXX11); + const char *Code = + R"( + namespace a { + struct X { char A; }; + } + )"; + Decl *FromTU = getTuDecl(Code, Lang_CXX11); + auto *FromX = FirstDeclMatcher<CXXRecordDecl>().match( + FromTU, cxxRecordDecl(hasName("X"))); + auto *ImportedX = Import(FromX, Lang_CXX11); + EXPECT_FALSE(ImportedX); +} + +TEST_P(ASTImporterOptionSpecificTestBase, ImportIntoReopenedNamespaceMatch1) { + const char *ToCode = + R"( + namespace a { + } + namespace a { + struct X { int A; }; + } + )"; + Decl *ToTU = getToTuDecl(ToCode, Lang_CXX11); + const char *Code = + R"( + namespace a { + struct X { int A; }; + } + )"; + Decl *FromTU = getTuDecl(Code, Lang_CXX11); + auto *FromX = FirstDeclMatcher<CXXRecordDecl>().match( + FromTU, cxxRecordDecl(hasName("X"))); + auto *ToX = FirstDeclMatcher<CXXRecordDecl>().match( + ToTU, cxxRecordDecl(hasName("X"))); + auto *ImportedX = Import(FromX, Lang_CXX11); + EXPECT_EQ(ImportedX, ToX); +} + +TEST_P(ASTImporterOptionSpecificTestBase, ImportIntoReopenedNamespaceMatch2) { + const char *ToCode = + R"( + namespace a { + struct X { int A; }; + } + namespace a { + } + )"; + Decl *ToTU = getToTuDecl(ToCode, Lang_CXX11); + const char *Code = + R"( + namespace a { + struct X { int A; }; + } + )"; + Decl *FromTU = getTuDecl(Code, Lang_CXX11); + auto *FromX = FirstDeclMatcher<CXXRecordDecl>().match( + FromTU, cxxRecordDecl(hasName("X"))); + auto *ToX = FirstDeclMatcher<CXXRecordDecl>().match( + ToTU, cxxRecordDecl(hasName("X"))); + auto *ImportedX = Import(FromX, Lang_CXX11); + EXPECT_EQ(ImportedX, ToX); +} + +TEST_P(ASTImporterLookupTableTest, PrimaryDCChangeAtImport) { + const char *ToCode = + R"( + template <class T> + struct X; + )"; + Decl *ToTU = getToTuDecl(ToCode, Lang_CXX11); + auto *ToX = FirstDeclMatcher<ClassTemplateDecl>().match( + ToTU, classTemplateDecl(hasName("X"))); + NamedDecl *ToParm = ToX->getTemplateParameters()->getParam(0); + DeclContext *OldPrimaryDC = ToX->getTemplatedDecl()->getPrimaryContext(); + ASSERT_EQ(ToParm->getDeclContext(), ToX->getTemplatedDecl()); + ASSERT_EQ(SharedStatePtr->getLookupTable() + ->lookup(ToX->getTemplatedDecl(), ToParm->getDeclName()) + .size(), + 1u); + ASSERT_TRUE(SharedStatePtr->getLookupTable()->contains( + ToX->getTemplatedDecl(), ToParm)); + + const char *Code = + R"( + template <class T> + struct X; + template <class T> + struct X {}; + )"; + Decl *FromTU = getTuDecl(Code, Lang_CXX11); + auto *FromX = LastDeclMatcher<ClassTemplateDecl>().match( + FromTU, classTemplateDecl(hasName("X"))); + + auto *ImportedX = Import(FromX, Lang_CXX11); + + EXPECT_TRUE(ImportedX); + EXPECT_EQ(ImportedX->getTemplateParameters()->getParam(0)->getDeclContext(), + ImportedX->getTemplatedDecl()); + + // ToX did not change at the import. + // Verify that primary context has changed after import of class definition. + DeclContext *NewPrimaryDC = ToX->getTemplatedDecl()->getPrimaryContext(); + EXPECT_NE(OldPrimaryDC, NewPrimaryDC); + // The lookup table should not be diff erent than it was before. + EXPECT_EQ(SharedStatePtr->getLookupTable() + ->lookup(ToX->getTemplatedDecl(), ToParm->getDeclName()) + .size(), + 1u); + EXPECT_TRUE(SharedStatePtr->getLookupTable()->contains( + ToX->getTemplatedDecl(), ToParm)); +} + TEST_P(ASTImporterOptionSpecificTestBase, ExistingUndeclaredImportDeclaredFriend) { Decl *ToTU = getToTuDecl( _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
