[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:43:23 PDT 2023
llvmbot wrote:
<!--IGNORE-->
>@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: heck_clang_tidy bugprone-compare-pointer-to-member-virtual-function
Error: Command failed due to missing milestone.
https://github.com/llvm/llvm-project/pull/66055
More information about the cfe-commits
mailing list