Author: Balázs Kéri Date: 2026-08-11T10:19:29+02:00 New Revision: d082719c27df1ffbcf7bed42252f7404eaf24e4e
URL: https://github.com/llvm/llvm-project/commit/d082719c27df1ffbcf7bed42252f7404eaf24e4e DIFF: https://github.com/llvm/llvm-project/commit/d082719c27df1ffbcf7bed42252f7404eaf24e4e.diff LOG: [clang][ASTImporter] Remove 'hasReturnTypeDeclaredInside' (#169504) The function `hasReturnTypeDeclaredInside` and a related visitor class was used to detect recursive import of function declarations. There is now a new way to detect the recursive import that works in all cases (unlike the previous method). A test is added to show a case when the old solution does not work. Added: clang/test/Analysis/ctu/import_incomplete_array_auto_return_type.cpp Modified: clang/lib/AST/ASTImporter.cpp Removed: ################################################################################ diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp index ba84929a33c10..5be9ce780aec5 100644 --- a/clang/lib/AST/ASTImporter.cpp +++ b/clang/lib/AST/ASTImporter.cpp @@ -755,13 +755,8 @@ namespace clang { Error ImportOverriddenMethods(CXXMethodDecl *ToMethod, CXXMethodDecl *FromMethod); - Expected<FunctionDecl *> 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<FunctionDecl *> + FindFunctionTemplateSpecialization(FunctionDecl *FromFD); }; template <typename InContainerTy> @@ -3781,223 +3776,6 @@ Error ASTNodeImporter::ImportFunctionDeclBody(FunctionDecl *FromFD, return Error::success(); } -// Returns true if the given D has a DeclContext up to the TranslationUnitDecl -// which is equal to the given DC, or D is equal to DC. -static bool isAncestorDeclContextOf(const DeclContext *DC, const Decl *D) { - const DeclContext *DCi = dyn_cast<DeclContext>(D); - if (!DCi) - DCi = D->getDeclContext(); - assert(DCi && "Declaration should have a context"); - while (DCi != D->getTranslationUnitDecl()) { - if (DCi == DC) - return true; - DCi = DCi->getParent(); - } - return false; -} - -// Check if there is a declaration that has 'DC' as parent context and is -// referenced from statement 'S' or one of its children. The search is done in -// BFS order through children of 'S'. -static bool isAncestorDeclContextOf(const DeclContext *DC, const Stmt *S) { - SmallVector<const Stmt *> ToProcess; - ToProcess.push_back(S); - while (!ToProcess.empty()) { - const Stmt *CurrentS = ToProcess.pop_back_val(); - ToProcess.append(CurrentS->child_begin(), CurrentS->child_end()); - if (const auto *DeclRef = dyn_cast<DeclRefExpr>(CurrentS)) { - if (const Decl *D = DeclRef->getDecl()) - if (isAncestorDeclContextOf(DC, D)) - return true; - } else if (const auto *E = - dyn_cast_or_null<SubstNonTypeTemplateParmExpr>(CurrentS)) { - if (const Decl *D = E->getAssociatedDecl()) - if (isAncestorDeclContextOf(DC, D)) - return true; - } - } - return false; -} - -namespace { -/// Check if a type has any reference to a declaration that is inside the body -/// of a function. -/// The \c CheckType(QualType) function should be used to determine -/// this property. -/// -/// The type visitor visits one type object only (not recursive). -/// To find all referenced declarations we must discover all type objects until -/// the canonical type is reached (walk over typedef and similar objects). This -/// is done by loop over all "sugar" type objects. For every such type we must -/// check all declarations that are referenced from it. For this check the -/// visitor is used. In the visit functions all referenced declarations except -/// the one that follows in the sugar chain (if any) must be checked. For this -/// check the same visitor is re-used (it has no state-dependent data). -/// -/// The visit functions have 3 possible return values: -/// - True, found a declaration inside \c ParentDC. -/// - False, found declarations only outside \c ParentDC and it is not possible -/// to find more declarations (the "sugar" chain does not continue). -/// - Empty optional value, found no declarations or only outside \c ParentDC, -/// but it is possible to find more declarations in the type "sugar" chain. -/// The loop over the "sugar" types can be implemented by using type visit -/// functions only (call \c CheckType with the desugared type). With the current -/// solution no visit function is needed if the type has only a desugared type -/// as data. -class IsTypeDeclaredInsideVisitor - : public TypeVisitor<IsTypeDeclaredInsideVisitor, std::optional<bool>> { -public: - IsTypeDeclaredInsideVisitor(const FunctionDecl *ParentDC) - : ParentDC(ParentDC) {} - - bool CheckType(QualType T) { - // Check the chain of "sugar" types. - // The "sugar" types are typedef or similar types that have the same - // canonical type. - if (std::optional<bool> Res = Visit(T.getTypePtr())) - return *Res; - QualType DsT = - T.getSingleStepDesugaredType(ParentDC->getParentASTContext()); - while (DsT != T) { - if (std::optional<bool> Res = Visit(DsT.getTypePtr())) - return *Res; - T = DsT; - DsT = T.getSingleStepDesugaredType(ParentDC->getParentASTContext()); - } - return false; - } - - std::optional<bool> VisitTagType(const TagType *T) { - if (auto *Spec = dyn_cast<ClassTemplateSpecializationDecl>(T->getDecl())) - for (const auto &Arg : Spec->getTemplateArgs().asArray()) - if (checkTemplateArgument(Arg)) - return true; - return isAncestorDeclContextOf(ParentDC, T->getDecl()); - } - - std::optional<bool> VisitPointerType(const PointerType *T) { - return CheckType(T->getPointeeType()); - } - - std::optional<bool> VisitReferenceType(const ReferenceType *T) { - return CheckType(T->getPointeeTypeAsWritten()); - } - - std::optional<bool> VisitTypedefType(const TypedefType *T) { - return isAncestorDeclContextOf(ParentDC, T->getDecl()); - } - - std::optional<bool> VisitUsingType(const UsingType *T) { - return isAncestorDeclContextOf(ParentDC, T->getDecl()); - } - - std::optional<bool> - VisitTemplateSpecializationType(const TemplateSpecializationType *T) { - for (const auto &Arg : T->template_arguments()) - if (checkTemplateArgument(Arg)) - return true; - // This type is a "sugar" to a record type, it can have a desugared type. - return {}; - } - - std::optional<bool> VisitUnaryTransformType(const UnaryTransformType *T) { - return CheckType(T->getBaseType()); - } - - std::optional<bool> - VisitSubstTemplateTypeParmType(const SubstTemplateTypeParmType *T) { - // The "associated declaration" can be the same as ParentDC. - if (isAncestorDeclContextOf(ParentDC, T->getAssociatedDecl())) - return true; - return {}; - } - - std::optional<bool> VisitConstantArrayType(const ConstantArrayType *T) { - if (T->getSizeExpr() && isAncestorDeclContextOf(ParentDC, T->getSizeExpr())) - return true; - - return CheckType(T->getElementType()); - } - - std::optional<bool> VisitVariableArrayType(const VariableArrayType *T) { - llvm_unreachable( - "Variable array should not occur in deduced return type of a function"); - } - - std::optional<bool> VisitIncompleteArrayType(const IncompleteArrayType *T) { - llvm_unreachable("Incomplete array should not occur in deduced return type " - "of a function"); - } - - std::optional<bool> VisitDependentArrayType(const IncompleteArrayType *T) { - llvm_unreachable("Dependent array should not occur in deduced return type " - "of a function"); - } - -private: - const DeclContext *const ParentDC; - - bool checkTemplateArgument(const TemplateArgument &Arg) { - switch (Arg.getKind()) { - case TemplateArgument::Null: - return false; - case TemplateArgument::Integral: - return CheckType(Arg.getIntegralType()); - case TemplateArgument::Type: - return CheckType(Arg.getAsType()); - case TemplateArgument::Expression: - return isAncestorDeclContextOf(ParentDC, Arg.getAsExpr()); - case TemplateArgument::Declaration: - // FIXME: The declaration in this case is not allowed to be in a function? - return isAncestorDeclContextOf(ParentDC, Arg.getAsDecl()); - case TemplateArgument::NullPtr: - // FIXME: The type is not allowed to be in the function? - return CheckType(Arg.getNullPtrType()); - case TemplateArgument::StructuralValue: - return CheckType(Arg.getStructuralValueType()); - case TemplateArgument::Pack: - for (const auto &PackArg : Arg.getPackAsArray()) - if (checkTemplateArgument(PackArg)) - return true; - return false; - case TemplateArgument::Template: - // Templates can not be defined locally in functions. - // A template passed as argument can be not in ParentDC. - return false; - case TemplateArgument::TemplateExpansion: - // Templates can not be defined locally in functions. - // A template passed as argument can be not in ParentDC. - return false; - } - llvm_unreachable("Unknown TemplateArgument::ArgKind enum"); - }; -}; -} // 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<FunctionProtoType>(); - 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<AutoType>(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(); @@ -4143,12 +3921,11 @@ ExpectedDecl ASTNodeImporter::VisitFunctionDecl(FunctionDecl *D) { // Functions with auto return type may define a struct inside their body // and the return type could refer to that struct. // E.g.: auto foo() { struct X{}; return X(); } - // To avoid an infinite recursion when importing, create the FunctionDecl - // with a simplified return type. - // Reuse this approach for auto return types declared as typenames from - // template params, tracked in FindFunctionDeclImportCycle. - if (hasReturnTypeDeclaredInside(D) || - Importer.FindFunctionDeclImportCycle.isCycle(D)) { + // There are many more cases when types inside the function declaration + // can appear in the return type, like types declared as typenames from + // template params. + // All such cases are tracked in FindFunctionDeclImportCycle. + if (Importer.FindFunctionDeclImportCycle.isCycle(D)) { FromReturnTy = Importer.getFromContext().VoidTy; UsedDifferentProtoType = true; } diff --git a/clang/test/Analysis/ctu/import_incomplete_array_auto_return_type.cpp b/clang/test/Analysis/ctu/import_incomplete_array_auto_return_type.cpp new file mode 100644 index 0000000000000..b8d8074ff6743 --- /dev/null +++ b/clang/test/Analysis/ctu/import_incomplete_array_auto_return_type.cpp @@ -0,0 +1,43 @@ +// RUN: rm -rf %t +// RUN: mkdir -p %t +// RUN: split-file %s %t + +// RUN: %clang_cc1 -std=c++14 -emit-pch -o %t/import.cpp.ast %t/import.cpp + +// RUN: %clang_extdef_map %t/import.cpp -- -std=c++14 > %t/externalDefMap.tmp.txt +// On windows, absolute paths generated by extdef_map are not recognized, +// so CSA prepends the workdir path to them. Force relative paths to work +// around this issue. +// RUN: sed -e 's| .*import\.cpp| import.cpp.ast|' \ +// RUN: %t/externalDefMap.tmp.txt > %t/externalDefMap.txt + +// RUN: %clang_cc1 -std=c++14 -analyze \ +// RUN: -analyzer-checker=core \ +// RUN: -analyzer-config experimental-enable-naive-ctu-analysis=true \ +// RUN: -analyzer-config ctu-dir=%t \ +// RUN: -verify %t/main.cpp + +//--- main.cpp + +// expected-no-diagnostics + +namespace llvm { void trigger(); } + +void entry() { llvm::trigger(); } + +//--- import.cpp + +template <typename> using a = int; + +namespace llvm { + +// This function's deduced return type retains the alias-template sugar +// `a<int[]>`, whose template argument is an incomplete array type. +// ASTImporter should be able to handle this case when importing the function. +auto plainFunc() { + return a<int[]>(); +} + +void trigger() {} + +} // namespace llvm _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
