https://github.com/t-a-james updated https://github.com/llvm/llvm-project/pull/154746
>From a40192197693973442d61823fadbb4b38df71276 Mon Sep 17 00:00:00 2001 From: Tom James <tom.ja...@siemens.com> Date: Thu, 21 Aug 2025 13:14:24 +0100 Subject: [PATCH 1/2] [clang-tidy] New bugprone-method-hiding check --- .../bugprone/BugproneTidyModule.cpp | 6 + .../clang-tidy/bugprone/CMakeLists.txt | 7 + .../clang-tidy/bugprone/MethodHidingCheck.cpp | 148 ++++++++++++++++++ .../clang-tidy/bugprone/MethodHidingCheck.h | 37 +++++ clang-tools-extra/docs/ReleaseNotes.rst | 11 ++ .../checks/bugprone/method-hiding.rst | 15 ++ .../docs/clang-tidy/checks/list.rst | 7 + .../checkers/bugprone/method-hiding.cpp | 118 ++++++++++++++ 8 files changed, 349 insertions(+) create mode 100644 clang-tools-extra/clang-tidy/bugprone/MethodHidingCheck.cpp create mode 100644 clang-tools-extra/clang-tidy/bugprone/MethodHidingCheck.h create mode 100644 clang-tools-extra/docs/clang-tidy/checks/bugprone/method-hiding.rst create mode 100644 clang-tools-extra/test/clang-tidy/checkers/bugprone/method-hiding.cpp diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp index 824ebdfbd00dc..d1d1979794743 100644 --- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp @@ -4,6 +4,9 @@ // See https://llvm.org/LICENSE.txt for license information. // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception // +// SPDX-FileCopyrightText: Portions Copyright 2025 Siemens and/or its affiliates +// May 2025 modified by Siemens and/or its affiliates by Tom James +// //===----------------------------------------------------------------------===// #include "../ClangTidy.h" @@ -42,6 +45,7 @@ #include "LambdaFunctionNameCheck.h" #include "MacroParenthesesCheck.h" #include "MacroRepeatedSideEffectsCheck.h" +#include "MethodHidingCheck.h" #include "MisleadingSetterOfReferenceCheck.h" #include "MisplacedOperatorInStrlenInAllocCheck.h" #include "MisplacedPointerArithmeticInAllocCheck.h" @@ -153,6 +157,8 @@ class BugproneModule : public ClangTidyModule { "bugprone-incorrect-enable-if"); CheckFactories.registerCheck<IncorrectEnableSharedFromThisCheck>( "bugprone-incorrect-enable-shared-from-this"); + CheckFactories.registerCheck<MethodHidingCheck>( + "bugprone-method-hiding"); CheckFactories.registerCheck<UnintendedCharOstreamOutputCheck>( "bugprone-unintended-char-ostream-output"); CheckFactories.registerCheck<ReturnConstRefFromParameterCheck>( diff --git a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt index 59928e5e47a09..5745654855108 100644 --- a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt @@ -1,3 +1,9 @@ +# 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 +# SPDX-FileCopyrightText: Portions Copyright 2025 Siemens and/or its affiliates +# May 2025 modified by Siemens and/or its affiliates by Tom James + set(LLVM_LINK_COMPONENTS support FrontendOpenMP @@ -31,6 +37,7 @@ add_clang_library(clangTidyBugproneModule STATIC IncorrectEnableIfCheck.cpp IncorrectEnableSharedFromThisCheck.cpp InvalidEnumDefaultInitializationCheck.cpp + MethodHidingCheck.cpp UnintendedCharOstreamOutputCheck.cpp ReturnConstRefFromParameterCheck.cpp SuspiciousStringviewDataUsageCheck.cpp diff --git a/clang-tools-extra/clang-tidy/bugprone/MethodHidingCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/MethodHidingCheck.cpp new file mode 100644 index 0000000000000..865ef115950e6 --- /dev/null +++ b/clang-tools-extra/clang-tidy/bugprone/MethodHidingCheck.cpp @@ -0,0 +1,148 @@ +//===--- MethodHidingCheck.cpp - clang-tidy ----------------------------===// +// +// SPDX-FileCopyrightText: 2025 Siemens Corporation and/or its affiliates +// +// 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 "MethodHidingCheck.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include <stack> + +using namespace clang::ast_matchers; + +namespace clang::tidy::bugprone { + +namespace { + +bool sameBasicType(ParmVarDecl const *Lhs, ParmVarDecl const *Rhs) { + if (Lhs && Rhs) { + return Lhs->getType() + .getCanonicalType() + .getNonReferenceType() + .getUnqualifiedType() == Rhs->getType() + .getCanonicalType() + .getNonReferenceType() + .getUnqualifiedType(); + } + return false; +} + +bool namesCollide(CXXMethodDecl const &Lhs, CXXMethodDecl const &Rhs) { + if (Lhs.getNameAsString() != Rhs.getNameAsString()) { + return false; + } + if (Lhs.isConst() != Rhs.isConst()) { + return false; + } + if (Lhs.getNumParams() != Rhs.getNumParams()) { + return false; + } + for (unsigned int It = 0; It < Lhs.getNumParams(); ++It) { + if (!sameBasicType(Lhs.getParamDecl(It), Rhs.getParamDecl(It))) { + return false; + } + } + // Templates are not handled yet + if (Lhs.isTemplated() || Rhs.isTemplated()) { + return false; + } + if (Lhs.isTemplateInstantiation() || Rhs.isTemplateInstantiation()) { + return false; + } + if (Lhs.isFunctionTemplateSpecialization() || + Rhs.isFunctionTemplateSpecialization()) { + return false; + } + return true; +} + +AST_MATCHER(CXXMethodDecl, nameCollidesWithMethodInBase) { + CXXRecordDecl const *DerivedClass = Node.getParent(); + for (auto const &Base : DerivedClass->bases()) { + std::stack<CXXBaseSpecifier const *> Stack; + Stack.push(&Base); + while (!Stack.empty()) { + CXXBaseSpecifier const *CurrentBaseSpec = Stack.top(); + Stack.pop(); + + if (CurrentBaseSpec->getAccessSpecifier() == + clang::AccessSpecifier::AS_private) { + continue; + } + + const auto *CurrentRecord = + CurrentBaseSpec->getType()->getAsCXXRecordDecl(); + if (!CurrentRecord) { + continue; + } + + for (auto const &BaseMethod : CurrentRecord->methods()) { + if (namesCollide(*BaseMethod, Node)) { + ast_matchers::internal::BoundNodesTreeBuilder Result(*Builder); + Builder->setBinding("base_method", + clang::DynTypedNode::create(*BaseMethod)); + return true; + } + } + + for (auto const &SubBase : CurrentRecord->bases()) { + Stack.push(&SubBase); + } + } + } + return false; +} + +// Same as clang-tools-extra/clang-tidy/modernize/UseEqualsDefaultCheck.cpp, +// similar matchers are used elsewhere in LLVM +AST_MATCHER(CXXMethodDecl, isOutOfLine) { return Node.isOutOfLine(); } + +} // namespace + +MethodHidingCheck::MethodHidingCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + +void MethodHidingCheck::registerMatchers(MatchFinder *Finder) { + Finder->addMatcher( + cxxMethodDecl( + unless(anyOf(isOutOfLine(), isStaticStorageClass(), isImplicit(), + cxxConstructorDecl(), isOverride(), + // isFinal(), //included with isOverride, + isPrivate())), + ofClass(cxxRecordDecl( + isDerivedFrom(cxxRecordDecl(unless(isInStdNamespace())))) + .bind("derived_class")), + nameCollidesWithMethodInBase()) + .bind("shadowing_method"), + this); +} + +void MethodHidingCheck::check(const MatchFinder::MatchResult &Result) { + auto const *ShadowingMethod = + Result.Nodes.getNodeAs<CXXMethodDecl>("shadowing_method"); + auto const *DerivedClass = + Result.Nodes.getNodeAs<CXXRecordDecl>("derived_class"); + auto const *BaseMethod = Result.Nodes.getNodeAs<CXXMethodDecl>("base_method"); + + if (!ShadowingMethod || !DerivedClass || !BaseMethod) { + llvm_unreachable("Required binding not found"); + } + + auto const MethodName = ShadowingMethod->getNameInfo().getAsString(); + auto const DerivedClassName = DerivedClass->getNameAsString(); + std::string BaseClassName = BaseMethod->getParent()->getNameAsString(); + + std::string Message; + llvm::raw_string_ostream StringStream(Message); + StringStream << "'" << ShadowingMethod->getQualifiedNameAsString() + << "' hides same method in '" << BaseClassName << "'"; + diag(ShadowingMethod->getBeginLoc(), Message); + diag(BaseMethod->getBeginLoc(), "previous definition is here", + DiagnosticIDs::Note); +} + +} // namespace clang::tidy::bugprone diff --git a/clang-tools-extra/clang-tidy/bugprone/MethodHidingCheck.h b/clang-tools-extra/clang-tidy/bugprone/MethodHidingCheck.h new file mode 100644 index 0000000000000..f00f46cdf72e2 --- /dev/null +++ b/clang-tools-extra/clang-tidy/bugprone/MethodHidingCheck.h @@ -0,0 +1,37 @@ +//===--- MethodHidingCheck.h - clang-tidy --------------------*- C++ -*-===// +// +// SPDX-FileCopyrightText: 2025 Siemens Corporation and/or its affiliates +// +// 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_METHODHIDINGCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_METHODHIDINGCHECK_H + +#include "../ClangTidyCheck.h" + +namespace clang::tidy::bugprone { + +/// Checks that a derived class does not define the same (non virtual) method as +/// a base class +/// +/// This anti-pattern is a Liskov violation. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/method-hiding.html +class MethodHidingCheck : public ClangTidyCheck { +public: + MethodHidingCheck(StringRef Name, ClangTidyContext *Context); + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + bool isLanguageVersionSupported(const LangOptions &LangOpts) const override { + return LangOpts.CPlusPlus; + } +}; + +} // namespace clang::tidy::bugprone + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_METHODHIDINGCHECK_H diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 388979d9577ba..4429580751067 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -1,6 +1,12 @@ .. If you want to modify sections/contents permanently, you should modify both ReleaseNotes.rst and ReleaseNotesTemplate.txt. +.. 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 +.. SPDX-FileCopyrightText: Portions Copyright 2025 Siemens and/or its affiliates +.. May 2025 modified by Siemens and/or its affiliates by Tom James + ==================================================== Extra Clang Tools |release| |ReleaseNotesTitle| ==================================================== @@ -131,6 +137,11 @@ New checks Detects default initialization (to 0) of variables with ``enum`` type where the enum has no enumerator with value of 0. +- New :doc:`bugprone-method-hiding + <clang-tidy/checks/bugprone/method-hiding>` check. + + Finds derived class methods that shadow a (non-virtual) base class method. + - New :doc:`llvm-mlir-op-builder <clang-tidy/checks/llvm/use-new-mlir-op-builder>` check. diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/method-hiding.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/method-hiding.rst new file mode 100644 index 0000000000000..dd58ec29e60c2 --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/method-hiding.rst @@ -0,0 +1,15 @@ +.. SPDX-FileCopyrightText: 2025 Siemens Corporation and/or its affiliates +.. 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 + +.. title:: clang-tidy - bugprone-method-hiding + +bugprone-method-hiding +========================= + +Finds derived class methods that hide a (non-virtual) base class method. + +In order to be considered "hiding", methods must have the same signature +(i.e. the same name, same number of parameters, same parameter types, etc). +Only checks public, non-templated methods. \ No newline at end of file diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst index b0961265345c0..c2f305a0223f1 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -1,3 +1,9 @@ +.. 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 +.. SPDX-FileCopyrightText: Portions Copyright 2025 Siemens and/or its affiliates +.. May 2025 modified by Siemens and/or its affiliates by Tom James + .. title:: clang-tidy - Clang-Tidy Checks Clang-Tidy Checks @@ -110,6 +116,7 @@ Clang-Tidy Checks :doc:`bugprone-lambda-function-name <bugprone/lambda-function-name>`, :doc:`bugprone-macro-parentheses <bugprone/macro-parentheses>`, "Yes" :doc:`bugprone-macro-repeated-side-effects <bugprone/macro-repeated-side-effects>`, + :doc:`bugprone-method-hiding <bugprone/method-shadowing>` :doc:`bugprone-misleading-setter-of-reference <bugprone/misleading-setter-of-reference>`, :doc:`bugprone-misplaced-operator-in-strlen-in-alloc <bugprone/misplaced-operator-in-strlen-in-alloc>`, "Yes" :doc:`bugprone-misplaced-pointer-arithmetic-in-alloc <bugprone/misplaced-pointer-arithmetic-in-alloc>`, "Yes" diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/method-hiding.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/method-hiding.cpp new file mode 100644 index 0000000000000..38511c8807611 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/method-hiding.cpp @@ -0,0 +1,118 @@ +//===--- MethodHidingCheck.cpp - clang-tidy ----------------------------===// +// +// SPDX-FileCopyrightText: 2025 Siemens Corporation and/or its affiliates +// +// 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 +// +//===----------------------------------------------------------------------===// + +// RUN: %check_clang_tidy %s bugprone-method-hiding %t + + +class Base +{ + void method(); + void methodWithArg(int I); + + virtual Base* getThis() = 0; +}; + +class A : public Base +{ +public: + void method(); +// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'A::method' hides same method in 'Base' [bugprone-method-hiding] +}; + +// only declaration should be checked +void A::method() +{ +} + +class B +{ +public: + void method(); +}; + +class D: public Base +{ + +}; + +// test indirect inheritance +class E : public D +{ +public: + void method(); +// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'E::method' hides same method in 'Base' [bugprone-method-hiding] +}; + +class H : public Base +{ +public: + Base* getThis() override; + Base const* getThis() const; +}; + +class I : public Base +{ +public: + // test with inline implementation + void method() +// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'I::method' hides same method in 'Base' [bugprone-method-hiding] + { + + } +}; + +class J : public Base +{ +public: + Base* getThis() final; +}; + +template<typename T> +class TemplateBase +{ +public: + virtual void size() const = 0; +}; + +template<typename T> +class K : public TemplateBase<T> +{ +public: + void size() const final; +}; + +class L : public Base +{ +public: +// not same signature (take const ref) but still ambiguous + void methodWithArg(int const& I); +// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'L::methodWithArg' hides same method in 'Base' [bugprone-method-hiding] + + void methodWithArg(int const I); +// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'L::methodWithArg' hides same method in 'Base' [bugprone-method-hiding] + + void methodWithArg(int *I); + void methodWithArg(int const* I); +}; + +class M : public Base +{ +public: + static void method(); +}; + +class N : public Base +{ +public: + template<typename T> + void methodWithArg(T I); + // TODO: Templates are not handled yet + template<> void methodWithArg<int>(int I); +}; >From d8da87196bce635b2db162d2aad550b3a68f7dc6 Mon Sep 17 00:00:00 2001 From: Tom <41913303+t-a-ja...@users.noreply.github.com> Date: Fri, 22 Aug 2025 09:40:34 +0100 Subject: [PATCH 2/2] Update clang-tools-extra/clang-tidy/bugprone/MethodHidingCheck.cpp Co-authored-by: EugeneZelenko <eugene.zele...@gmail.com> --- clang-tools-extra/clang-tidy/bugprone/MethodHidingCheck.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang-tools-extra/clang-tidy/bugprone/MethodHidingCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/MethodHidingCheck.cpp index 865ef115950e6..52e2c4a9d2626 100644 --- a/clang-tools-extra/clang-tidy/bugprone/MethodHidingCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/MethodHidingCheck.cpp @@ -1,4 +1,4 @@ -//===--- MethodHidingCheck.cpp - clang-tidy ----------------------------===// +//===----------------------------------------------------------------------===// // // SPDX-FileCopyrightText: 2025 Siemens Corporation and/or its affiliates // _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
