diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 7095c564444fe..4dfbd8ca49ab9 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -221,10 +221,6 @@ Changes in existing checks ` check by replacing the local option `HeaderFileExtensions` by the global option of the same name. -- Improved :doc:`misc-const-correctness - ` check by avoiding infinite recursion - for recursive forwarding reference. - - Improved :doc:`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 248374a71dd40..9da468128743e 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 void f1(T &&a); -template void f2(T &&a); -template void f1(T &&a) { f2(a); } -template void f2(T &&a) { f1(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 c4e5d0badb8e5..1ceef944fbc34 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 + #include "clang/AST/AST.h" #include "clang/ASTMatchers/ASTMatchers.h" #include "llvm/ADT/DenseMap.h" -#include namespace clang { @@ -21,15 +22,8 @@ class FunctionParmMutationAnalyzer; /// a given statement. class ExprMutationAnalyzer { public: - friend class FunctionParmMutationAnalyzer; - struct Cache { - llvm::SmallDenseMap> - FuncParmAnalyzer; - }; - ExprMutationAnalyzer(const Stmt &Stm, ASTContext &Context) - : ExprMutationAnalyzer(Stm, Context, std::make_shared()) {} + : 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; - ExprMutationAnalyzer(const Stmt &Stm, ASTContext &Context, - std::shared_ptr CrossAnalysisCache) - : Stm(Stm), Context(Context), - CrossAnalysisCache(std::move(CrossAnalysisCache)) {} - const Stmt *findMutationMemoized(const Expr *Exp, llvm::ArrayRef Finders, ResultMap &MemoizedResults); @@ -80,7 +69,9 @@ class ExprMutationAnalyzer { const Stmt &Stm; ASTContext &Context; - std::shared_ptr CrossAnalysisCache; + llvm::DenseMap> + 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()) {} - FunctionParmMutationAnalyzer( - const FunctionDecl &Func, ASTContext &Context, - std::shared_ptr 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 503ca4bea99e5..bb042760d297a 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()) { std::unique_ptr &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 CrossAnalysisCache) - : BodyAnalyzer(*Func.getBody(), Context, CrossAnalysisCache) { + const FunctionDecl &Func, ASTContext &Context) + : BodyAnalyzer(*Func.getBody(), Context) { if (const auto *Ctor = dyn_cast(&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 9c1dc1a76db63..f58ce4aebcbfc 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 void f1(T &&a);" - "template void f2(T &&a);" - "template void f1(T &&a) { f2(std::forward(a)); }" - "template void f2(T &&a) { f1(std::forward(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 void f1(T &&a);" - "template void f2(T &&a);" - "template void f1(T &&a) { f2(std::forward(a)); }" - "template void f2(T &&a) { f1(std::forward(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 void f1(T &&a);" - "template void f2(T &&a);" - "template void f1(T &&a) { f2(std::forward(a)); a++; }" - "template void f2(T &&a) { f1(std::forward(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) {