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