[clang-tools-extra] [clang-tidy]add new check `bugprone-compare-pointer-to-member-virtual-function` (PR #66055)
Congcong Cai via cfe-commits
cfe-commits at lists.llvm.org
Tue Sep 12 20:15:06 PDT 2023
https://github.com/HerrCai0907 updated https://github.com/llvm/llvm-project/pull/66055:
>From ed5150a69925dc3b6557796026a2820ac5c2e2f5 Mon Sep 17 00:00:00 2001
From: Congcong Cai <congcongcai0907 at 163.com>
Date: Tue, 12 Sep 2023 14:41:57 +0800
Subject: [PATCH 1/7] add
---
.../bugprone/BugproneTidyModule.cpp | 3 +
.../clang-tidy/bugprone/CMakeLists.txt | 1 +
...arePointerToMemberVirtualFunctionCheck.cpp | 90 +++++++++++++++++++
...mparePointerToMemberVirtualFunctionCheck.h | 30 +++++++
clang-tools-extra/docs/ReleaseNotes.rst | 6 ++
...are-pointer-to-member-virtual-function.rst | 44 +++++++++
.../docs/clang-tidy/checks/list.rst | 1 +
...are-pointer-to-member-virtual-function.cpp | 73 +++++++++++++++
8 files changed, 248 insertions(+)
create mode 100644 clang-tools-extra/clang-tidy/bugprone/ComparePointerToMemberVirtualFunctionCheck.cpp
create mode 100644 clang-tools-extra/clang-tidy/bugprone/ComparePointerToMemberVirtualFunctionCheck.h
create mode 100644 clang-tools-extra/docs/clang-tidy/checks/bugprone/compare-pointer-to-member-virtual-function.rst
create mode 100644 clang-tools-extra/test/clang-tidy/checkers/bugprone/compare-pointer-to-member-virtual-function.cpp
diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
index a67a91eedd10482..543c522899d7a52 100644
--- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
@@ -16,6 +16,7 @@
#include "BadSignalToKillThreadCheck.h"
#include "BoolPointerImplicitConversionCheck.h"
#include "BranchCloneCheck.h"
+#include "ComparePointerToMemberVirtualFunctionCheck.h"
#include "CopyConstructorInitCheck.h"
#include "DanglingHandleCheck.h"
#include "DynamicStaticInitializersCheck.h"
@@ -103,6 +104,8 @@ class BugproneModule : public ClangTidyModule {
CheckFactories.registerCheck<BoolPointerImplicitConversionCheck>(
"bugprone-bool-pointer-implicit-conversion");
CheckFactories.registerCheck<BranchCloneCheck>("bugprone-branch-clone");
+ CheckFactories.registerCheck<ComparePointerToMemberVirtualFunctionCheck>(
+ "bugprone-compare-pointer-to-member-virtual-function");
CheckFactories.registerCheck<CopyConstructorInitCheck>(
"bugprone-copy-constructor-init");
CheckFactories.registerCheck<DanglingHandleCheck>(
diff --git a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
index 3c768021feb1502..0df9e439b715e5a 100644
--- a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
@@ -11,6 +11,7 @@ add_clang_library(clangTidyBugproneModule
BoolPointerImplicitConversionCheck.cpp
BranchCloneCheck.cpp
BugproneTidyModule.cpp
+ ComparePointerToMemberVirtualFunctionCheck.cpp
CopyConstructorInitCheck.cpp
DanglingHandleCheck.cpp
DynamicStaticInitializersCheck.cpp
diff --git a/clang-tools-extra/clang-tidy/bugprone/ComparePointerToMemberVirtualFunctionCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/ComparePointerToMemberVirtualFunctionCheck.cpp
new file mode 100644
index 000000000000000..fed5825dfdbde23
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/bugprone/ComparePointerToMemberVirtualFunctionCheck.cpp
@@ -0,0 +1,90 @@
+//===--- ComparePointerToMemberVirtualFunctionCheck.cpp - clang-tidy ------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "ComparePointerToMemberVirtualFunctionCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/ASTTypeTraits.h"
+#include "clang/AST/DeclCXX.h"
+#include "clang/AST/OperationKinds.h"
+#include "clang/AST/Type.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/ASTMatchers/ASTMatchersMacros.h"
+#include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/DiagnosticIDs.h"
+#include "llvm/ADT/SmallVector.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::bugprone {
+
+// Matches a `UnaryOperator` whose operator is pre-increment:
+AST_MATCHER(CXXMethodDecl, isVirtual) { return Node.isVirtual(); }
+
+void ComparePointerToMemberVirtualFunctionCheck::registerMatchers(
+ MatchFinder *Finder) {
+
+ auto DirectMemberPointer = unaryOperator(
+ allOf(hasOperatorName("&"),
+ hasUnaryOperand(declRefExpr(to(cxxMethodDecl(isVirtual()))))));
+ auto IndirectMemberPointer = ignoringImpCasts(declRefExpr());
+
+ auto BinaryOperatorMatcher = [](auto &&Matcher) {
+ return binaryOperator(allOf(hasAnyOperatorName("==", "!="),
+ hasEitherOperand(hasType(memberPointerType(
+ pointee(functionType())))),
+ hasEitherOperand(Matcher)),
+ unless(hasEitherOperand(
+ castExpr(hasCastKind(CK_NullToMemberPointer)))));
+ };
+
+ Finder->addMatcher(
+ BinaryOperatorMatcher(DirectMemberPointer).bind("direct_member_pointer"),
+ this);
+
+ Finder->addMatcher(BinaryOperatorMatcher(IndirectMemberPointer)
+ .bind("indirect_member_pointer"),
+ this);
+}
+
+static const char *const ErrorMsg =
+ "A pointer to member virtual function shall only be tested for equality "
+ "with null-pointer-constant.";
+
+static const Expr *removeImplicitCast(const Expr *E) {
+ while (const auto *ICE = dyn_cast<ImplicitCastExpr>(E))
+ E = ICE->getSubExpr();
+ return E;
+}
+
+void ComparePointerToMemberVirtualFunctionCheck::check(
+ const MatchFinder::MatchResult &Result) {
+ if (const auto *DirectCompare =
+ Result.Nodes.getNodeAs<BinaryOperator>("direct_member_pointer")) {
+ diag(DirectCompare->getOperatorLoc(), ErrorMsg);
+ } else if (const auto *IndirectCompare =
+ Result.Nodes.getNodeAs<BinaryOperator>(
+ "indirect_member_pointer")) {
+ const Expr *E = removeImplicitCast(IndirectCompare->getLHS());
+ if (!isa<DeclRefExpr>(E))
+ E = removeImplicitCast(IndirectCompare->getRHS());
+ const auto *MPT = cast<MemberPointerType>(E->getType().getCanonicalType());
+ llvm::SmallVector<SourceLocation, 4U> SameSignatureVirtualMethods{};
+ for (const auto *D : MPT->getClass()->getAsCXXRecordDecl()->decls())
+ if (const auto *MD = dyn_cast<CXXMethodDecl>(D))
+ if (MD->isVirtual() && MD->getType() == MPT->getPointeeType())
+ SameSignatureVirtualMethods.push_back(MD->getBeginLoc());
+ if (!SameSignatureVirtualMethods.empty()) {
+ diag(IndirectCompare->getOperatorLoc(), ErrorMsg);
+ for (const auto Loc : SameSignatureVirtualMethods)
+ diag(Loc, "potential member virtual function.", DiagnosticIDs::Note);
+ }
+ }
+}
+
+} // namespace clang::tidy::bugprone
diff --git a/clang-tools-extra/clang-tidy/bugprone/ComparePointerToMemberVirtualFunctionCheck.h b/clang-tools-extra/clang-tidy/bugprone/ComparePointerToMemberVirtualFunctionCheck.h
new file mode 100644
index 000000000000000..6e10c4c3f551880
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/bugprone/ComparePointerToMemberVirtualFunctionCheck.h
@@ -0,0 +1,30 @@
+//===--- ComparePointerToMemberVirtualFunctionCheck.h - clang-tidy *- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_COMPAREPOINTERTOMEMBERVIRTUALFUNCTIONCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_COMPAREPOINTERTOMEMBERVIRTUALFUNCTIONCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang::tidy::bugprone {
+
+/// [expr.eq] If either is a pointer to a virtual member function, the result is unspecified.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/compare-pointer-to-member-virtual-function.html
+class ComparePointerToMemberVirtualFunctionCheck : public ClangTidyCheck {
+public:
+ ComparePointerToMemberVirtualFunctionCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context) {}
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace clang::tidy::bugprone
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_COMPAREPOINTERTOMEMBERVIRTUALFUNCTIONCHECK_H
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 63c644ca1a52239..4d2cd82ce0ff790 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -120,6 +120,12 @@ Improvements to clang-tidy
New checks
^^^^^^^^^^
+- New :doc:`bugprone-compare-pointer-to-member-virtual-function
+ <clang-tidy/checks/bugprone/compare-pointer-to-member-virtual-function>` check.
+
+ Detects equality comparison between pointer to member virtual function and
+ anything other than null-pointer-constant.
+
- New :doc:`bugprone-inc-dec-in-conditions
<clang-tidy/checks/bugprone/inc-dec-in-conditions>` check.
diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/compare-pointer-to-member-virtual-function.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/compare-pointer-to-member-virtual-function.rst
new file mode 100644
index 000000000000000..fc3d1d294537367
--- /dev/null
+++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/compare-pointer-to-member-virtual-function.rst
@@ -0,0 +1,44 @@
+.. title:: clang-tidy - bugprone-compare-pointer-to-member-virtual-function
+
+bugprone-compare-pointer-to-member-virtual-function
+===================================================
+
+Detects unspecified behavior about equality comparison between pointer to member virtual
+function and anything other than null-pointer-constant.
+
+
+.. code-block:: c++
+ struct A {
+ void f1();
+ void f2();
+ virtual void f3();
+ virtual void f4();
+
+ void g1(int);
+ };
+
+ void fn() {
+ bool r1 = (&A::f1 == &A::f2); // ok
+ bool r2 = (&A::f1 == &A::f3); // bugprone
+ bool r3 = (&A::f1 != &A::f3); // bugprone
+ bool r4 = (&A::f3 == nullptr); // ok
+ bool r5 = (&A::f3 == &A::f4); // bugprone
+
+ void (A::*v1)() = &A::f3;
+ bool r6 = (v1 == &A::f1); // bugprone
+ bool r6 = (v1 == nullptr); // ok
+
+ void (A::*v2)() = &A::f2;
+ bool r7 = (v2 == &A::f1); // false positive
+
+ void (A::*v3)(int) = &A::g1;
+ bool r8 = (v3 == &A::g1); // ok, no virtual function match void(A::*)(int) signature
+ }
+
+
+Limitations
+-----------
+
+The check will not analyze values stored in a variable. For variable, the check will analyze all
+virtual methods in the same ``class`` or ``struct`` and diagnose when assigning a pointer to member
+virtual function to this variable is possible.
diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst
index 75a86431d264412..1baabceea06ef48 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -82,6 +82,7 @@ Clang-Tidy Checks
:doc:`bugprone-bad-signal-to-kill-thread <bugprone/bad-signal-to-kill-thread>`,
:doc:`bugprone-bool-pointer-implicit-conversion <bugprone/bool-pointer-implicit-conversion>`, "Yes"
:doc:`bugprone-branch-clone <bugprone/branch-clone>`,
+ :doc:`bugprone-compare-pointer-to-member-virtual-function <bugprone/compare-pointer-to-member-virtual-function>`, "Yes"
:doc:`bugprone-copy-constructor-init <bugprone/copy-constructor-init>`, "Yes"
:doc:`bugprone-dangling-handle <bugprone/dangling-handle>`,
:doc:`bugprone-dynamic-static-initializers <bugprone/dynamic-static-initializers>`,
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/compare-pointer-to-member-virtual-function.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/compare-pointer-to-member-virtual-function.cpp
new file mode 100644
index 000000000000000..fd9d1c4d766af41
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/compare-pointer-to-member-virtual-function.cpp
@@ -0,0 +1,73 @@
+// RUN: %check_clang_tidy %s bugprone-compare-pointer-to-member-virtual-function %t
+
+struct A {
+ virtual ~A();
+ void f1();
+ void f2();
+ virtual void f3();
+ virtual void f4();
+
+ void g1(int);
+};
+
+bool Result;
+
+void base() {
+ Result = (&A::f1 == &A::f2);
+
+ Result = (&A::f1 == &A::f3);
+ // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: A pointer to member virtual function shall only be tested for equality with null-pointer-constant.
+
+ Result = (&A::f1 != &A::f3);
+ // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: A pointer to member virtual function shall only be tested for equality with null-pointer-constant.
+
+ Result = (&A::f3 == nullptr);
+
+ Result = (&A::f3 == &A::f4);
+ // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: A pointer to member virtual function shall only be tested for equality with null-pointer-constant.
+
+ void (A::*V1)() = &A::f3;
+ Result = (V1 == &A::f1);
+ // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: A pointer to member virtual function shall only be tested for equality with null-pointer-constant.
+ // CHECK-MESSAGES: :7:3: note: potential member virtual function.
+ // CHECK-MESSAGES: :8:3: note: potential member virtual function.
+ Result = (&A::f1 == V1);
+ // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: A pointer to member virtual function shall only be tested for equality with null-pointer-constant.
+ // CHECK-MESSAGES: :7:3: note: potential member virtual function.
+ // CHECK-MESSAGES: :8:3: note: potential member virtual function.
+
+ auto& V2 = V1;
+ Result = (&A::f1 == V2);
+ // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: A pointer to member virtual function shall only be tested for equality with null-pointer-constant.
+ // CHECK-MESSAGES: :7:3: note: potential member virtual function.
+ // CHECK-MESSAGES: :8:3: note: potential member virtual function.
+
+ void (A::*V3)(int) = &A::g1;
+ Result = (V3 == &A::g1);
+ Result = (&A::g1 == V3);
+}
+
+using B = A;
+void usingRecordName() {
+ Result = (&B::f1 == &B::f3);
+ // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: A pointer to member virtual function shall only be tested for equality with null-pointer-constant.
+
+ void (B::*V1)() = &B::f1;
+ Result = (V1 == &B::f1);
+ // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: A pointer to member virtual function shall only be tested for equality with null-pointer-constant.
+ // CHECK-MESSAGES: :7:3: note: potential member virtual function.
+ // CHECK-MESSAGES: :8:3: note: potential member virtual function.
+}
+
+
+typedef A C;
+void typedefRecordName() {
+ Result = (&C::f1 == &C::f3);
+ // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: A pointer to member virtual function shall only be tested for equality with null-pointer-constant.
+
+ void (C::*V1)() = &C::f1;
+ Result = (V1 == &C::f1);
+ // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: A pointer to member virtual function shall only be tested for equality with null-pointer-constant.
+ // CHECK-MESSAGES: :7:3: note: potential member virtual function.
+ // CHECK-MESSAGES: :8:3: note: potential member virtual function.
+}
>From 2ba04c09c51bdb304e103511a683127665c6fb84 Mon Sep 17 00:00:00 2001
From: Congcong Cai <congcongcai0907 at 163.com>
Date: Tue, 12 Sep 2023 16:42:32 +0800
Subject: [PATCH 2/7] remove useless comment
---
.../bugprone/ComparePointerToMemberVirtualFunctionCheck.cpp | 1 -
.../bugprone/ComparePointerToMemberVirtualFunctionCheck.h | 1 -
2 files changed, 2 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/bugprone/ComparePointerToMemberVirtualFunctionCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/ComparePointerToMemberVirtualFunctionCheck.cpp
index fed5825dfdbde23..a42cf01885690a9 100644
--- a/clang-tools-extra/clang-tidy/bugprone/ComparePointerToMemberVirtualFunctionCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/ComparePointerToMemberVirtualFunctionCheck.cpp
@@ -23,7 +23,6 @@ using namespace clang::ast_matchers;
namespace clang::tidy::bugprone {
-// Matches a `UnaryOperator` whose operator is pre-increment:
AST_MATCHER(CXXMethodDecl, isVirtual) { return Node.isVirtual(); }
void ComparePointerToMemberVirtualFunctionCheck::registerMatchers(
diff --git a/clang-tools-extra/clang-tidy/bugprone/ComparePointerToMemberVirtualFunctionCheck.h b/clang-tools-extra/clang-tidy/bugprone/ComparePointerToMemberVirtualFunctionCheck.h
index 6e10c4c3f551880..736c0fda1fd5cc8 100644
--- a/clang-tools-extra/clang-tidy/bugprone/ComparePointerToMemberVirtualFunctionCheck.h
+++ b/clang-tools-extra/clang-tidy/bugprone/ComparePointerToMemberVirtualFunctionCheck.h
@@ -14,7 +14,6 @@
namespace clang::tidy::bugprone {
/// [expr.eq] If either is a pointer to a virtual member function, the result is unspecified.
-///
/// For the user-facing documentation see:
/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/compare-pointer-to-member-virtual-function.html
class ComparePointerToMemberVirtualFunctionCheck : public ClangTidyCheck {
>From 9507317e5d3329c3a63c7da36aa74669d712059f Mon Sep 17 00:00:00 2001
From: Congcong Cai <congcongcai0907 at 163.com>
Date: Wed, 13 Sep 2023 09:41:10 +0800
Subject: [PATCH 3/7] fix ODR
---
...arePointerToMemberVirtualFunctionCheck.cpp | 24 +++++++++++--------
1 file changed, 14 insertions(+), 10 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/bugprone/ComparePointerToMemberVirtualFunctionCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/ComparePointerToMemberVirtualFunctionCheck.cpp
index a42cf01885690a9..36d0f3d41900092 100644
--- a/clang-tools-extra/clang-tidy/bugprone/ComparePointerToMemberVirtualFunctionCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/ComparePointerToMemberVirtualFunctionCheck.cpp
@@ -23,8 +23,22 @@ using namespace clang::ast_matchers;
namespace clang::tidy::bugprone {
+namespace {
+
AST_MATCHER(CXXMethodDecl, isVirtual) { return Node.isVirtual(); }
+static const char *const ErrorMsg =
+ "A pointer to member virtual function shall only be tested for equality "
+ "with null-pointer-constant.";
+
+static const Expr *removeImplicitCast(const Expr *E) {
+ while (const auto *ICE = dyn_cast<ImplicitCastExpr>(E))
+ E = ICE->getSubExpr();
+ return E;
+}
+
+} // namespace
+
void ComparePointerToMemberVirtualFunctionCheck::registerMatchers(
MatchFinder *Finder) {
@@ -51,16 +65,6 @@ void ComparePointerToMemberVirtualFunctionCheck::registerMatchers(
this);
}
-static const char *const ErrorMsg =
- "A pointer to member virtual function shall only be tested for equality "
- "with null-pointer-constant.";
-
-static const Expr *removeImplicitCast(const Expr *E) {
- while (const auto *ICE = dyn_cast<ImplicitCastExpr>(E))
- E = ICE->getSubExpr();
- return E;
-}
-
void ComparePointerToMemberVirtualFunctionCheck::check(
const MatchFinder::MatchResult &Result) {
if (const auto *DirectCompare =
>From c1f5c71d6af1f1b42c73fd83e7598f543172011c Mon Sep 17 00:00:00 2001
From: Congcong Cai <congcongcai0907 at 163.com>
Date: Wed, 13 Sep 2023 09:52:00 +0800
Subject: [PATCH 4/7] rewriter matcher
---
...arePointerToMemberVirtualFunctionCheck.cpp | 75 ++++++++-----------
1 file changed, 33 insertions(+), 42 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/bugprone/ComparePointerToMemberVirtualFunctionCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/ComparePointerToMemberVirtualFunctionCheck.cpp
index 36d0f3d41900092..e73bd6711e7868b 100644
--- a/clang-tools-extra/clang-tidy/bugprone/ComparePointerToMemberVirtualFunctionCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/ComparePointerToMemberVirtualFunctionCheck.cpp
@@ -31,62 +31,53 @@ static const char *const ErrorMsg =
"A pointer to member virtual function shall only be tested for equality "
"with null-pointer-constant.";
-static const Expr *removeImplicitCast(const Expr *E) {
- while (const auto *ICE = dyn_cast<ImplicitCastExpr>(E))
- E = ICE->getSubExpr();
- return E;
-}
-
} // namespace
void ComparePointerToMemberVirtualFunctionCheck::registerMatchers(
MatchFinder *Finder) {
- auto DirectMemberPointer = unaryOperator(
+ auto DirectMemberVirtualFunctionPointer = unaryOperator(
allOf(hasOperatorName("&"),
hasUnaryOperand(declRefExpr(to(cxxMethodDecl(isVirtual()))))));
- auto IndirectMemberPointer = ignoringImpCasts(declRefExpr());
-
- auto BinaryOperatorMatcher = [](auto &&Matcher) {
- return binaryOperator(allOf(hasAnyOperatorName("==", "!="),
- hasEitherOperand(hasType(memberPointerType(
- pointee(functionType())))),
- hasEitherOperand(Matcher)),
- unless(hasEitherOperand(
- castExpr(hasCastKind(CK_NullToMemberPointer)))));
- };
+ auto IndirectMemberPointer =
+ ignoringImpCasts(declRefExpr().bind("indirect_member_pointer"));
Finder->addMatcher(
- BinaryOperatorMatcher(DirectMemberPointer).bind("direct_member_pointer"),
+ binaryOperator(
+ allOf(hasAnyOperatorName("==", "!="),
+ hasEitherOperand(
+ hasType(memberPointerType(pointee(functionType())))),
+ anyOf(hasEitherOperand(DirectMemberVirtualFunctionPointer),
+ hasEitherOperand(IndirectMemberPointer)),
+ unless(hasEitherOperand(
+ castExpr(hasCastKind(CK_NullToMemberPointer))))))
+ .bind("binary_operator"),
this);
-
- Finder->addMatcher(BinaryOperatorMatcher(IndirectMemberPointer)
- .bind("indirect_member_pointer"),
- this);
}
void ComparePointerToMemberVirtualFunctionCheck::check(
const MatchFinder::MatchResult &Result) {
- if (const auto *DirectCompare =
- Result.Nodes.getNodeAs<BinaryOperator>("direct_member_pointer")) {
- diag(DirectCompare->getOperatorLoc(), ErrorMsg);
- } else if (const auto *IndirectCompare =
- Result.Nodes.getNodeAs<BinaryOperator>(
- "indirect_member_pointer")) {
- const Expr *E = removeImplicitCast(IndirectCompare->getLHS());
- if (!isa<DeclRefExpr>(E))
- E = removeImplicitCast(IndirectCompare->getRHS());
- const auto *MPT = cast<MemberPointerType>(E->getType().getCanonicalType());
- llvm::SmallVector<SourceLocation, 4U> SameSignatureVirtualMethods{};
- for (const auto *D : MPT->getClass()->getAsCXXRecordDecl()->decls())
- if (const auto *MD = dyn_cast<CXXMethodDecl>(D))
- if (MD->isVirtual() && MD->getType() == MPT->getPointeeType())
- SameSignatureVirtualMethods.push_back(MD->getBeginLoc());
- if (!SameSignatureVirtualMethods.empty()) {
- diag(IndirectCompare->getOperatorLoc(), ErrorMsg);
- for (const auto Loc : SameSignatureVirtualMethods)
- diag(Loc, "potential member virtual function.", DiagnosticIDs::Note);
- }
+ const BinaryOperator *BO =
+ Result.Nodes.getNodeAs<BinaryOperator>("binary_operator");
+ const DeclRefExpr *DRE =
+ Result.Nodes.getNodeAs<DeclRefExpr>("indirect_member_pointer");
+
+ if (DRE == nullptr) {
+ // compare with pointer to member virtual function.
+ diag(BO->getOperatorLoc(), ErrorMsg);
+ return;
+ }
+ // compare with variable which type is pointer to member function.
+ const auto *MPT = cast<MemberPointerType>(DRE->getType().getCanonicalType());
+ llvm::SmallVector<SourceLocation, 12U> SameSignatureVirtualMethods{};
+ for (const auto *D : MPT->getClass()->getAsCXXRecordDecl()->decls())
+ if (const auto *MD = dyn_cast<CXXMethodDecl>(D))
+ if (MD->isVirtual() && MD->getType() == MPT->getPointeeType())
+ SameSignatureVirtualMethods.push_back(MD->getBeginLoc());
+ if (!SameSignatureVirtualMethods.empty()) {
+ diag(BO->getOperatorLoc(), ErrorMsg);
+ for (const auto Loc : SameSignatureVirtualMethods)
+ diag(Loc, "potential member virtual function.", DiagnosticIDs::Note);
}
}
>From d21281322c811bd4afda25503e98a7750619dca3 Mon Sep 17 00:00:00 2001
From: Congcong Cai <congcongcai0907 at 163.com>
Date: Wed, 13 Sep 2023 10:05:19 +0800
Subject: [PATCH 5/7] check nullptr
---
...mparePointerToMemberVirtualFunctionCheck.cpp | 17 ++++++++++++-----
1 file changed, 12 insertions(+), 5 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/bugprone/ComparePointerToMemberVirtualFunctionCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/ComparePointerToMemberVirtualFunctionCheck.cpp
index e73bd6711e7868b..49f238d203c9651 100644
--- a/clang-tools-extra/clang-tidy/bugprone/ComparePointerToMemberVirtualFunctionCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/ComparePointerToMemberVirtualFunctionCheck.cpp
@@ -68,12 +68,19 @@ void ComparePointerToMemberVirtualFunctionCheck::check(
return;
}
// compare with variable which type is pointer to member function.
- const auto *MPT = cast<MemberPointerType>(DRE->getType().getCanonicalType());
llvm::SmallVector<SourceLocation, 12U> SameSignatureVirtualMethods{};
- for (const auto *D : MPT->getClass()->getAsCXXRecordDecl()->decls())
- if (const auto *MD = dyn_cast<CXXMethodDecl>(D))
- if (MD->isVirtual() && MD->getType() == MPT->getPointeeType())
- SameSignatureVirtualMethods.push_back(MD->getBeginLoc());
+ const auto *MPT = cast<MemberPointerType>(DRE->getType().getCanonicalType());
+ const Type *T = MPT->getClass();
+ if (T == nullptr)
+ return;
+ const CXXRecordDecl *RD = T->getAsCXXRecordDecl();
+ if (RD == nullptr)
+ return;
+ for (const auto *D : RD->decls()) {
+ const auto *MD = dyn_cast<CXXMethodDecl>(D);
+ if (MD && MD->isVirtual() && MD->getType() == MPT->getPointeeType())
+ SameSignatureVirtualMethods.push_back(MD->getBeginLoc());
+ }
if (!SameSignatureVirtualMethods.empty()) {
diag(BO->getOperatorLoc(), ErrorMsg);
for (const auto Loc : SameSignatureVirtualMethods)
>From 366645f9d5b1f1bece713047698251f61d4fabb2 Mon Sep 17 00:00:00 2001
From: Congcong Cai <congcongcai0907 at 163.com>
Date: Wed, 13 Sep 2023 10:18:52 +0800
Subject: [PATCH 6/7] fix docs
---
.../bugprone/ComparePointerToMemberVirtualFunctionCheck.h | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/clang-tools-extra/clang-tidy/bugprone/ComparePointerToMemberVirtualFunctionCheck.h b/clang-tools-extra/clang-tidy/bugprone/ComparePointerToMemberVirtualFunctionCheck.h
index 736c0fda1fd5cc8..f04b0e9a86d93a5 100644
--- a/clang-tools-extra/clang-tidy/bugprone/ComparePointerToMemberVirtualFunctionCheck.h
+++ b/clang-tools-extra/clang-tidy/bugprone/ComparePointerToMemberVirtualFunctionCheck.h
@@ -13,13 +13,19 @@
namespace clang::tidy::bugprone {
-/// [expr.eq] If either is a pointer to a virtual member function, the result is unspecified.
+/// Detects unspecified behavior about equality comparison between pointer to
+/// member virtual function and anything other than null-pointer-constant.
+///
/// For the user-facing documentation see:
/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/compare-pointer-to-member-virtual-function.html
class ComparePointerToMemberVirtualFunctionCheck : public ClangTidyCheck {
public:
ComparePointerToMemberVirtualFunctionCheck(StringRef Name, ClangTidyContext *Context)
: ClangTidyCheck(Name, Context) {}
+
+ bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
+ return LangOpts.CPlusPlus;
+ }
void registerMatchers(ast_matchers::MatchFinder *Finder) override;
void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
};
>From 91c118c1b09a6484d3c5651f45f28ef8c5c167c4 Mon Sep 17 00:00:00 2001
From: Congcong Cai <congcongcai0907 at 163.com>
Date: Wed, 13 Sep 2023 11:14:49 +0800
Subject: [PATCH 7/7] consider inherit case
---
...arePointerToMemberVirtualFunctionCheck.cpp | 22 +++++++++++++++----
...are-pointer-to-member-virtual-function.cpp | 17 ++++++++++++++
2 files changed, 35 insertions(+), 4 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/bugprone/ComparePointerToMemberVirtualFunctionCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/ComparePointerToMemberVirtualFunctionCheck.cpp
index 49f238d203c9651..2b908705f198a7a 100644
--- a/clang-tools-extra/clang-tidy/bugprone/ComparePointerToMemberVirtualFunctionCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/ComparePointerToMemberVirtualFunctionCheck.cpp
@@ -76,11 +76,25 @@ void ComparePointerToMemberVirtualFunctionCheck::check(
const CXXRecordDecl *RD = T->getAsCXXRecordDecl();
if (RD == nullptr)
return;
- for (const auto *D : RD->decls()) {
- const auto *MD = dyn_cast<CXXMethodDecl>(D);
- if (MD && MD->isVirtual() && MD->getType() == MPT->getPointeeType())
- SameSignatureVirtualMethods.push_back(MD->getBeginLoc());
+
+ const bool StopVisit = false;
+
+ auto VisitSameSignatureVirtualMethods =
+ [&](const CXXRecordDecl *CurrentRecordDecl) -> bool {
+ bool Ret = !StopVisit;
+ for (const auto *MD : CurrentRecordDecl->methods()) {
+ if (MD->isVirtual() && MD->getType() == MPT->getPointeeType()) {
+ SameSignatureVirtualMethods.push_back(MD->getBeginLoc());
+ Ret = StopVisit;
+ }
+ }
+ return Ret;
+ };
+
+ if (StopVisit != VisitSameSignatureVirtualMethods(RD)) {
+ RD->forallBases(VisitSameSignatureVirtualMethods);
}
+
if (!SameSignatureVirtualMethods.empty()) {
diag(BO->getOperatorLoc(), ErrorMsg);
for (const auto Loc : SameSignatureVirtualMethods)
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/compare-pointer-to-member-virtual-function.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/compare-pointer-to-member-virtual-function.cpp
index fd9d1c4d766af41..6969b39a0e21383 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/compare-pointer-to-member-virtual-function.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/compare-pointer-to-member-virtual-function.cpp
@@ -71,3 +71,20 @@ void typedefRecordName() {
// CHECK-MESSAGES: :7:3: note: potential member virtual function.
// CHECK-MESSAGES: :8:3: note: potential member virtual function.
}
+
+
+struct A1 : public A {
+};
+
+void inheritClass() {
+ Result = (&A1::f1 == &A1::f3);
+ // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: A pointer to member virtual function shall only be tested for equality with null-pointer-constant.
+
+ void (A1::*V1)() = &A1::f1;
+ Result = (V1 == &A1::f1);
+ // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: A pointer to member virtual function shall only be tested for equality with null-pointer-constant.
+ // CHECK-MESSAGES: :7:3: note: potential member virtual function.
+ // CHECK-MESSAGES: :8:3: note: potential member virtual function.
+}
+
+
More information about the cfe-commits
mailing list