[clang] feb7b19 - [clang-analysis]Fix false positive in mutation check when using pointer to member function (#66846)

via cfe-commits cfe-commits at lists.llvm.org
Mon Sep 25 16:40:19 PDT 2023


Author: Congcong Cai
Date: 2023-09-26T07:40:15+08:00
New Revision: feb7b1914d513c709b9e024dfed709bb889cc853

URL: https://github.com/llvm/llvm-project/commit/feb7b1914d513c709b9e024dfed709bb889cc853
DIFF: https://github.com/llvm/llvm-project/commit/feb7b1914d513c709b9e024dfed709bb889cc853.diff

LOG: [clang-analysis]Fix false positive in mutation check when using pointer to member function (#66846)

Fixes: #66204

Added: 
    

Modified: 
    clang-tools-extra/docs/ReleaseNotes.rst
    clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-values.cpp
    clang/docs/ReleaseNotes.rst
    clang/lib/Analysis/ExprMutationAnalyzer.cpp
    clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 466c9b08c578890..8fc28c090341802 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -246,6 +246,10 @@ Changes in existing checks
   customizable namespace. This further allows for testing the libc when the
   system-libc is also LLVM's libc.
 
+- 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 688454d6b562ec3..8a136aae5489a8c 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -502,6 +502,9 @@ 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.
+  (`#66204: <https://github.com/llvm/llvm-project/issues/66204>`_).
+
 - The ``alpha.security.taint.TaintPropagation`` checker no longer propagates
   taint on ``strlen`` and ``strnlen`` calls, unless these are marked
   explicitly propagators in the user-provided taint configuration file.

diff  --git a/clang/lib/Analysis/ExprMutationAnalyzer.cpp b/clang/lib/Analysis/ExprMutationAnalyzer.cpp
index 90803830ff41949..f2e1beb025423cf 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 Decl *CalleeDecl = Node.getCalleeDecl();
+  const auto *VD = dyn_cast_or_null<ValueDecl>(CalleeDecl);
+  if (!VD)
+    return false;
+  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);
+  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(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..c0a398394093c48 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) {


        


More information about the cfe-commits mailing list