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] [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) { _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits