balazske created this revision.
Herald added subscribers: whisperity, martong, teemperor, gamesh411, Szelethus,
dkrupp, kristof.beyls.
Herald added a reviewer: a.sidorin.
Herald added a reviewer: shafik.
balazske requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
ParmVarDecl is created with translation unit as the parent DeclContext
and later moved to the correct DeclContext. ASTImporterLookupTable
should be updated at this move.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D103231
Files:
clang/include/clang/AST/ASTImporterLookupTable.h
clang/lib/AST/ASTImporter.cpp
clang/lib/AST/ASTImporterLookupTable.cpp
clang/unittests/AST/ASTImporterTest.cpp
Index: clang/unittests/AST/ASTImporterTest.cpp
===================================================================
--- clang/unittests/AST/ASTImporterTest.cpp
+++ clang/unittests/AST/ASTImporterTest.cpp
@@ -5341,6 +5341,33 @@
CheckError(FromFooC);
}
+TEST_P(ErrorHandlingTest, ODRViolationWithinParmVarDecls) {
+ // Importing of 'f' and parameter 'P' should cause an ODR error.
+ // The error happens after the ParmVarDecl for 'P' was already created.
+ // This is a special case because the ParmVarDecl has a temporary DeclContext.
+ // Expected is no crash at error handling of ASTImporter.
+ constexpr auto ToTUCode = R"(
+ struct X {
+ char A;
+ };
+ )";
+ constexpr auto FromTUCode = R"(
+ struct X {
+ enum Y { Z };
+ };
+ void f(int P = X::Z);
+ )";
+ Decl *ToTU = getToTuDecl(ToTUCode, Lang_CXX11);
+ static_cast<void>(ToTU);
+ Decl *FromTU = getTuDecl(FromTUCode, Lang_CXX11);
+ auto *FromF = FirstDeclMatcher<FunctionDecl>().match(
+ FromTU, functionDecl(hasName("f")));
+ ASSERT_TRUE(FromF);
+
+ auto *ImportedF = Import(FromF, Lang_CXX11);
+ EXPECT_FALSE(ImportedF);
+}
+
TEST_P(ASTImporterOptionSpecificTestBase, LambdaInFunctionBody) {
Decl *FromTU = getTuDecl(
R"(
@@ -6158,6 +6185,23 @@
ASSERT_TRUE(ToD);
}
+TEST_P(ImportFunctions, ParmVarDeclDeclContext) {
+ constexpr auto FromTUCode = R"(
+ void f(int P);
+ )";
+ Decl *FromTU = getTuDecl(FromTUCode, Lang_CXX11);
+ auto *FromF = FirstDeclMatcher<FunctionDecl>().match(
+ FromTU, functionDecl(hasName("f")));
+ ASSERT_TRUE(FromF);
+
+ auto *ImportedF = Import(FromF, Lang_CXX11);
+ EXPECT_TRUE(ImportedF);
+ EXPECT_FALSE(
+ SharedStatePtr->getLookupTable()
+ ->lookup(ImportedF, ImportedF->getParamDecl(0)->getDeclName())
+ .empty());
+}
+
// FIXME Move these tests out of ASTImporterTest. For that we need to factor
// out the ASTImporter specific pars from ASTImporterOptionSpecificTestBase
// into a new test Fixture. Then we should lift up this Fixture to its own
Index: clang/lib/AST/ASTImporterLookupTable.cpp
===================================================================
--- clang/lib/AST/ASTImporterLookupTable.cpp
+++ clang/lib/AST/ASTImporterLookupTable.cpp
@@ -117,6 +117,17 @@
remove(ReDC, ND);
}
+void ASTImporterLookupTable::update(NamedDecl *ND, DeclContext *OldDC) {
+ assert(OldDC != ND->getDeclContext());
+ if (!lookup(ND->getDeclContext(), ND->getDeclName()).empty()) {
+ assert(lookup(OldDC, ND->getDeclName()).empty());
+ return;
+ }
+
+ remove(OldDC, ND);
+ add(ND);
+}
+
ASTImporterLookupTable::LookupResult
ASTImporterLookupTable::lookup(DeclContext *DC, DeclarationName Name) const {
auto DCI = LookupTable.find(DC->getPrimaryContext());
Index: clang/lib/AST/ASTImporter.cpp
===================================================================
--- clang/lib/AST/ASTImporter.cpp
+++ clang/lib/AST/ASTImporter.cpp
@@ -3501,6 +3501,8 @@
for (auto *Param : Parameters) {
Param->setOwningFunction(ToFunction);
ToFunction->addDeclInternal(Param);
+ if (ASTImporterLookupTable *LT = Importer.SharedState->getLookupTable())
+ LT->update(Param, Importer.getToContext().getTranslationUnitDecl());
}
ToFunction->setParams(Parameters);
Index: clang/include/clang/AST/ASTImporterLookupTable.h
===================================================================
--- clang/include/clang/AST/ASTImporterLookupTable.h
+++ clang/include/clang/AST/ASTImporterLookupTable.h
@@ -63,6 +63,18 @@
ASTImporterLookupTable(TranslationUnitDecl &TU);
void add(NamedDecl *ND);
void remove(NamedDecl *ND);
+ // Sometimes a declaration is created first with a temporarily value of decl
+ // context (often the translation unit) and later moved to the final context.
+ // This happens for declarations that are created before the final declaration
+ // context. In such cases the lookup table needs to be updated.
+ // (The declaration is in these cases not added to the temporary decl context,
+ // only its parent is set.)
+ // FIXME: It would be better to not add the declaration to the temporary
+ // context at all in the lookup table, but this requires big change in
+ // ASTImporter.
+ // The function should be called when the old context is definitely different
+ // from the new.
+ void update(NamedDecl *ND, DeclContext *OldDC);
using LookupResult = DeclList;
LookupResult lookup(DeclContext *DC, DeclarationName Name) const;
void dump(DeclContext *DC) const;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
