llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Congcong Cai (HerrCai0907) <details> <summary>Changes</summary> Partialy fixes: #<!-- -->60895 --- Full diff: https://github.com/llvm/llvm-project/pull/87954.diff 3 Files Affected: - (modified) clang/include/clang/Analysis/Analyses/ExprMutationAnalyzer.h (+33-7) - (modified) clang/lib/Analysis/ExprMutationAnalyzer.cpp (+14-8) - (modified) clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp (+30) ``````````diff diff --git a/clang/include/clang/Analysis/Analyses/ExprMutationAnalyzer.h b/clang/include/clang/Analysis/Analyses/ExprMutationAnalyzer.h index 1ceef944fbc34e..af6106fe303afd 100644 --- a/clang/include/clang/Analysis/Analyses/ExprMutationAnalyzer.h +++ b/clang/include/clang/Analysis/Analyses/ExprMutationAnalyzer.h @@ -8,11 +8,10 @@ #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 <variant> namespace clang { @@ -22,8 +21,15 @@ class FunctionParmMutationAnalyzer; /// a given statement. class ExprMutationAnalyzer { public: + friend class FunctionParmMutationAnalyzer; + struct Cache { + llvm::DenseMap<const FunctionDecl *, + std::unique_ptr<FunctionParmMutationAnalyzer>> + FuncParmAnalyzer; + }; + ExprMutationAnalyzer(const Stmt &Stm, ASTContext &Context) - : Stm(Stm), Context(Context) {} + : ExprMutationAnalyzer(Stm, Context, nullptr) {} bool isMutated(const Expr *Exp) { return findMutation(Exp) != nullptr; } bool isMutated(const Decl *Dec) { return findMutation(Dec) != nullptr; } @@ -45,6 +51,19 @@ class ExprMutationAnalyzer { using MutationFinder = const Stmt *(ExprMutationAnalyzer::*)(const Expr *); using ResultMap = llvm::DenseMap<const Expr *, const Stmt *>; + ExprMutationAnalyzer(const Stmt &Stm, ASTContext &Context, Cache *ParentCache) + : Stm(Stm), Context(Context) { + if (ParentCache != nullptr) { + CrossAnalysisCache = ParentCache; + } else { + CrossAnalysisCache = std::make_unique<Cache>(); + } + } + ExprMutationAnalyzer(const Stmt &Stm, ASTContext &Context, + std::unique_ptr<Cache> CrossAnalysisCache) + : Stm(Stm), Context(Context), + CrossAnalysisCache(std::move(CrossAnalysisCache)) {} + const Stmt *findMutationMemoized(const Expr *Exp, llvm::ArrayRef<MutationFinder> Finders, ResultMap &MemoizedResults); @@ -67,11 +86,15 @@ class ExprMutationAnalyzer { const Stmt *findReferenceMutation(const Expr *Exp); const Stmt *findFunctionArgMutation(const Expr *Exp); + Cache *getCache() { + return std::holds_alternative<Cache *>(CrossAnalysisCache) + ? std::get<Cache *>(CrossAnalysisCache) + : std::get<std::unique_ptr<Cache>>(CrossAnalysisCache).get(); + } + const Stmt &Stm; ASTContext &Context; - llvm::DenseMap<const FunctionDecl *, - std::unique_ptr<FunctionParmMutationAnalyzer>> - FuncParmAnalyzer; + std::variant<Cache *, std::unique_ptr<Cache>> CrossAnalysisCache; ResultMap Results; ResultMap PointeeResults; }; @@ -80,7 +103,10 @@ class ExprMutationAnalyzer { // params. class FunctionParmMutationAnalyzer { public: - FunctionParmMutationAnalyzer(const FunctionDecl &Func, ASTContext &Context); + FunctionParmMutationAnalyzer(const FunctionDecl &Func, ASTContext &Context) + : FunctionParmMutationAnalyzer(Func, Context, nullptr) {} + FunctionParmMutationAnalyzer(const FunctionDecl &Func, ASTContext &Context, + ExprMutationAnalyzer::Cache *CrossAnalysisCache); bool isMutated(const ParmVarDecl *Parm) { return findMutation(Parm) != nullptr; diff --git a/clang/lib/Analysis/ExprMutationAnalyzer.cpp b/clang/lib/Analysis/ExprMutationAnalyzer.cpp index bb042760d297a7..dba6f2a3c02112 100644 --- a/clang/lib/Analysis/ExprMutationAnalyzer.cpp +++ b/clang/lib/Analysis/ExprMutationAnalyzer.cpp @@ -638,9 +638,10 @@ const Stmt *ExprMutationAnalyzer::findFunctionArgMutation(const Expr *Exp) { if (!RefType->getPointeeType().getQualifiers() && RefType->getPointeeType()->getAs<TemplateTypeParmType>()) { std::unique_ptr<FunctionParmMutationAnalyzer> &Analyzer = - FuncParmAnalyzer[Func]; + getCache()->FuncParmAnalyzer[Func]; if (!Analyzer) - Analyzer.reset(new FunctionParmMutationAnalyzer(*Func, Context)); + Analyzer.reset( + new FunctionParmMutationAnalyzer(*Func, Context, getCache())); if (Analyzer->findMutation(Parm)) return Exp; continue; @@ -653,13 +654,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::Cache *CrossAnalysisCache) + : BodyAnalyzer(*Func.getBody(), Context, CrossAnalysisCache) { 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 InitAnalyzer(*Init->getInit(), Context, + CrossAnalysisCache); for (const ParmVarDecl *Parm : Ctor->parameters()) { if (Results.contains(Parm)) continue; @@ -675,11 +678,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) { `````````` </details> https://github.com/llvm/llvm-project/pull/87954 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
