martong created this revision.
martong added reviewers: teemperor, balazske.
Herald added subscribers: cfe-commits, gamesh411, Szelethus, dkrupp, rnkovacs.
Herald added a reviewer: a.sidorin.
Herald added a reviewer: shafik.
Herald added a project: clang.
martong requested review of this revision.
CXXDeductionGuideDecl with a local typedef has its own copy of the
TypedefDecl with the CXXDeductionGuideDecl as the DeclContext of that
TypedefDecl.
template <typename T> struct A {
typedef T U;
A(U, T);
};
A a{(int)0, (int)0};
Related discussion on cfe-dev:
http://lists.llvm.org/pipermail/cfe-dev/2020-November/067252.html
Without this fix, when we import the CXXDeductionGuideDecl (via
VisitFunctionDecl) then before creating the Decl we must import the
FunctionType. However, the first parameter's type is the afore mentioned
local typedef. So, we then start importing the TypedefDecl whose
DeclContext is the CXXDeductionGuideDecl itself. The infinite loop is
formed.
#0
clang::ASTNodeImporter::VisitCXXDeductionGuideDecl(clang::CXXDeductionGuideDecl*)
clang/lib/AST/ASTImporter.cpp:3543:0
#1 clang::declvisitor::Base<std::add_pointer, clang::ASTNodeImporter,
llvm::Expected<clang::Decl*> >::Visit(clang::Decl*)
/home/egbomrt/WORK/llvm5/build/debug/tools/clang/include/clang/AST/DeclNodes.inc:405:0
#2 clang::ASTImporter::ImportImpl(clang::Decl*)
clang/lib/AST/ASTImporter.cpp:8038:0
#3 clang::ASTImporter::Import(clang::Decl*)
clang/lib/AST/ASTImporter.cpp:8200:0
#4 clang::ASTImporter::ImportContext(clang::DeclContext*)
clang/lib/AST/ASTImporter.cpp:8297:0
#5 clang::ASTNodeImporter::ImportDeclContext(clang::Decl*,
clang::DeclContext*&, clang::DeclContext*&) clang/lib/AST/ASTImporter.cpp:1852:0
#6 clang::ASTNodeImporter::ImportDeclParts(clang::NamedDecl*,
clang::DeclContext*&, clang::DeclContext*&, clang::DeclarationName&,
clang::NamedDecl*&, clang::SourceLocation&) clang/lib/AST/ASTImporter.cpp:1628:0
#7 clang::ASTNodeImporter::VisitTypedefNameDecl(clang::TypedefNameDecl*,
bool) clang/lib/AST/ASTImporter.cpp:2419:0
#8 clang::ASTNodeImporter::VisitTypedefDecl(clang::TypedefDecl*)
clang/lib/AST/ASTImporter.cpp:2500:0
#9 clang::declvisitor::Base<std::add_pointer, clang::ASTNodeImporter,
llvm::Expected<clang::Decl*> >::Visit(clang::Decl*)
/home/egbomrt/WORK/llvm5/build/debug/tools/clang/include/clang/AST/DeclNodes.inc:315:0
#10 clang::ASTImporter::ImportImpl(clang::Decl*)
clang/lib/AST/ASTImporter.cpp:8038:0
#11 clang::ASTImporter::Import(clang::Decl*)
clang/lib/AST/ASTImporter.cpp:8200:0
#12 llvm::Expected<clang::TypedefNameDecl*>
clang::ASTNodeImporter::import<clang::TypedefNameDecl>(clang::TypedefNameDecl*)
clang/lib/AST/ASTImporter.cpp:165:0
#13 clang::ASTNodeImporter::VisitTypedefType(clang::TypedefType const*)
clang/lib/AST/ASTImporter.cpp:1304:0
#14 clang::TypeVisitor<clang::ASTNodeImporter,
llvm::Expected<clang::QualType> >::Visit(clang::Type const*)
/home/egbomrt/WORK/llvm5/build/debug/tools/clang/include/clang/AST/TypeNodes.inc:74:0
#15 clang::ASTImporter::Import(clang::QualType)
clang/lib/AST/ASTImporter.cpp:8071:0
#16 llvm::Expected<clang::QualType>
clang::ASTNodeImporter::import<clang::QualType>(clang::QualType const&)
clang/lib/AST/ASTImporter.cpp:179:0
#17 clang::ASTNodeImporter::VisitFunctionProtoType(clang::FunctionProtoType
const*) clang/lib/AST/ASTImporter.cpp:1244:0
#18 clang::TypeVisitor<clang::ASTNodeImporter,
llvm::Expected<clang::QualType> >::Visit(clang::Type const*)
/home/egbomrt/WORK/llvm5/build/debug/tools/clang/include/clang/AST/TypeNodes.inc:47:0
#19 clang::ASTImporter::Import(clang::QualType)
clang/lib/AST/ASTImporter.cpp:8071:0
#20 llvm::Expected<clang::QualType>
clang::ASTNodeImporter::import<clang::QualType>(clang::QualType const&)
clang/lib/AST/ASTImporter.cpp:179:0
#21 clang::QualType
clang::ASTNodeImporter::importChecked<clang::QualType>(llvm::Error&,
clang::QualType const&) clang/lib/AST/ASTImporter.cpp:198:0
#22 clang::ASTNodeImporter::VisitFunctionDecl(clang::FunctionDecl*)
clang/lib/AST/ASTImporter.cpp:3313:0
#23
clang::ASTNodeImporter::VisitCXXDeductionGuideDecl(clang::CXXDeductionGuideDecl*)
clang/lib/AST/ASTImporter.cpp:3543:0
The fix is to first create the TypedefDecl and only then start to import
the DeclContext.
Basically, we could do this during the import of all other Decls (not
just for typedefs). But it seems, there is only one another AST
construct that has a similar cycle: a struct defined as a function
parameter:
int struct_in_proto(struct data_t{int a;int b;} *d);
In that case, however, we had decided to return simply with an error
back then because that seemed to be a very rare construct.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D92209
Files:
clang/lib/AST/ASTImporter.cpp
clang/unittests/AST/ASTImporterTest.cpp
Index: clang/unittests/AST/ASTImporterTest.cpp
===================================================================
--- clang/unittests/AST/ASTImporterTest.cpp
+++ clang/unittests/AST/ASTImporterTest.cpp
@@ -5924,6 +5924,22 @@
EXPECT_TRUE(ToD->isExplicit());
}
+TEST_P(ImportFunctions, CTADWithLocalTypedef) {
+ Decl *TU = getTuDecl(
+ R"(
+ template <typename T> struct A {
+ typedef T U;
+ A(U, T);
+ };
+ A a{(int)0, (int)0};
+ )",
+ Lang_CXX17, "input.cc");
+ auto *FromD = FirstDeclMatcher<CXXDeductionGuideDecl>().match(
+ TU, cxxDeductionGuideDecl());
+ auto *ToD = Import(FromD, Lang_CXX17);
+ ASSERT_TRUE(ToD);
+}
+
INSTANTIATE_TEST_CASE_P(ParameterizedTests, ASTImporterLookupTableTest,
DefaultTestValuesForRunOptions, );
Index: clang/lib/AST/ASTImporter.cpp
===================================================================
--- clang/lib/AST/ASTImporter.cpp
+++ clang/lib/AST/ASTImporter.cpp
@@ -385,6 +385,8 @@
ExpectedType VisitObjCObjectPointerType(const ObjCObjectPointerType *T);
// Importing declarations
+ Error ImportDeclParts(NamedDecl *D, DeclarationName &Name, NamedDecl *&ToD,
+ SourceLocation &Loc);
Error ImportDeclParts(
NamedDecl *D, DeclContext *&DC, DeclContext *&LexicalDC,
DeclarationName &Name, NamedDecl *&ToD, SourceLocation &Loc);
@@ -1644,6 +1646,25 @@
return Error::success();
}
+Error ASTNodeImporter::ImportDeclParts(NamedDecl *D, DeclarationName &Name,
+ NamedDecl *&ToD, SourceLocation &Loc) {
+
+ // Import the name of this declaration.
+ if (Error Err = importInto(Name, D->getDeclName()))
+ return Err;
+
+ // Import the location of this declaration.
+ if (Error Err = importInto(Loc, D->getLocation()))
+ return Err;
+
+ ToD = cast_or_null<NamedDecl>(Importer.GetAlreadyImportedOrNull(D));
+ if (ToD)
+ if (Error Err = ASTNodeImporter(*this).ImportDefinitionIfNeeded(D, ToD))
+ return Err;
+
+ return Error::success();
+}
+
Error ASTNodeImporter::ImportDefinitionIfNeeded(Decl *FromD, Decl *ToD) {
if (!FromD)
return Error::success();
@@ -2412,22 +2433,29 @@
ExpectedDecl
ASTNodeImporter::VisitTypedefNameDecl(TypedefNameDecl *D, bool IsAlias) {
// Import the major distinguishing characteristics of this typedef.
- DeclContext *DC, *LexicalDC;
DeclarationName Name;
SourceLocation Loc;
NamedDecl *ToD;
- if (Error Err = ImportDeclParts(D, DC, LexicalDC, Name, ToD, Loc))
+ // Do not import the DeclContext, we will import it once the TypedefNameDecl
+ // is created.
+ if (Error Err = ImportDeclParts(D, Name, ToD, Loc))
return std::move(Err);
if (ToD)
return ToD;
+ DeclContext *DC = cast_or_null<DeclContext>(
+ Importer.GetAlreadyImportedOrNull(cast<Decl>(D->getDeclContext())));
+ DeclContext *LexicalDC =
+ cast_or_null<DeclContext>(Importer.GetAlreadyImportedOrNull(
+ cast<Decl>(D->getLexicalDeclContext())));
+
// If this typedef is not in block scope, determine whether we've
// seen a typedef with the same name (that we can merge with) or any
// other entity by that name (which name lookup could conflict with).
// Note: Repeated typedefs are not valid in C99:
// 'typedef int T; typedef int T;' is invalid
// We do not care about this now.
- if (!DC->isFunctionOrMethod()) {
+ if (DC && !DC->isFunctionOrMethod()) {
SmallVector<NamedDecl *, 4> ConflictingDecls;
unsigned IDNS = Decl::IDNS_Ordinary;
auto FoundDecls = Importer.findDeclsInToCtx(DC, Name);
@@ -2484,8 +2512,16 @@
Name.getAsIdentifierInfo(), ToTypeSourceInfo))
return ToTypedef;
- ToTypedef->setAccess(D->getAccess());
+ // Import the DeclContext and set it to the Typedef.
+ if (!DC || !LexicalDC)
+ if ((Err = ImportDeclContext(D, DC, LexicalDC)))
+ return std::move(Err);
+ ToTypedef->setDeclContext(DC);
ToTypedef->setLexicalDeclContext(LexicalDC);
+ // Add to the lookupTable because we could not do that in MapImported.
+ Importer.AddToLookupTable(ToTypedef);
+
+ ToTypedef->setAccess(D->getAccess());
// Templated declarations should not appear in DeclContext.
TypeAliasDecl *FromAlias = IsAlias ? cast<TypeAliasDecl>(D) : nullptr;
@@ -9189,7 +9225,11 @@
// This mapping should be maintained only in this function. Therefore do not
// check for additional consistency.
ImportedFromDecls[To] = From;
- AddToLookupTable(To);
+ // In the case of TypedefNameDecl we create the Decl first and only then we
+ // import and set its DeclContext. So, the DC is still not set when we reach
+ // here from GetImportedOrCreateDecl.
+ if (To->getDeclContext())
+ AddToLookupTable(To);
return To;
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
