[clang] b270525 - [clang][ASTImporter] Not using primary context in lookup table (#118466)
via cfe-commits
cfe-commits at lists.llvm.org
Mon Jan 13 00:46:50 PST 2025
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(
More information about the cfe-commits
mailing list