Skip to content

Commit b270525

Browse files
balazskebalazske
authored
[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.
1 parent 4f96fb5 commit b270525

File tree

3 files changed

+181
-15
lines changed

3 files changed

+181
-15
lines changed

‎clang/lib/AST/ASTImporter.cpp‎

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3165,6 +3165,7 @@ ExpectedDecl ASTNodeImporter::VisitRecordDecl(RecordDecl *D) {
31653165
if (Error Err = ImportImplicitMethods(DCXX, FoundCXX))
31663166
return std::move(Err);
31673167
}
3168+
// FIXME: We can return FoundDef here.
31683169
}
31693170
PrevDecl = FoundRecord->getMostRecentDecl();
31703171
break;
@@ -9064,9 +9065,26 @@ ASTImporter::findDeclsInToCtx(DeclContext *DC, DeclarationName Name) {
90649065
// We can diagnose this only if we search in the redecl context.
90659066
DeclContext *ReDC = DC->getRedeclContext();
90669067
if (SharedState->getLookupTable()) {
9067-
ASTImporterLookupTable::LookupResult LookupResult =
9068-
SharedState->getLookupTable()->lookup(ReDC, Name);
9069-
return FoundDeclsTy(LookupResult.begin(), LookupResult.end());
9068+
if (ReDC->isNamespace()) {
9069+
// Namespaces can be reopened.
9070+
// Lookup table does not handle this, we must search here in all linked
9071+
// namespaces.
9072+
FoundDeclsTy Result;
9073+
SmallVector<Decl *, 2> NSChain =
9074+
getCanonicalForwardRedeclChain<NamespaceDecl>(
9075+
dyn_cast<NamespaceDecl>(ReDC));
9076+
for (auto *D : NSChain) {
9077+
ASTImporterLookupTable::LookupResult LookupResult =
9078+
SharedState->getLookupTable()->lookup(dyn_cast<NamespaceDecl>(D),
9079+
Name);
9080+
Result.append(LookupResult.begin(), LookupResult.end());
9081+
}
9082+
return Result;
9083+
} else {
9084+
ASTImporterLookupTable::LookupResult LookupResult =
9085+
SharedState->getLookupTable()->lookup(ReDC, Name);
9086+
return FoundDeclsTy(LookupResult.begin(), LookupResult.end());
9087+
}
90709088
} else {
90719089
DeclContext::lookup_result NoloadLookupResult = ReDC->noload_lookup(Name);
90729090
FoundDeclsTy Result(NoloadLookupResult.begin(), NoloadLookupResult.end());

‎clang/lib/AST/ASTImporterLookupTable.cpp‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -115,8 +115,9 @@ void ASTImporterLookupTable::remove(DeclContext *DC, NamedDecl *ND) {
115115
#ifndef NDEBUG
116116
if (!EraseResult) {
117117
std::string Message =
118-
llvm::formatv("Trying to remove not contained Decl '{0}' of type {1}",
119-
Name.getAsString(), DC->getDeclKindName())
118+
llvm::formatv(
119+
"Trying to remove not contained Decl '{0}' of type {1} from a {2}",
120+
Name.getAsString(), ND->getDeclKindName(), DC->getDeclKindName())
120121
.str();
121122
llvm_unreachable(Message.c_str());
122123
}
@@ -125,18 +126,18 @@ void ASTImporterLookupTable::remove(DeclContext *DC, NamedDecl *ND) {
125126

126127
void ASTImporterLookupTable::add(NamedDecl *ND) {
127128
assert(ND);
128-
DeclContext *DC = ND->getDeclContext()->getPrimaryContext();
129+
DeclContext *DC = ND->getDeclContext();
129130
add(DC, ND);
130-
DeclContext *ReDC = DC->getRedeclContext()->getPrimaryContext();
131+
DeclContext *ReDC = DC->getRedeclContext();
131132
if (DC != ReDC)
132133
add(ReDC, ND);
133134
}
134135

135136
void ASTImporterLookupTable::remove(NamedDecl *ND) {
136137
assert(ND);
137-
DeclContext *DC = ND->getDeclContext()->getPrimaryContext();
138+
DeclContext *DC = ND->getDeclContext();
138139
remove(DC, ND);
139-
DeclContext *ReDC = DC->getRedeclContext()->getPrimaryContext();
140+
DeclContext *ReDC = DC->getRedeclContext();
140141
if (DC != ReDC)
141142
remove(ReDC, ND);
142143
}
@@ -161,7 +162,7 @@ void ASTImporterLookupTable::updateForced(NamedDecl *ND, DeclContext *OldDC) {
161162

162163
ASTImporterLookupTable::LookupResult
163164
ASTImporterLookupTable::lookup(DeclContext *DC, DeclarationName Name) const {
164-
auto DCI = LookupTable.find(DC->getPrimaryContext());
165+
auto DCI = LookupTable.find(DC);
165166
if (DCI == LookupTable.end())
166167
return {};
167168

@@ -178,7 +179,7 @@ bool ASTImporterLookupTable::contains(DeclContext *DC, NamedDecl *ND) const {
178179
}
179180

180181
void ASTImporterLookupTable::dump(DeclContext *DC) const {
181-
auto DCI = LookupTable.find(DC->getPrimaryContext());
182+
auto DCI = LookupTable.find(DC);
182183
if (DCI == LookupTable.end())
183184
llvm::errs() << "empty\n";
184185
const auto &FoundNameMap = DCI->second;
@@ -196,8 +197,7 @@ void ASTImporterLookupTable::dump(DeclContext *DC) const {
196197
void ASTImporterLookupTable::dump() const {
197198
for (const auto &Entry : LookupTable) {
198199
DeclContext *DC = Entry.first;
199-
StringRef Primary = DC->getPrimaryContext() ? " primary" : "";
200-
llvm::errs() << "== DC:" << cast<Decl>(DC) << Primary << "\n";
200+
llvm::errs() << "== DC:" << cast<Decl>(DC) << "\n";
201201
dump(DC);
202202
}
203203
}

‎clang/unittests/AST/ASTImporterTest.cpp‎

Lines changed: 150 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6052,7 +6052,7 @@ TEST_P(ASTImporterLookupTableTest, EnumConstantDecl) {
60526052
EXPECT_EQ(*Res.begin(), A);
60536053
}
60546054

6055-
TEST_P(ASTImporterLookupTableTest, LookupSearchesInTheWholeRedeclChain) {
6055+
TEST_P(ASTImporterLookupTableTest, LookupSearchesInActualNamespaceOnly) {
60566056
TranslationUnitDecl *ToTU = getToTuDecl(
60576057
R"(
60586058
namespace N {
@@ -6062,7 +6062,9 @@ TEST_P(ASTImporterLookupTableTest, LookupSearchesInTheWholeRedeclChain) {
60626062
}
60636063
)",
60646064
Lang_CXX03);
6065-
auto *N1 =
6065+
auto *N1 = FirstDeclMatcher<NamespaceDecl>().match(
6066+
ToTU, namespaceDecl(hasName("N")));
6067+
auto *N2 =
60666068
LastDeclMatcher<NamespaceDecl>().match(ToTU, namespaceDecl(hasName("N")));
60676069
auto *A = FirstDeclMatcher<VarDecl>().match(ToTU, varDecl(hasName("A")));
60686070
DeclarationName Name = A->getDeclName();
@@ -6071,6 +6073,7 @@ TEST_P(ASTImporterLookupTableTest, LookupSearchesInTheWholeRedeclChain) {
60716073
auto Res = LT.lookup(N1, Name);
60726074
ASSERT_EQ(Res.size(), 1u);
60736075
EXPECT_EQ(*Res.begin(), A);
6076+
EXPECT_TRUE(LT.lookup(N2, Name).empty());
60746077
}
60756078

60766079
TEST_P(ASTImporterOptionSpecificTestBase,
@@ -10170,6 +10173,151 @@ TEST_P(ImportTemplateParmDeclDefaultValue,
1017010173
FromD, FromDInherited);
1017110174
}
1017210175

10176+
TEST_P(ASTImporterOptionSpecificTestBase, ImportIntoReopenedNamespaceNoMatch1) {
10177+
const char *ToCode =
10178+
R"(
10179+
namespace a {
10180+
}
10181+
namespace a {
10182+
struct X { int A; };
10183+
}
10184+
)";
10185+
Decl *ToTU = getToTuDecl(ToCode, Lang_CXX11);
10186+
const char *Code =
10187+
R"(
10188+
namespace a {
10189+
struct X { char A; };
10190+
}
10191+
)";
10192+
Decl *FromTU = getTuDecl(Code, Lang_CXX11);
10193+
auto *FromX = FirstDeclMatcher<CXXRecordDecl>().match(
10194+
FromTU, cxxRecordDecl(hasName("X")));
10195+
auto *ImportedX = Import(FromX, Lang_CXX11);
10196+
EXPECT_FALSE(ImportedX);
10197+
}
10198+
10199+
TEST_P(ASTImporterOptionSpecificTestBase, ImportIntoReopenedNamespaceNoMatch2) {
10200+
const char *ToCode =
10201+
R"(
10202+
namespace a {
10203+
struct X { int A; };
10204+
}
10205+
namespace a {
10206+
}
10207+
)";
10208+
Decl *ToTU = getToTuDecl(ToCode, Lang_CXX11);
10209+
const char *Code =
10210+
R"(
10211+
namespace a {
10212+
struct X { char A; };
10213+
}
10214+
)";
10215+
Decl *FromTU = getTuDecl(Code, Lang_CXX11);
10216+
auto *FromX = FirstDeclMatcher<CXXRecordDecl>().match(
10217+
FromTU, cxxRecordDecl(hasName("X")));
10218+
auto *ImportedX = Import(FromX, Lang_CXX11);
10219+
EXPECT_FALSE(ImportedX);
10220+
}
10221+
10222+
TEST_P(ASTImporterOptionSpecificTestBase, ImportIntoReopenedNamespaceMatch1) {
10223+
const char *ToCode =
10224+
R"(
10225+
namespace a {
10226+
}
10227+
namespace a {
10228+
struct X { int A; };
10229+
}
10230+
)";
10231+
Decl *ToTU = getToTuDecl(ToCode, Lang_CXX11);
10232+
const char *Code =
10233+
R"(
10234+
namespace a {
10235+
struct X { int A; };
10236+
}
10237+
)";
10238+
Decl *FromTU = getTuDecl(Code, Lang_CXX11);
10239+
auto *FromX = FirstDeclMatcher<CXXRecordDecl>().match(
10240+
FromTU, cxxRecordDecl(hasName("X")));
10241+
auto *ToX = FirstDeclMatcher<CXXRecordDecl>().match(
10242+
ToTU, cxxRecordDecl(hasName("X")));
10243+
auto *ImportedX = Import(FromX, Lang_CXX11);
10244+
EXPECT_EQ(ImportedX, ToX);
10245+
}
10246+
10247+
TEST_P(ASTImporterOptionSpecificTestBase, ImportIntoReopenedNamespaceMatch2) {
10248+
const char *ToCode =
10249+
R"(
10250+
namespace a {
10251+
struct X { int A; };
10252+
}
10253+
namespace a {
10254+
}
10255+
)";
10256+
Decl *ToTU = getToTuDecl(ToCode, Lang_CXX11);
10257+
const char *Code =
10258+
R"(
10259+
namespace a {
10260+
struct X { int A; };
10261+
}
10262+
)";
10263+
Decl *FromTU = getTuDecl(Code, Lang_CXX11);
10264+
auto *FromX = FirstDeclMatcher<CXXRecordDecl>().match(
10265+
FromTU, cxxRecordDecl(hasName("X")));
10266+
auto *ToX = FirstDeclMatcher<CXXRecordDecl>().match(
10267+
ToTU, cxxRecordDecl(hasName("X")));
10268+
auto *ImportedX = Import(FromX, Lang_CXX11);
10269+
EXPECT_EQ(ImportedX, ToX);
10270+
}
10271+
10272+
TEST_P(ASTImporterLookupTableTest, PrimaryDCChangeAtImport) {
10273+
const char *ToCode =
10274+
R"(
10275+
template <class T>
10276+
struct X;
10277+
)";
10278+
Decl *ToTU = getToTuDecl(ToCode, Lang_CXX11);
10279+
auto *ToX = FirstDeclMatcher<ClassTemplateDecl>().match(
10280+
ToTU, classTemplateDecl(hasName("X")));
10281+
NamedDecl *ToParm = ToX->getTemplateParameters()->getParam(0);
10282+
DeclContext *OldPrimaryDC = ToX->getTemplatedDecl()->getPrimaryContext();
10283+
ASSERT_EQ(ToParm->getDeclContext(), ToX->getTemplatedDecl());
10284+
ASSERT_EQ(SharedStatePtr->getLookupTable()
10285+
->lookup(ToX->getTemplatedDecl(), ToParm->getDeclName())
10286+
.size(),
10287+
1u);
10288+
ASSERT_TRUE(SharedStatePtr->getLookupTable()->contains(
10289+
ToX->getTemplatedDecl(), ToParm));
10290+
10291+
const char *Code =
10292+
R"(
10293+
template <class T>
10294+
struct X;
10295+
template <class T>
10296+
struct X {};
10297+
)";
10298+
Decl *FromTU = getTuDecl(Code, Lang_CXX11);
10299+
auto *FromX = LastDeclMatcher<ClassTemplateDecl>().match(
10300+
FromTU, classTemplateDecl(hasName("X")));
10301+
10302+
auto *ImportedX = Import(FromX, Lang_CXX11);
10303+
10304+
EXPECT_TRUE(ImportedX);
10305+
EXPECT_EQ(ImportedX->getTemplateParameters()->getParam(0)->getDeclContext(),
10306+
ImportedX->getTemplatedDecl());
10307+
10308+
// ToX did not change at the import.
10309+
// Verify that primary context has changed after import of class definition.
10310+
DeclContext *NewPrimaryDC = ToX->getTemplatedDecl()->getPrimaryContext();
10311+
EXPECT_NE(OldPrimaryDC, NewPrimaryDC);
10312+
// The lookup table should not be different than it was before.
10313+
EXPECT_EQ(SharedStatePtr->getLookupTable()
10314+
->lookup(ToX->getTemplatedDecl(), ToParm->getDeclName())
10315+
.size(),
10316+
1u);
10317+
EXPECT_TRUE(SharedStatePtr->getLookupTable()->contains(
10318+
ToX->getTemplatedDecl(), ToParm));
10319+
}
10320+
1017310321
TEST_P(ASTImporterOptionSpecificTestBase,
1017410322
ExistingUndeclaredImportDeclaredFriend) {
1017510323
Decl *ToTU = getToTuDecl(

0 commit comments

Comments
 (0)