Skip to content

[clang analysis] ExprMutationAnalyzer support recursive forwarding reference #88843

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Apr 17, 2024
Merged

[clang analysis] ExprMutationAnalyzer support recursive forwarding reference #88843

merged 1 commit into from
Apr 17, 2024

Conversation

HerrCai0907
Copy link
Contributor

@HerrCai0907 HerrCai0907 commented Apr 16, 2024 •
edited
Loading

Reapply for #88765.
Partially fixes: #60895.

@llvmbot llvmbot added clang Clang issues not falling into any other category clang-tools-extra clang-tidy clang:analysis labels Apr 16, 2024
@llvmbot
Copy link
Member

llvmbot commented Apr 16, 2024 •
edited
Loading

@llvm/pr-subscribers-clang-tidy

@llvm/pr-subscribers-clang-tools-extra

Author: Congcong Cai (HerrCai0907)

Changes

Partialy fixes: #60895

Patch is 25.20 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/88843.diff

7 Files Affected:

  • (modified) clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp (+5-5)
  • (modified) clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.h (+1-2)
  • (modified) clang-tools-extra/docs/ReleaseNotes.rst (+4)
  • (modified) clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-templates.cpp (+15)
  • (modified) clang/include/clang/Analysis/Analyses/ExprMutationAnalyzer.h (+93-43)
  • (modified) clang/lib/Analysis/ExprMutationAnalyzer.cpp (+72-53)
  • (modified) clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp (+30)
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 *
...
[truncated]

@llvmbot
Copy link
Member

llvmbot commented Apr 16, 2024

@llvm/pr-subscribers-clang

Author: Congcong Cai (HerrCai0907)

Changes

Partialy fixes: #60895

Patch is 25.20 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/88843.diff

7 Files Affected:

  • (modified) clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp (+5-5)
  • (modified) clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.h (+1-2)
  • (modified) clang-tools-extra/docs/ReleaseNotes.rst (+4)
  • (modified) clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-templates.cpp (+15)
  • (modified) clang/include/clang/Analysis/Analyses/ExprMutationAnalyzer.h (+93-43)
  • (modified) clang/lib/Analysis/ExprMutationAnalyzer.cpp (+72-53)
  • (modified) clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp (+30)
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 *
...
[truncated]

@llvmbot
Copy link
Member

llvmbot commented Apr 16, 2024

@llvm/pr-subscribers-clang-analysis

Author: Congcong Cai (HerrCai0907)

Changes

Partialy fixes: #60895

Patch is 25.20 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/88843.diff

7 Files Affected:

  • (modified) clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp (+5-5)
  • (modified) clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.h (+1-2)
  • (modified) clang-tools-extra/docs/ReleaseNotes.rst (+4)
  • (modified) clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-templates.cpp (+15)
  • (modified) clang/include/clang/Analysis/Analyses/ExprMutationAnalyzer.h (+93-43)
  • (modified) clang/lib/Analysis/ExprMutationAnalyzer.cpp (+72-53)
  • (modified) clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp (+30)
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 *
...
[truncated]

@HerrCai0907 HerrCai0907 requested a review from PiotrZSL April 16, 2024 05:15
PiotrZSL
PiotrZSL approved these changes Apr 16, 2024
View reviewed changes
Copy link
Member

@PiotrZSL PiotrZSL left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@HerrCai0907 HerrCai0907 merged commit f40f4fc into llvm:main Apr 17, 2024
10 checks passed
@HerrCai0907 HerrCai0907 deleted the fix/expr-mutation branch April 17, 2024 01:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@PiotrZSL PiotrZSL PiotrZSL approved these changes

Assignees
No one assigned
Labels
clang:analysis clang Clang issues not falling into any other category clang-tidy clang-tools-extra
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Clang-Tidy] misc-const-correctness crash due to seemingly infinite recursion
3 participants
@HerrCai0907 @llvmbot @PiotrZSL