Commit a5e6590b authored by Gabor Marton's avatar Gabor Marton
Browse files

[ASTImporter] Support CXXDeductionGuideDecl with local typedef 

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.

Differential Revision: https://reviews.llvm.org/D92209
parent 4ae8651c

clang/lib/AST/ASTImporter.cpp

+44 −5
Original line number Diff line number Diff line
@@ -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);
@@ -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();
@@ -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);
@@ -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;
@@ -9198,6 +9233,10 @@ 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;

  // 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;

}

clang/unittests/AST/ASTImporterTest.cpp

+16 −0
Original line number Diff line number Diff line
@@ -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