diff --git a/clang/include/clang/AST/ASTImporter.h b/clang/include/clang/AST/ASTImporter.h index 4a0ca45b785a9..3169b0074cc8d 100644 --- a/clang/include/clang/AST/ASTImporter.h +++ b/clang/include/clang/AST/ASTImporter.h @@ -190,6 +190,8 @@ class TypeSourceInfo; llvm::SmallDenseMap Aux; }; + class FunctionReturnTypeDeclCycleDetector; + private: std::shared_ptr SharedState = nullptr; @@ -254,6 +256,13 @@ class TypeSourceInfo; /// Declaration (from, to) pairs that are known not to be equivalent /// (which we have already complained about). NonEquivalentDeclSet NonEquivalentDecls; + /// A FunctionDecl can have properties that have a reference to the + /// function itself and are imported before the function is created. This + /// can come for example from auto return type or when template parameters + /// are used in the return type or parameters. This member is used to detect + /// cyclic import of FunctionDecl objects to avoid infinite recursion. + std::unique_ptr + FunctionReturnTypeCycleDetector; using FoundDeclsTy = SmallVector; FoundDeclsTy findDeclsInToCtx(DeclContext *DC, DeclarationName Name); diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp index f43fa8c90ad3b..c1e0781f0c023 100644 --- a/clang/lib/AST/ASTImporter.cpp +++ b/clang/lib/AST/ASTImporter.cpp @@ -744,13 +744,8 @@ namespace clang { Error ImportOverriddenMethods(CXXMethodDecl *ToMethod, CXXMethodDecl *FromMethod); - Expected FindFunctionTemplateSpecialization( - FunctionDecl *FromFD); - - // Returns true if the given function has a placeholder return type and - // that type is declared inside the body of the function. - // E.g. auto f() { struct X{}; return X(); } - bool hasReturnTypeDeclaredInside(FunctionDecl *D); + Expected + FindFunctionTemplateSpecialization(FunctionDecl *FromFD); }; template @@ -1287,6 +1282,44 @@ bool ASTNodeImporter::hasSameVisibilityContextAndLinkage(TypedefNameDecl *Found, using namespace clang; +class ASTImporter::FunctionReturnTypeDeclCycleDetector { +public: + class DeclCycleMapInserter { + public: + // Do not track cycles on D == nullptr. + DeclCycleMapInserter(FunctionReturnTypeDeclCycleDetector &owner, + const FunctionDecl *D) + : CycleDetector(owner), D(D) { + if (D) + CycleDetector.FunctionReturnTypeDeclCycles.insert(D); + } + ~DeclCycleMapInserter() { + if (D) + CycleDetector.FunctionReturnTypeDeclCycles.erase(D); + } + DeclCycleMapInserter(const DeclCycleMapInserter &) = delete; + DeclCycleMapInserter &operator=(const DeclCycleMapInserter &) = delete; + + private: + FunctionReturnTypeDeclCycleDetector &CycleDetector; + const FunctionDecl *D; + }; + + DeclCycleMapInserter detectImportCycle(const FunctionDecl *D) { + if (!isCycle(D)) + return DeclCycleMapInserter(*this, D); + return DeclCycleMapInserter(*this, nullptr); + } + + bool isCycle(const FunctionDecl *D) const { + return FunctionReturnTypeDeclCycles.find(D) != + FunctionReturnTypeDeclCycles.end(); + } + +private: + llvm::DenseSet FunctionReturnTypeDeclCycles; +}; + ExpectedType ASTNodeImporter::VisitType(const Type *T) { Importer.FromDiag(SourceLocation(), diag::err_unsupported_ast_node) << T->getTypeClassName(); @@ -3864,30 +3897,6 @@ class IsTypeDeclaredInsideVisitor }; } // namespace -/// This function checks if the given function has a return type that contains -/// a reference (in any way) to a declaration inside the same function. -bool ASTNodeImporter::hasReturnTypeDeclaredInside(FunctionDecl *D) { - QualType FromTy = D->getType(); - const auto *FromFPT = FromTy->getAs(); - assert(FromFPT && "Must be called on FunctionProtoType"); - - auto IsCXX11Lambda = [&]() { - if (Importer.FromContext.getLangOpts().CPlusPlus14) // C++14 or later - return false; - - return isLambdaMethod(D); - }; - - QualType RetT = FromFPT->getReturnType(); - if (isa(RetT.getTypePtr()) || IsCXX11Lambda()) { - FunctionDecl *Def = D->getDefinition(); - IsTypeDeclaredInsideVisitor Visitor(Def ? Def : D); - return Visitor.CheckType(RetT); - } - - return false; -} - ExplicitSpecifier ASTNodeImporter::importExplicitSpecifier(Error &Err, ExplicitSpecifier ESpec) { Expr *ExplicitExpr = ESpec.getExpr(); @@ -4035,7 +4044,9 @@ ExpectedDecl ASTNodeImporter::VisitFunctionDecl(FunctionDecl *D) { // E.g.: auto foo() { struct X{}; return X(); } // To avoid an infinite recursion when importing, create the FunctionDecl // with a simplified return type. - if (hasReturnTypeDeclaredInside(D)) { + // Reuse this approach for auto return types declared as typenames from + // template params, tracked in FunctionReturnTypeCycleDetector. + if (Importer.FunctionReturnTypeCycleDetector->isCycle(D)) { FromReturnTy = Importer.getFromContext().VoidTy; UsedDifferentProtoType = true; } @@ -4058,6 +4069,8 @@ ExpectedDecl ASTNodeImporter::VisitFunctionDecl(FunctionDecl *D) { } Error Err = Error::success(); + auto ScopedReturnTypeDeclCycleDetector = + Importer.FunctionReturnTypeCycleDetector->detectImportCycle(D); auto T = importChecked(Err, FromTy); auto TInfo = importChecked(Err, FromTSI); auto ToInnerLocStart = importChecked(Err, D->getInnerLocStart()); @@ -9294,7 +9307,9 @@ ASTImporter::ASTImporter(ASTContext &ToContext, FileManager &ToFileManager, std::shared_ptr SharedState) : SharedState(SharedState), ToContext(ToContext), FromContext(FromContext), ToFileManager(ToFileManager), FromFileManager(FromFileManager), - Minimal(MinimalImport), ODRHandling(ODRHandlingType::Conservative) { + Minimal(MinimalImport), ODRHandling(ODRHandlingType::Conservative), + FunctionReturnTypeCycleDetector( + std::make_unique()) { // Create a default state without the lookup table: LLDB case. if (!SharedState) { diff --git a/clang/unittests/AST/ASTImporterTest.cpp b/clang/unittests/AST/ASTImporterTest.cpp index e7160bcf2e0c2..bbfa591d3efa1 100644 --- a/clang/unittests/AST/ASTImporterTest.cpp +++ b/clang/unittests/AST/ASTImporterTest.cpp @@ -3204,6 +3204,57 @@ TEST_P(ImportExpr, UnresolvedMemberExpr) { compoundStmt(has(callExpr(has(unresolvedMemberExpr()))))))))); } +TEST_P(ImportDecl, CycleInAutoTemplateSpec) { + MatchVerifier Verifier; + const char *Code = R"( + template + struct basic_string { + using value_type = _CharT; + }; + + template + struct basic_string_view { + using value_type = T; + }; + + using string_view = basic_string_view; + using string = basic_string; + + template + struct span { + }; + + template + auto StrCatT(span pieces) { + basic_string result; + return result; + } + + string StrCat(span pieces) { + return StrCatT(pieces); + } + + string StrCat(span pieces) { + return StrCatT(pieces); + } + + template + auto declToImport(T pieces) { + return StrCat(pieces); + } + + void test() { + span pieces; + auto result = declToImport(pieces); + } +)"; + // This test reproduces the StrCatT recursion pattern with concepts and span + // that may cause infinite recursion during AST import due to circular + // dependencies + testImport(Code, Lang_CXX20, "", Lang_CXX20, Verifier, + functionTemplateDecl(hasName("declToImport"))); +} + TEST_P(ImportExpr, ConceptNoRequirement) { MatchVerifier Verifier; const char *Code = R"(