[clang] f40f4fc - [clang analysis] ExprMutationAnalyzer support recursive forwarding reference (#88843)
via cfe-commits
cfe-commits at lists.llvm.org
Tue Apr 16 18:57:34 PDT 2024
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) {
More information about the cfe-commits
mailing list