Author: Congcong Cai Date: 2024-04-17T09:57:30+08:00 New Revision: f40f4fcee506deacda0594362509ee7dddcf5e37
URL: https://github.com/llvm/llvm-project/commit/f40f4fcee506deacda0594362509ee7dddcf5e37 DIFF: https://github.com/llvm/llvm-project/commit/f40f4fcee506deacda0594362509ee7dddcf5e37.diff LOG: [clang analysis] ExprMutationAnalyzer support recursive forwarding reference (#88843) Reapply for #88765. Partially fixes: #60895. Added: Modified: clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.h clang-tools-extra/docs/ReleaseNotes.rst clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-templates.cpp clang/include/clang/Analysis/Analyses/ExprMutationAnalyzer.h clang/lib/Analysis/ExprMutationAnalyzer.cpp clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp Removed: ################################################################################ diff --git a/clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp b/clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp index 2fa7cd0baf98f6..c507043c367a86 100644 --- a/clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp +++ b/clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp @@ -85,10 +85,10 @@ void UnnecessaryValueParamCheck::check(const MatchFinder::MatchResult &Result) { TraversalKindScope RAII(*Result.Context, TK_AsIs); - FunctionParmMutationAnalyzer &Analyzer = - MutationAnalyzers.try_emplace(Function, *Function, *Result.Context) - .first->second; - if (Analyzer.isMutated(Param)) + FunctionParmMutationAnalyzer *Analyzer = + FunctionParmMutationAnalyzer::getFunctionParmMutationAnalyzer( + *Function, *Result.Context, MutationAnalyzerCache); + if (Analyzer->isMutated(Param)) return; const bool IsConstQualified = @@ -169,7 +169,7 @@ void UnnecessaryValueParamCheck::storeOptions( } void UnnecessaryValueParamCheck::onEndOfTranslationUnit() { - MutationAnalyzers.clear(); + MutationAnalyzerCache.clear(); } void UnnecessaryValueParamCheck::handleMoveFix(const ParmVarDecl &Var, diff --git a/clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.h b/clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.h index 1872e3bc9bf29c..7250bffd20b2f9 100644 --- a/clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.h +++ b/clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.h @@ -37,8 +37,7 @@ class UnnecessaryValueParamCheck : public ClangTidyCheck { void handleMoveFix(const ParmVarDecl &Var, const DeclRefExpr &CopyArgument, const ASTContext &Context); - llvm::DenseMap<const FunctionDecl *, FunctionParmMutationAnalyzer> - MutationAnalyzers; + ExprMutationAnalyzer::Memoized MutationAnalyzerCache; utils::IncludeInserter Inserter; const std::vector<StringRef> AllowedTypes; }; diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 4dfbd8ca49ab9b..7095c564444fe6 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -221,6 +221,10 @@ Changes in existing checks <clang-tidy/checks/llvm/header-guard>` check by replacing the local option `HeaderFileExtensions` by the global option of the same name. +- Improved :doc:`misc-const-correctness + <clang-tidy/checks/misc/const-correctness>` check by avoiding infinite recursion + for recursive forwarding reference. + - Improved :doc:`misc-definitions-in-headers <clang-tidy/checks/misc/definitions-in-headers>` check by replacing the local option `HeaderFileExtensions` by the global option of the same name. diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-templates.cpp b/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-templates.cpp index 9da468128743e9..248374a71dd40b 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-templates.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-templates.cpp @@ -58,3 +58,18 @@ void concatenate3(Args... args) (..., (stream << args)); } } // namespace gh70323 + +namespace gh60895 { + +template <class T> void f1(T &&a); +template <class T> void f2(T &&a); +template <class T> void f1(T &&a) { f2<T>(a); } +template <class T> void f2(T &&a) { f1<T>(a); } +void f() { + int x = 0; + // CHECK-MESSAGES:[[@LINE-1]]:3: warning: variable 'x' of type 'int' can be declared 'const' + // CHECK-FIXES: int const x = 0; + f1(x); +} + +} // namespace gh60895 diff --git a/clang/include/clang/Analysis/Analyses/ExprMutationAnalyzer.h b/clang/include/clang/Analysis/Analyses/ExprMutationAnalyzer.h index 1ceef944fbc34e..117173ba9a0958 100644 --- a/clang/include/clang/Analysis/Analyses/ExprMutationAnalyzer.h +++ b/clang/include/clang/Analysis/Analyses/ExprMutationAnalyzer.h @@ -8,11 +8,9 @@ #ifndef LLVM_CLANG_ANALYSIS_ANALYSES_EXPRMUTATIONANALYZER_H #define LLVM_CLANG_ANALYSIS_ANALYSES_EXPRMUTATIONANALYZER_H -#include <type_traits> - -#include "clang/AST/AST.h" #include "clang/ASTMatchers/ASTMatchers.h" #include "llvm/ADT/DenseMap.h" +#include <memory> namespace clang { @@ -21,14 +19,74 @@ class FunctionParmMutationAnalyzer; /// Analyzes whether any mutative operations are applied to an expression within /// a given statement. class ExprMutationAnalyzer { + friend class FunctionParmMutationAnalyzer; + public: + struct Memoized { + using ResultMap = llvm::DenseMap<const Expr *, const Stmt *>; + using FunctionParaAnalyzerMap = + llvm::SmallDenseMap<const FunctionDecl *, + std::unique_ptr<FunctionParmMutationAnalyzer>>; + + ResultMap Results; + ResultMap PointeeResults; + FunctionParaAnalyzerMap FuncParmAnalyzer; + + void clear() { + Results.clear(); + PointeeResults.clear(); + FuncParmAnalyzer.clear(); + } + }; + struct Analyzer { + Analyzer(const Stmt &Stm, ASTContext &Context, Memoized &Memorized) + : Stm(Stm), Context(Context), Memorized(Memorized) {} + + const Stmt *findMutation(const Expr *Exp); + const Stmt *findMutation(const Decl *Dec); + + const Stmt *findPointeeMutation(const Expr *Exp); + const Stmt *findPointeeMutation(const Decl *Dec); + static bool isUnevaluated(const Stmt *Smt, const Stmt &Stm, + ASTContext &Context); + + private: + using MutationFinder = const Stmt *(Analyzer::*)(const Expr *); + + const Stmt *findMutationMemoized(const Expr *Exp, + llvm::ArrayRef<MutationFinder> Finders, + Memoized::ResultMap &MemoizedResults); + const Stmt *tryEachDeclRef(const Decl *Dec, MutationFinder Finder); + + bool isUnevaluated(const Expr *Exp); + + const Stmt *findExprMutation(ArrayRef<ast_matchers::BoundNodes> Matches); + const Stmt *findDeclMutation(ArrayRef<ast_matchers::BoundNodes> Matches); + const Stmt * + findExprPointeeMutation(ArrayRef<ast_matchers::BoundNodes> Matches); + const Stmt * + findDeclPointeeMutation(ArrayRef<ast_matchers::BoundNodes> Matches); + + const Stmt *findDirectMutation(const Expr *Exp); + const Stmt *findMemberMutation(const Expr *Exp); + const Stmt *findArrayElementMutation(const Expr *Exp); + const Stmt *findCastMutation(const Expr *Exp); + const Stmt *findRangeLoopMutation(const Expr *Exp); + const Stmt *findReferenceMutation(const Expr *Exp); + const Stmt *findFunctionArgMutation(const Expr *Exp); + + const Stmt &Stm; + ASTContext &Context; + Memoized &Memorized; + }; + ExprMutationAnalyzer(const Stmt &Stm, ASTContext &Context) - : Stm(Stm), Context(Context) {} + : Memorized(), A(Stm, Context, Memorized) {} bool isMutated(const Expr *Exp) { return findMutation(Exp) != nullptr; } bool isMutated(const Decl *Dec) { return findMutation(Dec) != nullptr; } - const Stmt *findMutation(const Expr *Exp); - const Stmt *findMutation(const Decl *Dec); + const Stmt *findMutation(const Expr *Exp) { return A.findMutation(Exp); } + const Stmt *findMutation(const Decl *Dec) { return A.findMutation(Dec); } bool isPointeeMutated(const Expr *Exp) { return findPointeeMutation(Exp) != nullptr; @@ -36,51 +94,40 @@ class ExprMutationAnalyzer { bool isPointeeMutated(const Decl *Dec) { return findPointeeMutation(Dec) != nullptr; } - const Stmt *findPointeeMutation(const Expr *Exp); - const Stmt *findPointeeMutation(const Decl *Dec); + const Stmt *findPointeeMutation(const Expr *Exp) { + return A.findPointeeMutation(Exp); + } + const Stmt *findPointeeMutation(const Decl *Dec) { + return A.findPointeeMutation(Dec); + } + static bool isUnevaluated(const Stmt *Smt, const Stmt &Stm, - ASTContext &Context); + ASTContext &Context) { + return Analyzer::isUnevaluated(Smt, Stm, Context); + } private: - using MutationFinder = const Stmt *(ExprMutationAnalyzer::*)(const Expr *); - using ResultMap = llvm::DenseMap<const Expr *, const Stmt *>; - - const Stmt *findMutationMemoized(const Expr *Exp, - llvm::ArrayRef<MutationFinder> Finders, - ResultMap &MemoizedResults); - const Stmt *tryEachDeclRef(const Decl *Dec, MutationFinder Finder); - - bool isUnevaluated(const Expr *Exp); - - const Stmt *findExprMutation(ArrayRef<ast_matchers::BoundNodes> Matches); - const Stmt *findDeclMutation(ArrayRef<ast_matchers::BoundNodes> Matches); - const Stmt * - findExprPointeeMutation(ArrayRef<ast_matchers::BoundNodes> Matches); - const Stmt * - findDeclPointeeMutation(ArrayRef<ast_matchers::BoundNodes> Matches); - - const Stmt *findDirectMutation(const Expr *Exp); - const Stmt *findMemberMutation(const Expr *Exp); - const Stmt *findArrayElementMutation(const Expr *Exp); - const Stmt *findCastMutation(const Expr *Exp); - const Stmt *findRangeLoopMutation(const Expr *Exp); - const Stmt *findReferenceMutation(const Expr *Exp); - const Stmt *findFunctionArgMutation(const Expr *Exp); - - const Stmt &Stm; - ASTContext &Context; - llvm::DenseMap<const FunctionDecl *, - std::unique_ptr<FunctionParmMutationAnalyzer>> - FuncParmAnalyzer; - ResultMap Results; - ResultMap PointeeResults; + Memoized Memorized; + Analyzer A; }; // A convenient wrapper around ExprMutationAnalyzer for analyzing function // params. class FunctionParmMutationAnalyzer { public: - FunctionParmMutationAnalyzer(const FunctionDecl &Func, ASTContext &Context); + static FunctionParmMutationAnalyzer * + getFunctionParmMutationAnalyzer(const FunctionDecl &Func, ASTContext &Context, + ExprMutationAnalyzer::Memoized &Memorized) { + auto it = Memorized.FuncParmAnalyzer.find(&Func); + if (it == Memorized.FuncParmAnalyzer.end()) + it = + Memorized.FuncParmAnalyzer + .try_emplace(&Func, std::unique_ptr<FunctionParmMutationAnalyzer>( + new FunctionParmMutationAnalyzer( + Func, Context, Memorized))) + .first; + return it->getSecond().get(); + } bool isMutated(const ParmVarDecl *Parm) { return findMutation(Parm) != nullptr; @@ -88,8 +135,11 @@ class FunctionParmMutationAnalyzer { const Stmt *findMutation(const ParmVarDecl *Parm); private: - ExprMutationAnalyzer BodyAnalyzer; + ExprMutationAnalyzer::Analyzer BodyAnalyzer; llvm::DenseMap<const ParmVarDecl *, const Stmt *> Results; + + FunctionParmMutationAnalyzer(const FunctionDecl &Func, ASTContext &Context, + ExprMutationAnalyzer::Memoized &Memorized); }; } // namespace clang diff --git a/clang/lib/Analysis/ExprMutationAnalyzer.cpp b/clang/lib/Analysis/ExprMutationAnalyzer.cpp index bb042760d297a7..941322be8f870b 100644 --- a/clang/lib/Analysis/ExprMutationAnalyzer.cpp +++ b/clang/lib/Analysis/ExprMutationAnalyzer.cpp @@ -186,9 +186,10 @@ template <> struct NodeID<Decl> { static constexpr StringRef value = "decl"; }; constexpr StringRef NodeID<Expr>::value; constexpr StringRef NodeID<Decl>::value; -template <class T, class F = const Stmt *(ExprMutationAnalyzer::*)(const T *)> +template <class T, + class F = const Stmt *(ExprMutationAnalyzer::Analyzer::*)(const T *)> const Stmt *tryEachMatch(ArrayRef<ast_matchers::BoundNodes> Matches, - ExprMutationAnalyzer *Analyzer, F Finder) { + ExprMutationAnalyzer::Analyzer *Analyzer, F Finder) { const StringRef ID = NodeID<T>::value; for (const auto &Nodes : Matches) { if (const Stmt *S = (Analyzer->*Finder)(Nodes.getNodeAs<T>(ID))) @@ -199,33 +200,37 @@ const Stmt *tryEachMatch(ArrayRef<ast_matchers::BoundNodes> Matches, } // namespace -const Stmt *ExprMutationAnalyzer::findMutation(const Expr *Exp) { - return findMutationMemoized(Exp, - {&ExprMutationAnalyzer::findDirectMutation, - &ExprMutationAnalyzer::findMemberMutation, - &ExprMutationAnalyzer::findArrayElementMutation, - &ExprMutationAnalyzer::findCastMutation, - &ExprMutationAnalyzer::findRangeLoopMutation, - &ExprMutationAnalyzer::findReferenceMutation, - &ExprMutationAnalyzer::findFunctionArgMutation}, - Results); +const Stmt *ExprMutationAnalyzer::Analyzer::findMutation(const Expr *Exp) { + return findMutationMemoized( + Exp, + {&ExprMutationAnalyzer::Analyzer::findDirectMutation, + &ExprMutationAnalyzer::Analyzer::findMemberMutation, + &ExprMutationAnalyzer::Analyzer::findArrayElementMutation, + &ExprMutationAnalyzer::Analyzer::findCastMutation, + &ExprMutationAnalyzer::Analyzer::findRangeLoopMutation, + &ExprMutationAnalyzer::Analyzer::findReferenceMutation, + &ExprMutationAnalyzer::Analyzer::findFunctionArgMutation}, + Memorized.Results); } -const Stmt *ExprMutationAnalyzer::findMutation(const Decl *Dec) { - return tryEachDeclRef(Dec, &ExprMutationAnalyzer::findMutation); +const Stmt *ExprMutationAnalyzer::Analyzer::findMutation(const Decl *Dec) { + return tryEachDeclRef(Dec, &ExprMutationAnalyzer::Analyzer::findMutation); } -const Stmt *ExprMutationAnalyzer::findPointeeMutation(const Expr *Exp) { - return findMutationMemoized(Exp, {/*TODO*/}, PointeeResults); +const Stmt * +ExprMutationAnalyzer::Analyzer::findPointeeMutation(const Expr *Exp) { + return findMutationMemoized(Exp, {/*TODO*/}, Memorized.PointeeResults); } -const Stmt *ExprMutationAnalyzer::findPointeeMutation(const Decl *Dec) { - return tryEachDeclRef(Dec, &ExprMutationAnalyzer::findPointeeMutation); +const Stmt * +ExprMutationAnalyzer::Analyzer::findPointeeMutation(const Decl *Dec) { + return tryEachDeclRef(Dec, + &ExprMutationAnalyzer::Analyzer::findPointeeMutation); } -const Stmt *ExprMutationAnalyzer::findMutationMemoized( +const Stmt *ExprMutationAnalyzer::Analyzer::findMutationMemoized( const Expr *Exp, llvm::ArrayRef<MutationFinder> Finders, - ResultMap &MemoizedResults) { + Memoized::ResultMap &MemoizedResults) { const auto Memoized = MemoizedResults.find(Exp); if (Memoized != MemoizedResults.end()) return Memoized->second; @@ -241,8 +246,9 @@ const Stmt *ExprMutationAnalyzer::findMutationMemoized( return MemoizedResults[Exp] = nullptr; } -const Stmt *ExprMutationAnalyzer::tryEachDeclRef(const Decl *Dec, - MutationFinder Finder) { +const Stmt * +ExprMutationAnalyzer::Analyzer::tryEachDeclRef(const Decl *Dec, + MutationFinder Finder) { const auto Refs = match( findAll( declRefExpr(to( @@ -261,8 +267,9 @@ const Stmt *ExprMutationAnalyzer::tryEachDeclRef(const Decl *Dec, return nullptr; } -bool ExprMutationAnalyzer::isUnevaluated(const Stmt *Exp, const Stmt &Stm, - ASTContext &Context) { +bool ExprMutationAnalyzer::Analyzer::isUnevaluated(const Stmt *Exp, + const Stmt &Stm, + ASTContext &Context) { return selectFirst<Stmt>( NodeID<Expr>::value, match( @@ -293,33 +300,36 @@ bool ExprMutationAnalyzer::isUnevaluated(const Stmt *Exp, const Stmt &Stm, Stm, Context)) != nullptr; } -bool ExprMutationAnalyzer::isUnevaluated(const Expr *Exp) { +bool ExprMutationAnalyzer::Analyzer::isUnevaluated(const Expr *Exp) { return isUnevaluated(Exp, Stm, Context); } const Stmt * -ExprMutationAnalyzer::findExprMutation(ArrayRef<BoundNodes> Matches) { - return tryEachMatch<Expr>(Matches, this, &ExprMutationAnalyzer::findMutation); +ExprMutationAnalyzer::Analyzer::findExprMutation(ArrayRef<BoundNodes> Matches) { + return tryEachMatch<Expr>(Matches, this, + &ExprMutationAnalyzer::Analyzer::findMutation); } const Stmt * -ExprMutationAnalyzer::findDeclMutation(ArrayRef<BoundNodes> Matches) { - return tryEachMatch<Decl>(Matches, this, &ExprMutationAnalyzer::findMutation); +ExprMutationAnalyzer::Analyzer::findDeclMutation(ArrayRef<BoundNodes> Matches) { + return tryEachMatch<Decl>(Matches, this, + &ExprMutationAnalyzer::Analyzer::findMutation); } -const Stmt *ExprMutationAnalyzer::findExprPointeeMutation( +const Stmt *ExprMutationAnalyzer::Analyzer::findExprPointeeMutation( ArrayRef<ast_matchers::BoundNodes> Matches) { - return tryEachMatch<Expr>(Matches, this, - &ExprMutationAnalyzer::findPointeeMutation); + return tryEachMatch<Expr>( + Matches, this, &ExprMutationAnalyzer::Analyzer::findPointeeMutation); } -const Stmt *ExprMutationAnalyzer::findDeclPointeeMutation( +const Stmt *ExprMutationAnalyzer::Analyzer::findDeclPointeeMutation( ArrayRef<ast_matchers::BoundNodes> Matches) { - return tryEachMatch<Decl>(Matches, this, - &ExprMutationAnalyzer::findPointeeMutation); + return tryEachMatch<Decl>( + Matches, this, &ExprMutationAnalyzer::Analyzer::findPointeeMutation); } -const Stmt *ExprMutationAnalyzer::findDirectMutation(const Expr *Exp) { +const Stmt * +ExprMutationAnalyzer::Analyzer::findDirectMutation(const Expr *Exp) { // LHS of any assignment operators. const auto AsAssignmentLhs = binaryOperator(isAssignmentOperator(), hasLHS(canResolveToExpr(Exp))); @@ -426,7 +436,7 @@ const Stmt *ExprMutationAnalyzer::findDirectMutation(const Expr *Exp) { const auto AsNonConstRefReturn = returnStmt(hasReturnValue(canResolveToExpr(Exp))); - // It is used as a non-const-reference for initalizing a range-for loop. + // It is used as a non-const-reference for initializing a range-for loop. const auto AsNonConstRefRangeInit = cxxForRangeStmt(hasRangeInit(declRefExpr( allOf(canResolveToExpr(Exp), hasType(nonConstReferenceType()))))); @@ -443,7 +453,8 @@ const Stmt *ExprMutationAnalyzer::findDirectMutation(const Expr *Exp) { return selectFirst<Stmt>("stmt", Matches); } -const Stmt *ExprMutationAnalyzer::findMemberMutation(const Expr *Exp) { +const Stmt * +ExprMutationAnalyzer::Analyzer::findMemberMutation(const Expr *Exp) { // Check whether any member of 'Exp' is mutated. const auto MemberExprs = match( findAll(expr(anyOf(memberExpr(hasObjectExpression(canResolveToExpr(Exp))), @@ -456,7 +467,8 @@ const Stmt *ExprMutationAnalyzer::findMemberMutation(const Expr *Exp) { return findExprMutation(MemberExprs); } -const Stmt *ExprMutationAnalyzer::findArrayElementMutation(const Expr *Exp) { +const Stmt * +ExprMutationAnalyzer::Analyzer::findArrayElementMutation(const Expr *Exp) { // Check whether any element of an array is mutated. const auto SubscriptExprs = match( findAll(arraySubscriptExpr( @@ -469,7 +481,7 @@ const Stmt *ExprMutationAnalyzer::findArrayElementMutation(const Expr *Exp) { return findExprMutation(SubscriptExprs); } -const Stmt *ExprMutationAnalyzer::findCastMutation(const Expr *Exp) { +const Stmt *ExprMutationAnalyzer::Analyzer::findCastMutation(const Expr *Exp) { // If the 'Exp' is explicitly casted to a non-const reference type the // 'Exp' is considered to be modified. const auto ExplicitCast = @@ -504,7 +516,8 @@ const Stmt *ExprMutationAnalyzer::findCastMutation(const Expr *Exp) { return findExprMutation(Calls); } -const Stmt *ExprMutationAnalyzer::findRangeLoopMutation(const Expr *Exp) { +const Stmt * +ExprMutationAnalyzer::Analyzer::findRangeLoopMutation(const Expr *Exp) { // Keep the ordering for the specific initialization matches to happen first, // because it is cheaper to match all potential modifications of the loop // variable. @@ -567,7 +580,8 @@ const Stmt *ExprMutationAnalyzer::findRangeLoopMutation(const Expr *Exp) { return findDeclMutation(LoopVars); } -const Stmt *ExprMutationAnalyzer::findReferenceMutation(const Expr *Exp) { +const Stmt * +ExprMutationAnalyzer::Analyzer::findReferenceMutation(const Expr *Exp) { // Follow non-const reference returned by `operator*()` of move-only classes. // These are typically smart pointers with unique ownership so we treat // mutation of pointee as mutation of the smart pointer itself. @@ -599,7 +613,8 @@ const Stmt *ExprMutationAnalyzer::findReferenceMutation(const Expr *Exp) { return findDeclMutation(Refs); } -const Stmt *ExprMutationAnalyzer::findFunctionArgMutation(const Expr *Exp) { +const Stmt * +ExprMutationAnalyzer::Analyzer::findFunctionArgMutation(const Expr *Exp) { const auto NonConstRefParam = forEachArgumentWithParam( canResolveToExpr(Exp), parmVarDecl(hasType(nonConstReferenceType())).bind("parm")); @@ -637,10 +652,9 @@ const Stmt *ExprMutationAnalyzer::findFunctionArgMutation(const Expr *Exp) { if (const auto *RefType = ParmType->getAs<RValueReferenceType>()) { if (!RefType->getPointeeType().getQualifiers() && RefType->getPointeeType()->getAs<TemplateTypeParmType>()) { - std::unique_ptr<FunctionParmMutationAnalyzer> &Analyzer = - FuncParmAnalyzer[Func]; - if (!Analyzer) - Analyzer.reset(new FunctionParmMutationAnalyzer(*Func, Context)); + FunctionParmMutationAnalyzer *Analyzer = + FunctionParmMutationAnalyzer::getFunctionParmMutationAnalyzer( + *Func, Context, Memorized); if (Analyzer->findMutation(Parm)) return Exp; continue; @@ -653,13 +667,15 @@ const Stmt *ExprMutationAnalyzer::findFunctionArgMutation(const Expr *Exp) { } FunctionParmMutationAnalyzer::FunctionParmMutationAnalyzer( - const FunctionDecl &Func, ASTContext &Context) - : BodyAnalyzer(*Func.getBody(), Context) { + const FunctionDecl &Func, ASTContext &Context, + ExprMutationAnalyzer::Memoized &Memorized) + : BodyAnalyzer(*Func.getBody(), Context, Memorized) { if (const auto *Ctor = dyn_cast<CXXConstructorDecl>(&Func)) { // CXXCtorInitializer might also mutate Param but they're not part of // function body, check them eagerly here since they're typically trivial. for (const CXXCtorInitializer *Init : Ctor->inits()) { - ExprMutationAnalyzer InitAnalyzer(*Init->getInit(), Context); + ExprMutationAnalyzer::Analyzer InitAnalyzer(*Init->getInit(), Context, + Memorized); for (const ParmVarDecl *Parm : Ctor->parameters()) { if (Results.contains(Parm)) continue; @@ -675,11 +691,14 @@ FunctionParmMutationAnalyzer::findMutation(const ParmVarDecl *Parm) { const auto Memoized = Results.find(Parm); if (Memoized != Results.end()) return Memoized->second; - + // To handle call A -> call B -> call A. Assume parameters of A is not mutated + // before analyzing parameters of A. Then when analyzing the second "call A", + // FunctionParmMutationAnalyzer can use this memoized value to avoid infinite + // recursion. + Results[Parm] = nullptr; if (const Stmt *S = BodyAnalyzer.findMutation(Parm)) return Results[Parm] = S; - - return Results[Parm] = nullptr; + return Results[Parm]; } } // namespace clang diff --git a/clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp b/clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp index f58ce4aebcbfc8..9c1dc1a76db63d 100644 --- a/clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp +++ b/clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp @@ -977,6 +977,36 @@ TEST(ExprMutationAnalyzerTest, FollowFuncArgModified) { "void f() { int x; g(x); }"); Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("g(x)")); + + AST = buildASTFromCode( + StdRemoveReference + StdForward + + "template <class T> void f1(T &&a);" + "template <class T> void f2(T &&a);" + "template <class T> void f1(T &&a) { f2<T>(std::forward<T>(a)); }" + "template <class T> void f2(T &&a) { f1<T>(std::forward<T>(a)); }" + "void f() { int x; f1(x); }"); + Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); + EXPECT_FALSE(isMutated(Results, AST.get())); + + AST = buildASTFromCode( + StdRemoveReference + StdForward + + "template <class T> void f1(T &&a);" + "template <class T> void f2(T &&a);" + "template <class T> void f1(T &&a) { f2<T>(std::forward<T>(a)); }" + "template <class T> void f2(T &&a) { f1<T>(std::forward<T>(a)); a++; }" + "void f() { int x; f1(x); }"); + Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); + EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("f1(x)")); + + AST = buildASTFromCode( + StdRemoveReference + StdForward + + "template <class T> void f1(T &&a);" + "template <class T> void f2(T &&a);" + "template <class T> void f1(T &&a) { f2<T>(std::forward<T>(a)); a++; }" + "template <class T> void f2(T &&a) { f1<T>(std::forward<T>(a)); }" + "void f() { int x; f1(x); }"); + Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); + EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("f1(x)")); } TEST(ExprMutationAnalyzerTest, FollowFuncArgNotModified) { _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits