Skip to content

Commit

Permalink
[ASTImporter] Support CXXDeductionGuideDecl with local typedef
Browse files Browse the repository at this point in the history
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
 rust-lang#2 clang::ASTImporter::ImportImpl(clang::Decl*) clang/lib/AST/ASTImporter.cpp:8038:0
 rust-lang#3 clang::ASTImporter::Import(clang::Decl*) clang/lib/AST/ASTImporter.cpp:8200:0
 rust-lang#4 clang::ASTImporter::ImportContext(clang::DeclContext*) clang/lib/AST/ASTImporter.cpp:8297:0
 rust-lang#5 clang::ASTNodeImporter::ImportDeclContext(clang::Decl*, clang::DeclContext*&, clang::DeclContext*&) clang/lib/AST/ASTImporter.cpp:1852:0
 rust-lang#6 clang::ASTNodeImporter::ImportDeclParts(clang::NamedDecl*, clang::DeclContext*&, clang::DeclContext*&, clang::DeclarationName&, clang::NamedDecl*&, clang::SourceLocation&) clang/lib/AST/ASTImporter.cpp:1628:0
 rust-lang#7 clang::ASTNodeImporter::VisitTypedefNameDecl(clang::TypedefNameDecl*, bool) clang/lib/AST/ASTImporter.cpp:2419:0
 rust-lang#8 clang::ASTNodeImporter::VisitTypedefDecl(clang::TypedefDecl*) clang/lib/AST/ASTImporter.cpp:2500:0
 rust-lang#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
 rust-lang#10 clang::ASTImporter::ImportImpl(clang::Decl*) clang/lib/AST/ASTImporter.cpp:8038:0
 rust-lang#11 clang::ASTImporter::Import(clang::Decl*) clang/lib/AST/ASTImporter.cpp:8200:0
 rust-lang#12 llvm::Expected<clang::TypedefNameDecl*> clang::ASTNodeImporter::import<clang::TypedefNameDecl>(clang::TypedefNameDecl*) clang/lib/AST/ASTImporter.cpp:165:0
 rust-lang#13 clang::ASTNodeImporter::VisitTypedefType(clang::TypedefType const*) clang/lib/AST/ASTImporter.cpp:1304:0
 rust-lang#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
 rust-lang#15 clang::ASTImporter::Import(clang::QualType) clang/lib/AST/ASTImporter.cpp:8071:0
 rust-lang#16 llvm::Expected<clang::QualType> clang::ASTNodeImporter::import<clang::QualType>(clang::QualType const&) clang/lib/AST/ASTImporter.cpp:179:0
 rust-lang#17 clang::ASTNodeImporter::VisitFunctionProtoType(clang::FunctionProtoType const*) clang/lib/AST/ASTImporter.cpp:1244:0
 rust-lang#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
 rust-lang#19 clang::ASTImporter::Import(clang::QualType) clang/lib/AST/ASTImporter.cpp:8071:0
 rust-lang#20 llvm::Expected<clang::QualType> clang::ASTNodeImporter::import<clang::QualType>(clang::QualType const&) clang/lib/AST/ASTImporter.cpp:179:0
 rust-lang#21 clang::QualType clang::ASTNodeImporter::importChecked<clang::QualType>(llvm::Error&, clang::QualType const&) clang/lib/AST/ASTImporter.cpp:198:0
 rust-lang#22 clang::ASTNodeImporter::VisitFunctionDecl(clang::FunctionDecl*) clang/lib/AST/ASTImporter.cpp:3313:0
 rust-lang#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.

Differential Revision: https://reviews.llvm.org/D92209
  • Loading branch information
Gabor Marton committed Dec 9, 2020
1 parent 4ae8651 commit a5e6590
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 5 deletions.
49 changes: 44 additions & 5 deletions clang/lib/AST/ASTImporter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -388,6 +388,8 @@ namespace clang {
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);
Expand Down Expand Up @@ -1647,6 +1649,25 @@ Error ASTNodeImporter::ImportDeclParts(
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();
Expand Down Expand Up @@ -2415,22 +2436,29 @@ ExpectedDecl ASTNodeImporter::VisitNamespaceAliasDecl(NamespaceAliasDecl *D) {
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);
Expand Down Expand Up @@ -2487,8 +2515,15 @@ ASTNodeImporter::VisitTypedefNameDecl(TypedefNameDecl *D, bool IsAlias) {
Name.getAsIdentifierInfo(), ToTypeSourceInfo))
return ToTypedef;

ToTypedef->setAccess(D->getAccess());
// Import the DeclContext and set it to the Typedef.
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;
Expand Down Expand Up @@ -9198,7 +9233,11 @@ Decl *ASTImporter::MapImported(Decl *From, Decl *To) {
// 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;
}

Expand Down
16 changes: 16 additions & 0 deletions clang/unittests/AST/ASTImporterTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5998,6 +5998,22 @@ TEST_P(ImportFunctions, CTADUserDefinedExplicit) {
EXPECT_TRUE(ToD->isExplicit());
}

TEST_P(ImportFunctions, CTADWithLocalTypedef) {
Decl *TU = getTuDecl(
R"(
template <typename T> struct A {
typedef T U;
A(U);
};
A a{(int)0};
)",
Lang_CXX17, "input.cc");
auto *FromD = FirstDeclMatcher<CXXDeductionGuideDecl>().match(
TU, cxxDeductionGuideDecl());
auto *ToD = Import(FromD, Lang_CXX17);
ASSERT_TRUE(ToD);
}

// 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
Expand Down

0 comments on commit a5e6590

Please sign in to comment.