https://github.com/fmayer created https://github.com/llvm/llvm-project/pull/88765
Reverts llvm/llvm-project#87954 Broke sanitizer bots, e.g. https://lab.llvm.org/buildbot/#/builders/239/builds/6587/steps/10/logs/stdio >From 82b9a06f73df5301ffd950775055304124f63e02 Mon Sep 17 00:00:00 2001 From: Florian Mayer <florian.ma...@bitsrc.org> Date: Mon, 15 Apr 2024 10:46:21 -0700 Subject: [PATCH] =?UTF-8?q?Revert=20"[clang=20analysis]=20ExprMutationAnal?= =?UTF-8?q?yzer=20avoid=20infinite=20recursion=20for=20re=E2=80=A6"?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit 8095b9ce6bf5831a14c72028920708f38d13d0c3. --- clang-tools-extra/docs/ReleaseNotes.rst | 4 --- .../misc/const-correctness-templates.cpp | 15 ---------- .../Analysis/Analyses/ExprMutationAnalyzer.h | 28 +++++------------ clang/lib/Analysis/ExprMutationAnalyzer.cpp | 22 +++++--------- .../Analysis/ExprMutationAnalyzerTest.cpp | 30 ------------------- 5 files changed, 15 insertions(+), 84 deletions(-) diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 7095c564444fe6..4dfbd8ca49ab9b 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -221,10 +221,6 @@ 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 248374a71dd40b..9da468128743e9 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,18 +58,3 @@ 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 c4e5d0badb8e58..1ceef944fbc34e 100644 --- a/clang/include/clang/Analysis/Analyses/ExprMutationAnalyzer.h +++ b/clang/include/clang/Analysis/Analyses/ExprMutationAnalyzer.h @@ -8,10 +8,11 @@ #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 { @@ -21,15 +22,8 @@ class FunctionParmMutationAnalyzer; /// a given statement. class ExprMutationAnalyzer { public: - friend class FunctionParmMutationAnalyzer; - struct Cache { - llvm::SmallDenseMap<const FunctionDecl *, - std::unique_ptr<FunctionParmMutationAnalyzer>> - FuncParmAnalyzer; - }; - ExprMutationAnalyzer(const Stmt &Stm, ASTContext &Context) - : ExprMutationAnalyzer(Stm, Context, std::make_shared<Cache>()) {} + : Stm(Stm), Context(Context) {} bool isMutated(const Expr *Exp) { return findMutation(Exp) != nullptr; } bool isMutated(const Decl *Dec) { return findMutation(Dec) != nullptr; } @@ -51,11 +45,6 @@ class ExprMutationAnalyzer { using MutationFinder = const Stmt *(ExprMutationAnalyzer::*)(const Expr *); using ResultMap = llvm::DenseMap<const Expr *, const Stmt *>; - ExprMutationAnalyzer(const Stmt &Stm, ASTContext &Context, - std::shared_ptr<Cache> CrossAnalysisCache) - : Stm(Stm), Context(Context), - CrossAnalysisCache(std::move(CrossAnalysisCache)) {} - const Stmt *findMutationMemoized(const Expr *Exp, llvm::ArrayRef<MutationFinder> Finders, ResultMap &MemoizedResults); @@ -80,7 +69,9 @@ class ExprMutationAnalyzer { const Stmt &Stm; ASTContext &Context; - std::shared_ptr<Cache> CrossAnalysisCache; + llvm::DenseMap<const FunctionDecl *, + std::unique_ptr<FunctionParmMutationAnalyzer>> + FuncParmAnalyzer; ResultMap Results; ResultMap PointeeResults; }; @@ -89,12 +80,7 @@ class ExprMutationAnalyzer { // params. class FunctionParmMutationAnalyzer { public: - FunctionParmMutationAnalyzer(const FunctionDecl &Func, ASTContext &Context) - : FunctionParmMutationAnalyzer( - Func, Context, std::make_shared<ExprMutationAnalyzer::Cache>()) {} - FunctionParmMutationAnalyzer( - const FunctionDecl &Func, ASTContext &Context, - std::shared_ptr<ExprMutationAnalyzer::Cache> CrossAnalysisCache); + FunctionParmMutationAnalyzer(const FunctionDecl &Func, ASTContext &Context); bool isMutated(const ParmVarDecl *Parm) { return findMutation(Parm) != nullptr; diff --git a/clang/lib/Analysis/ExprMutationAnalyzer.cpp b/clang/lib/Analysis/ExprMutationAnalyzer.cpp index 503ca4bea99e50..bb042760d297a7 100644 --- a/clang/lib/Analysis/ExprMutationAnalyzer.cpp +++ b/clang/lib/Analysis/ExprMutationAnalyzer.cpp @@ -638,10 +638,9 @@ const Stmt *ExprMutationAnalyzer::findFunctionArgMutation(const Expr *Exp) { if (!RefType->getPointeeType().getQualifiers() && RefType->getPointeeType()->getAs<TemplateTypeParmType>()) { std::unique_ptr<FunctionParmMutationAnalyzer> &Analyzer = - CrossAnalysisCache->FuncParmAnalyzer[Func]; + FuncParmAnalyzer[Func]; if (!Analyzer) - Analyzer.reset(new FunctionParmMutationAnalyzer(*Func, Context, - CrossAnalysisCache)); + Analyzer.reset(new FunctionParmMutationAnalyzer(*Func, Context)); if (Analyzer->findMutation(Parm)) return Exp; continue; @@ -654,15 +653,13 @@ const Stmt *ExprMutationAnalyzer::findFunctionArgMutation(const Expr *Exp) { } FunctionParmMutationAnalyzer::FunctionParmMutationAnalyzer( - const FunctionDecl &Func, ASTContext &Context, - std::shared_ptr<ExprMutationAnalyzer::Cache> CrossAnalysisCache) - : BodyAnalyzer(*Func.getBody(), Context, CrossAnalysisCache) { + const FunctionDecl &Func, ASTContext &Context) + : BodyAnalyzer(*Func.getBody(), Context) { 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, - CrossAnalysisCache); + ExprMutationAnalyzer InitAnalyzer(*Init->getInit(), Context); for (const ParmVarDecl *Parm : Ctor->parameters()) { if (Results.contains(Parm)) continue; @@ -678,14 +675,11 @@ 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]; + + return Results[Parm] = nullptr; } } // namespace clang diff --git a/clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp b/clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp index 9c1dc1a76db63d..f58ce4aebcbfc8 100644 --- a/clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp +++ b/clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp @@ -977,36 +977,6 @@ 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
