[clang-tools-extra] [clang-tidy]add new check `bugprone-compare-pointer-to-member-virtual-function` (PR #66055)
via cfe-commits
cfe-commits at lists.llvm.org
Tue Sep 12 01:41:57 PDT 2023
llvmbot wrote:
@llvm/pr-subscribers-clang-tidy
<details>
<summary>Changes</summary>
None
--
Full diff: https://github.com/llvm/llvm-project/pull/66055.diff
8 Files Affected:
- (modified) clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp (+3)
- (modified) clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt (+1)
- (added) clang-tools-extra/clang-tidy/bugprone/ComparePointerToMemberVirtualFunctionCheck.cpp (+90)
- (added) clang-tools-extra/clang-tidy/bugprone/ComparePointerToMemberVirtualFunctionCheck.h (+30)
- (modified) clang-tools-extra/docs/ReleaseNotes.rst (+6)
- (added) clang-tools-extra/docs/clang-tidy/checks/bugprone/compare-pointer-to-member-virtual-function.rst (+44)
- (modified) clang-tools-extra/docs/clang-tidy/checks/list.rst (+1)
- (added) clang-tools-extra/test/clang-tidy/checkers/bugprone/compare-pointer-to-member-virtual-function.cpp (+73)
<pre>
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.
+}
</pre>
</details>
https://github.com/llvm/llvm-project/pull/66055
More information about the cfe-commits
mailing list