[clang analysis] ExprMutationAnalyzer support recursive forwarding re…

…ference (#88843) Reapply for #88765. Partially fixes: #60895.
llvm · Apr 17, 2024 · f40f4fc · f40f4fc
1 parent 0af8cae
commit f40f4fc
Show file tree

Hide file tree

Showing 7 changed files with 220 additions and 103 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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,75 +19,127 @@ 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;
   }
   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;
   }
   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