https://github.com/HerrCai0907 updated
https://github.com/llvm/llvm-project/pull/66846
>From 6d8e737ab1a883371a7491b676e1e202a087701f Mon Sep 17 00:00:00 2001
From: Congcong Cai <congcongcai0...@163.com>
Date: Tue, 19 Sep 2023 16:15:20 +0800
Subject: [PATCH 1/4] [clang-analysis]Fix false positive in mutation check when
using pointer to member function
Fixes: #66204
---
clang-tools-extra/docs/ReleaseNotes.rst | 4 +++
.../misc/const-correctness-values.cpp | 13 ++++++++
clang/docs/ReleaseNotes.rst | 2 ++
clang/lib/Analysis/ExprMutationAnalyzer.cpp | 22 ++++++++++++--
.../Analysis/ExprMutationAnalyzerTest.cpp | 30 +++++++++++++++++++
5 files changed, 68 insertions(+), 3 deletions(-)
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst
b/clang-tools-extra/docs/ReleaseNotes.rst
index a2cde526a8c04d9..4370059653dfc09 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -222,6 +222,10 @@ Changes in existing checks
<clang-tidy/checks/llvm/namespace-comment>` check to provide fixes for
``inline`` namespaces in the same format as :program:`clang-format`.
+- Improved :doc:`misc-const-correctness
+ <clang-tidy/checks/misc/const-correctness>` check to avoid false positive
when
+ using pointer to member function.
+
- Improved :doc:`misc-include-cleaner
<clang-tidy/checks/misc/include-cleaner>` check by adding option
`DeduplicateFindings` to output one finding per symbol occurrence.
diff --git
a/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-values.cpp
b/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-values.cpp
index 186e3cf5a179b24..cb6bfcc1dccba39 100644
---
a/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-values.cpp
+++
b/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-values.cpp
@@ -976,3 +976,16 @@ void auto_usage_variants() {
auto &auto_td1 = auto_td0;
auto *auto_td2 = &auto_td0;
}
+
+using PointerToMemberFunction = int (Value::*)();
+void member_pointer(Value &x, PointerToMemberFunction m) {
+ Value &member_pointer_tmp = x;
+ (member_pointer_tmp.*m)();
+}
+
+using PointerToConstMemberFunction = int (Value::*)() const;
+void member_pointer_const(Value &x, PointerToConstMemberFunction m) {
+ Value &member_pointer_tmp = x;
+ // CHECK-MESSAGES:[[@LINE-1]]:3: warning: variable 'member_pointer_tmp' of
type 'Value &' can be declared 'const'
+ (member_pointer_tmp.*m)();
+}
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 5b2a6349a5b15bf..1594e4d012a4aff 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -384,6 +384,8 @@ Static Analyzer
bitwise shift operators produce undefined behavior (because some operand is
negative or too large).
+- Fix false positive in mutation check when using pointer to member function.
+
.. _release-notes-sanitizers:
Sanitizers
diff --git a/clang/lib/Analysis/ExprMutationAnalyzer.cpp
b/clang/lib/Analysis/ExprMutationAnalyzer.cpp
index 90803830ff41949..fcd909be88c999b 100644
--- a/clang/lib/Analysis/ExprMutationAnalyzer.cpp
+++ b/clang/lib/Analysis/ExprMutationAnalyzer.cpp
@@ -100,6 +100,20 @@ AST_MATCHER(CXXTypeidExpr, isPotentiallyEvaluated) {
return Node.isPotentiallyEvaluated();
}
+AST_MATCHER(CXXMemberCallExpr, isConstCallee) {
+ const auto *CalleeDecl = Node.getCalleeDecl();
+ const auto *VD = dyn_cast_or_null<ValueDecl>(CalleeDecl);
+ if (!VD)
+ return false;
+ const auto T = VD->getType().getCanonicalType();
+ const auto *MPT = dyn_cast<MemberPointerType>(T);
+ const auto *FPT = MPT ? cast<FunctionProtoType>(MPT->getPointeeType())
+ : dyn_cast<FunctionProtoType>(T);
+ if (!FPT)
+ return false;
+ return FPT->isConst();
+}
+
AST_MATCHER_P(GenericSelectionExpr, hasControllingExpr,
ast_matchers::internal::Matcher<Expr>, InnerMatcher) {
if (Node.isTypePredicate())
@@ -274,8 +288,8 @@ const Stmt *ExprMutationAnalyzer::findDirectMutation(const
Expr *Exp) {
const auto NonConstMethod = cxxMethodDecl(unless(isConst()));
const auto AsNonConstThis = expr(anyOf(
- cxxMemberCallExpr(callee(NonConstMethod),
- on(canResolveToExpr(equalsNode(Exp)))),
+ cxxMemberCallExpr(on(canResolveToExpr(equalsNode(Exp))),
+ unless(isConstCallee())),
cxxOperatorCallExpr(callee(NonConstMethod),
hasArgument(0, canResolveToExpr(equalsNode(Exp)))),
// In case of a templated type, calling overloaded operators is not
@@ -391,7 +405,9 @@ const Stmt *ExprMutationAnalyzer::findMemberMutation(const
Expr *Exp) {
match(findAll(expr(anyOf(memberExpr(hasObjectExpression(
canResolveToExpr(equalsNode(Exp)))),
cxxDependentScopeMemberExpr(hasObjectExpression(
- canResolveToExpr(equalsNode(Exp))))))
+ canResolveToExpr(equalsNode(Exp)))),
+ binaryOperator(allOf(hasOperatorName(".*"),
+ hasLHS(equalsNode(Exp))))))
.bind(NodeID<Expr>::value)),
Stm, Context);
return findExprMutation(MemberExprs);
diff --git a/clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp
b/clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp
index b886259ef4d7709..96972401ebb5eda 100644
--- a/clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp
+++ b/clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp
@@ -284,6 +284,36 @@ TEST(ExprMutationAnalyzerTest, TypeDependentMemberCall) {
EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("x.push_back(T())"));
}
+TEST(ExprMutationAnalyzerTest, MemberPointerMemberCall) {
+ {
+ const auto AST =
+ buildASTFromCode("struct X {};"
+ "using T = int (X::*)();"
+ " void f(X &x, T m) { X &ref = x; (ref.*m)(); }");
+ const auto Results =
+ match(withEnclosingCompound(declRefTo("ref")), AST->getASTContext());
+ EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("(ref .* m)()"));
+ }
+ {
+ const auto AST = buildASTFromCode(
+ "struct X {};"
+ "using T = int (X::*)();"
+ " void f(X &x, T const m) { X &ref = x; (ref.*m)(); }");
+ const auto Results =
+ match(withEnclosingCompound(declRefTo("ref")), AST->getASTContext());
+ EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("(ref .* m)()"));
+ }
+ {
+ const auto AST =
+ buildASTFromCode("struct X {};"
+ "using T = int (X::*)() const;"
+ " void f(X &x, T m) { X &ref = x; (ref.*m)(); }");
+ const auto Results =
+ match(withEnclosingCompound(declRefTo("ref")), AST->getASTContext());
+ EXPECT_FALSE(isMutated(Results, AST.get()));
+ }
+}
+
// Section: overloaded operators
TEST(ExprMutationAnalyzerTest, NonConstOperator) {
>From a7894d680b2732430ac5ca2c2155391be710e91a Mon Sep 17 00:00:00 2001
From: Congcong Cai <congcongcai0...@163.com>
Date: Thu, 21 Sep 2023 14:26:27 +0800
Subject: [PATCH 2/4] not use auto
---
clang/lib/Analysis/ExprMutationAnalyzer.cpp | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/clang/lib/Analysis/ExprMutationAnalyzer.cpp
b/clang/lib/Analysis/ExprMutationAnalyzer.cpp
index fcd909be88c999b..7bbd721c379b7e0 100644
--- a/clang/lib/Analysis/ExprMutationAnalyzer.cpp
+++ b/clang/lib/Analysis/ExprMutationAnalyzer.cpp
@@ -101,11 +101,11 @@ AST_MATCHER(CXXTypeidExpr, isPotentiallyEvaluated) {
}
AST_MATCHER(CXXMemberCallExpr, isConstCallee) {
- const auto *CalleeDecl = Node.getCalleeDecl();
+ const Decl *CalleeDecl = Node.getCalleeDecl();
const auto *VD = dyn_cast_or_null<ValueDecl>(CalleeDecl);
if (!VD)
return false;
- const auto T = VD->getType().getCanonicalType();
+ const QualType T = VD->getType().getCanonicalType();
const auto *MPT = dyn_cast<MemberPointerType>(T);
const auto *FPT = MPT ? cast<FunctionProtoType>(MPT->getPointeeType())
: dyn_cast<FunctionProtoType>(T);
>From 857b247d61509a6f18d26b63cb2b76e34ace922b Mon Sep 17 00:00:00 2001
From: Congcong Cai <congcongcai0...@163.com>
Date: Mon, 25 Sep 2023 21:32:23 +0800
Subject: [PATCH 3/4] remove meaningless allOf
---
clang/lib/Analysis/ExprMutationAnalyzer.cpp | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/clang/lib/Analysis/ExprMutationAnalyzer.cpp
b/clang/lib/Analysis/ExprMutationAnalyzer.cpp
index 7bbd721c379b7e0..f2e1beb025423cf 100644
--- a/clang/lib/Analysis/ExprMutationAnalyzer.cpp
+++ b/clang/lib/Analysis/ExprMutationAnalyzer.cpp
@@ -406,8 +406,8 @@ const Stmt *ExprMutationAnalyzer::findMemberMutation(const
Expr *Exp) {
canResolveToExpr(equalsNode(Exp)))),
cxxDependentScopeMemberExpr(hasObjectExpression(
canResolveToExpr(equalsNode(Exp)))),
- binaryOperator(allOf(hasOperatorName(".*"),
- hasLHS(equalsNode(Exp))))))
+ binaryOperator(hasOperatorName(".*"),
+ hasLHS(equalsNode(Exp)))))
.bind(NodeID<Expr>::value)),
Stm, Context);
return findExprMutation(MemberExprs);
>From 3187fd37cb1263fb9cdb7274793aef56306e38b4 Mon Sep 17 00:00:00 2001
From: Congcong Cai <congcongcai0...@163.com>
Date: Mon, 25 Sep 2023 21:41:43 +0800
Subject: [PATCH 4/4] remove space in test code
---
.../unittests/Analysis/ExprMutationAnalyzerTest.cpp | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp
b/clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp
index 96972401ebb5eda..c0a398394093c48 100644
--- a/clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp
+++ b/clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp
@@ -289,16 +289,16 @@ TEST(ExprMutationAnalyzerTest, MemberPointerMemberCall) {
const auto AST =
buildASTFromCode("struct X {};"
"using T = int (X::*)();"
- " void f(X &x, T m) { X &ref = x; (ref.*m)(); }");
+ "void f(X &x, T m) { X &ref = x; (ref.*m)(); }");
const auto Results =
match(withEnclosingCompound(declRefTo("ref")), AST->getASTContext());
EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("(ref .* m)()"));
}
{
- const auto AST = buildASTFromCode(
- "struct X {};"
- "using T = int (X::*)();"
- " void f(X &x, T const m) { X &ref = x; (ref.*m)(); }");
+ const auto AST =
+ buildASTFromCode("struct X {};"
+ "using T = int (X::*)();"
+ "void f(X &x, T const m) { X &ref = x; (ref.*m)();
}");
const auto Results =
match(withEnclosingCompound(declRefTo("ref")), AST->getASTContext());
EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("(ref .* m)()"));
@@ -307,7 +307,7 @@ TEST(ExprMutationAnalyzerTest, MemberPointerMemberCall) {
const auto AST =
buildASTFromCode("struct X {};"
"using T = int (X::*)() const;"
- " void f(X &x, T m) { X &ref = x; (ref.*m)(); }");
+ "void f(X &x, T m) { X &ref = x; (ref.*m)(); }");
const auto Results =
match(withEnclosingCompound(declRefTo("ref")), AST->getASTContext());
EXPECT_FALSE(isMutated(Results, AST.get()));
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
