https://github.com/HerrCai0907 updated
https://github.com/llvm/llvm-project/pull/87954
>From 19f66851204547232d586288fba79d8770837350 Mon Sep 17 00:00:00 2001
From: Congcong Cai <congcongcai0...@163.com>
Date: Mon, 8 Apr 2024 09:20:58 +0800
Subject: [PATCH 1/4] [clang analysis] ExprMutationAnalyzer support recursive
forwarding reference
Partialy fixes: #60895
---
.../Analysis/Analyses/ExprMutationAnalyzer.h | 40 +++++++++++++++----
clang/lib/Analysis/ExprMutationAnalyzer.cpp | 22 ++++++----
.../Analysis/ExprMutationAnalyzerTest.cpp | 30 ++++++++++++++
3 files changed, 77 insertions(+), 15 deletions(-)
diff --git a/clang/include/clang/Analysis/Analyses/ExprMutationAnalyzer.h
b/clang/include/clang/Analysis/Analyses/ExprMutationAnalyzer.h
index 1ceef944fbc34e..af6106fe303afd 100644
--- a/clang/include/clang/Analysis/Analyses/ExprMutationAnalyzer.h
+++ b/clang/include/clang/Analysis/Analyses/ExprMutationAnalyzer.h
@@ -8,11 +8,10 @@
#ifndef LLVM_CLANG_ANALYSIS_ANALYSES_EXPRMUTATIONANALYZER_H
#define LLVM_CLANG_ANALYSIS_ANALYSES_EXPRMUTATIONANALYZER_H
-#include <type_traits>
-
#include "clang/AST/AST.h"
#include "clang/ASTMatchers/ASTMatchers.h"
#include "llvm/ADT/DenseMap.h"
+#include <variant>
namespace clang {
@@ -22,8 +21,15 @@ class FunctionParmMutationAnalyzer;
/// a given statement.
class ExprMutationAnalyzer {
public:
+ friend class FunctionParmMutationAnalyzer;
+ struct Cache {
+ llvm::DenseMap<const FunctionDecl *,
+ std::unique_ptr<FunctionParmMutationAnalyzer>>
+ FuncParmAnalyzer;
+ };
+
ExprMutationAnalyzer(const Stmt &Stm, ASTContext &Context)
- : Stm(Stm), Context(Context) {}
+ : ExprMutationAnalyzer(Stm, Context, nullptr) {}
bool isMutated(const Expr *Exp) { return findMutation(Exp) != nullptr; }
bool isMutated(const Decl *Dec) { return findMutation(Dec) != nullptr; }
@@ -45,6 +51,19 @@ class ExprMutationAnalyzer {
using MutationFinder = const Stmt *(ExprMutationAnalyzer::*)(const Expr *);
using ResultMap = llvm::DenseMap<const Expr *, const Stmt *>;
+ ExprMutationAnalyzer(const Stmt &Stm, ASTContext &Context, Cache
*ParentCache)
+ : Stm(Stm), Context(Context) {
+ if (ParentCache != nullptr) {
+ CrossAnalysisCache = ParentCache;
+ } else {
+ CrossAnalysisCache = std::make_unique<Cache>();
+ }
+ }
+ ExprMutationAnalyzer(const Stmt &Stm, ASTContext &Context,
+ std::unique_ptr<Cache> CrossAnalysisCache)
+ : Stm(Stm), Context(Context),
+ CrossAnalysisCache(std::move(CrossAnalysisCache)) {}
+
const Stmt *findMutationMemoized(const Expr *Exp,
llvm::ArrayRef<MutationFinder> Finders,
ResultMap &MemoizedResults);
@@ -67,11 +86,15 @@ class ExprMutationAnalyzer {
const Stmt *findReferenceMutation(const Expr *Exp);
const Stmt *findFunctionArgMutation(const Expr *Exp);
+ Cache *getCache() {
+ return std::holds_alternative<Cache *>(CrossAnalysisCache)
+ ? std::get<Cache *>(CrossAnalysisCache)
+ : std::get<std::unique_ptr<Cache>>(CrossAnalysisCache).get();
+ }
+
const Stmt &Stm;
ASTContext &Context;
- llvm::DenseMap<const FunctionDecl *,
- std::unique_ptr<FunctionParmMutationAnalyzer>>
- FuncParmAnalyzer;
+ std::variant<Cache *, std::unique_ptr<Cache>> CrossAnalysisCache;
ResultMap Results;
ResultMap PointeeResults;
};
@@ -80,7 +103,10 @@ class ExprMutationAnalyzer {
// params.
class FunctionParmMutationAnalyzer {
public:
- FunctionParmMutationAnalyzer(const FunctionDecl &Func, ASTContext &Context);
+ FunctionParmMutationAnalyzer(const FunctionDecl &Func, ASTContext &Context)
+ : FunctionParmMutationAnalyzer(Func, Context, nullptr) {}
+ FunctionParmMutationAnalyzer(const FunctionDecl &Func, ASTContext &Context,
+ ExprMutationAnalyzer::Cache
*CrossAnalysisCache);
bool isMutated(const ParmVarDecl *Parm) {
return findMutation(Parm) != nullptr;
diff --git a/clang/lib/Analysis/ExprMutationAnalyzer.cpp
b/clang/lib/Analysis/ExprMutationAnalyzer.cpp
index bb042760d297a7..dba6f2a3c02112 100644
--- a/clang/lib/Analysis/ExprMutationAnalyzer.cpp
+++ b/clang/lib/Analysis/ExprMutationAnalyzer.cpp
@@ -638,9 +638,10 @@ const Stmt
*ExprMutationAnalyzer::findFunctionArgMutation(const Expr *Exp) {
if (!RefType->getPointeeType().getQualifiers() &&
RefType->getPointeeType()->getAs<TemplateTypeParmType>()) {
std::unique_ptr<FunctionParmMutationAnalyzer> &Analyzer =
- FuncParmAnalyzer[Func];
+ getCache()->FuncParmAnalyzer[Func];
if (!Analyzer)
- Analyzer.reset(new FunctionParmMutationAnalyzer(*Func, Context));
+ Analyzer.reset(
+ new FunctionParmMutationAnalyzer(*Func, Context, getCache()));
if (Analyzer->findMutation(Parm))
return Exp;
continue;
@@ -653,13 +654,15 @@ const Stmt
*ExprMutationAnalyzer::findFunctionArgMutation(const Expr *Exp) {
}
FunctionParmMutationAnalyzer::FunctionParmMutationAnalyzer(
- const FunctionDecl &Func, ASTContext &Context)
- : BodyAnalyzer(*Func.getBody(), Context) {
+ const FunctionDecl &Func, ASTContext &Context,
+ ExprMutationAnalyzer::Cache *CrossAnalysisCache)
+ : BodyAnalyzer(*Func.getBody(), Context, CrossAnalysisCache) {
if (const auto *Ctor = dyn_cast<CXXConstructorDecl>(&Func)) {
// CXXCtorInitializer might also mutate Param but they're not part of
// function body, check them eagerly here since they're typically trivial.
for (const CXXCtorInitializer *Init : Ctor->inits()) {
- ExprMutationAnalyzer InitAnalyzer(*Init->getInit(), Context);
+ ExprMutationAnalyzer InitAnalyzer(*Init->getInit(), Context,
+ CrossAnalysisCache);
for (const ParmVarDecl *Parm : Ctor->parameters()) {
if (Results.contains(Parm))
continue;
@@ -675,11 +678,14 @@ FunctionParmMutationAnalyzer::findMutation(const
ParmVarDecl *Parm) {
const auto Memoized = Results.find(Parm);
if (Memoized != Results.end())
return Memoized->second;
-
+ // To handle call A -> call B -> call A. Assume parameters of A is not
mutated
+ // before analyzing parameters of A. Then when analyzing the second "call A",
+ // FunctionParmMutationAnalyzer can use this memoized value to avoid infinite
+ // recursion.
+ Results[Parm] = nullptr;
if (const Stmt *S = BodyAnalyzer.findMutation(Parm))
return Results[Parm] = S;
-
- return Results[Parm] = nullptr;
+ return Results[Parm];
}
} // namespace clang
diff --git a/clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp
b/clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp
index f58ce4aebcbfc8..9c1dc1a76db63d 100644
--- a/clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp
+++ b/clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp
@@ -977,6 +977,36 @@ TEST(ExprMutationAnalyzerTest, FollowFuncArgModified) {
"void f() { int x; g(x); }");
Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("g(x)"));
+
+ AST = buildASTFromCode(
+ StdRemoveReference + StdForward +
+ "template <class T> void f1(T &&a);"
+ "template <class T> void f2(T &&a);"
+ "template <class T> void f1(T &&a) { f2<T>(std::forward<T>(a)); }"
+ "template <class T> void f2(T &&a) { f1<T>(std::forward<T>(a)); }"
+ "void f() { int x; f1(x); }");
+ Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+ EXPECT_FALSE(isMutated(Results, AST.get()));
+
+ AST = buildASTFromCode(
+ StdRemoveReference + StdForward +
+ "template <class T> void f1(T &&a);"
+ "template <class T> void f2(T &&a);"
+ "template <class T> void f1(T &&a) { f2<T>(std::forward<T>(a)); }"
+ "template <class T> void f2(T &&a) { f1<T>(std::forward<T>(a)); a++; }"
+ "void f() { int x; f1(x); }");
+ Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+ EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("f1(x)"));
+
+ AST = buildASTFromCode(
+ StdRemoveReference + StdForward +
+ "template <class T> void f1(T &&a);"
+ "template <class T> void f2(T &&a);"
+ "template <class T> void f1(T &&a) { f2<T>(std::forward<T>(a)); a++; }"
+ "template <class T> void f2(T &&a) { f1<T>(std::forward<T>(a)); }"
+ "void f() { int x; f1(x); }");
+ Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+ EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("f1(x)"));
}
TEST(ExprMutationAnalyzerTest, FollowFuncArgNotModified) {
>From c5d0580b736eafc20c081e9b36f48ea606f5df11 Mon Sep 17 00:00:00 2001
From: Congcong Cai <congcongcai0...@163.com>
Date: Mon, 8 Apr 2024 12:11:45 +0800
Subject: [PATCH 2/4] add release-note
---
clang-tools-extra/docs/ReleaseNotes.rst | 4 ++++
.../checkers/misc/const-correctness-templates.cpp | 15 +++++++++++++++
2 files changed, 19 insertions(+)
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst
b/clang-tools-extra/docs/ReleaseNotes.rst
index 309b844615a121..19ab5bc4eae882 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -214,6 +214,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
>From 111009710d56d1dd514c8458063f3e033aee152b Mon Sep 17 00:00:00 2001
From: Congcong Cai <congcongcai0...@163.com>
Date: Wed, 10 Apr 2024 14:08:58 +0800
Subject: [PATCH 3/4] use shared_ptr
---
.../Analysis/Analyses/ExprMutationAnalyzer.h | 28 ++++++-------------
clang/lib/Analysis/ExprMutationAnalyzer.cpp | 8 +++---
2 files changed, 12 insertions(+), 24 deletions(-)
diff --git a/clang/include/clang/Analysis/Analyses/ExprMutationAnalyzer.h
b/clang/include/clang/Analysis/Analyses/ExprMutationAnalyzer.h
index af6106fe303afd..ae62135a075717 100644
--- a/clang/include/clang/Analysis/Analyses/ExprMutationAnalyzer.h
+++ b/clang/include/clang/Analysis/Analyses/ExprMutationAnalyzer.h
@@ -29,7 +29,7 @@ class ExprMutationAnalyzer {
};
ExprMutationAnalyzer(const Stmt &Stm, ASTContext &Context)
- : ExprMutationAnalyzer(Stm, Context, nullptr) {}
+ : ExprMutationAnalyzer(Stm, Context, std::make_shared<Cache>()) {}
bool isMutated(const Expr *Exp) { return findMutation(Exp) != nullptr; }
bool isMutated(const Decl *Dec) { return findMutation(Dec) != nullptr; }
@@ -51,16 +51,8 @@ class ExprMutationAnalyzer {
using MutationFinder = const Stmt *(ExprMutationAnalyzer::*)(const Expr *);
using ResultMap = llvm::DenseMap<const Expr *, const Stmt *>;
- ExprMutationAnalyzer(const Stmt &Stm, ASTContext &Context, Cache
*ParentCache)
- : Stm(Stm), Context(Context) {
- if (ParentCache != nullptr) {
- CrossAnalysisCache = ParentCache;
- } else {
- CrossAnalysisCache = std::make_unique<Cache>();
- }
- }
ExprMutationAnalyzer(const Stmt &Stm, ASTContext &Context,
- std::unique_ptr<Cache> CrossAnalysisCache)
+ std::shared_ptr<Cache> CrossAnalysisCache)
: Stm(Stm), Context(Context),
CrossAnalysisCache(std::move(CrossAnalysisCache)) {}
@@ -86,15 +78,9 @@ class ExprMutationAnalyzer {
const Stmt *findReferenceMutation(const Expr *Exp);
const Stmt *findFunctionArgMutation(const Expr *Exp);
- Cache *getCache() {
- return std::holds_alternative<Cache *>(CrossAnalysisCache)
- ? std::get<Cache *>(CrossAnalysisCache)
- : std::get<std::unique_ptr<Cache>>(CrossAnalysisCache).get();
- }
-
const Stmt &Stm;
ASTContext &Context;
- std::variant<Cache *, std::unique_ptr<Cache>> CrossAnalysisCache;
+ std::shared_ptr<Cache> CrossAnalysisCache;
ResultMap Results;
ResultMap PointeeResults;
};
@@ -104,9 +90,11 @@ class ExprMutationAnalyzer {
class FunctionParmMutationAnalyzer {
public:
FunctionParmMutationAnalyzer(const FunctionDecl &Func, ASTContext &Context)
- : FunctionParmMutationAnalyzer(Func, Context, nullptr) {}
- FunctionParmMutationAnalyzer(const FunctionDecl &Func, ASTContext &Context,
- ExprMutationAnalyzer::Cache
*CrossAnalysisCache);
+ : FunctionParmMutationAnalyzer(
+ Func, Context, std::make_shared<ExprMutationAnalyzer::Cache>()) {}
+ FunctionParmMutationAnalyzer(
+ const FunctionDecl &Func, ASTContext &Context,
+ std::shared_ptr<ExprMutationAnalyzer::Cache> CrossAnalysisCache);
bool isMutated(const ParmVarDecl *Parm) {
return findMutation(Parm) != nullptr;
diff --git a/clang/lib/Analysis/ExprMutationAnalyzer.cpp
b/clang/lib/Analysis/ExprMutationAnalyzer.cpp
index dba6f2a3c02112..503ca4bea99e50 100644
--- a/clang/lib/Analysis/ExprMutationAnalyzer.cpp
+++ b/clang/lib/Analysis/ExprMutationAnalyzer.cpp
@@ -638,10 +638,10 @@ const Stmt
*ExprMutationAnalyzer::findFunctionArgMutation(const Expr *Exp) {
if (!RefType->getPointeeType().getQualifiers() &&
RefType->getPointeeType()->getAs<TemplateTypeParmType>()) {
std::unique_ptr<FunctionParmMutationAnalyzer> &Analyzer =
- getCache()->FuncParmAnalyzer[Func];
+ CrossAnalysisCache->FuncParmAnalyzer[Func];
if (!Analyzer)
- Analyzer.reset(
- new FunctionParmMutationAnalyzer(*Func, Context, getCache()));
+ Analyzer.reset(new FunctionParmMutationAnalyzer(*Func, Context,
+ CrossAnalysisCache));
if (Analyzer->findMutation(Parm))
return Exp;
continue;
@@ -655,7 +655,7 @@ const Stmt
*ExprMutationAnalyzer::findFunctionArgMutation(const Expr *Exp) {
FunctionParmMutationAnalyzer::FunctionParmMutationAnalyzer(
const FunctionDecl &Func, ASTContext &Context,
- ExprMutationAnalyzer::Cache *CrossAnalysisCache)
+ std::shared_ptr<ExprMutationAnalyzer::Cache> CrossAnalysisCache)
: BodyAnalyzer(*Func.getBody(), Context, CrossAnalysisCache) {
if (const auto *Ctor = dyn_cast<CXXConstructorDecl>(&Func)) {
// CXXCtorInitializer might also mutate Param but they're not part of
>From e9e5931f7771ac7b5ab3d7cb2dde2d70321e940c Mon Sep 17 00:00:00 2001
From: Congcong Cai <congcongcai0...@163.com>
Date: Mon, 15 Apr 2024 16:06:03 +0800
Subject: [PATCH 4/4] use SmallDenseMap
---
clang/include/clang/Analysis/Analyses/ExprMutationAnalyzer.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/clang/include/clang/Analysis/Analyses/ExprMutationAnalyzer.h
b/clang/include/clang/Analysis/Analyses/ExprMutationAnalyzer.h
index ae62135a075717..c4e5d0badb8e58 100644
--- a/clang/include/clang/Analysis/Analyses/ExprMutationAnalyzer.h
+++ b/clang/include/clang/Analysis/Analyses/ExprMutationAnalyzer.h
@@ -23,8 +23,8 @@ class ExprMutationAnalyzer {
public:
friend class FunctionParmMutationAnalyzer;
struct Cache {
- llvm::DenseMap<const FunctionDecl *,
- std::unique_ptr<FunctionParmMutationAnalyzer>>
+ llvm::SmallDenseMap<const FunctionDecl *,
+ std::unique_ptr<FunctionParmMutationAnalyzer>>
FuncParmAnalyzer;
};
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
