https://github.com/t-a-james updated https://github.com/llvm/llvm-project/pull/154746
>From 39ad4945d05f3e88fd32dfdea585aa99c6ba985f 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 01/10] [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 4fee4f93908da..8147aae4a0201 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| ==================================================== @@ -135,6 +141,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:`cppcoreguidelines-pro-bounds-avoid-unchecked-container-access <clang-tidy/checks/cppcoreguidelines/pro-bounds-avoid-unchecked-container-access>` 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 5e3ffc4f8aca3..3a942c18c80b0 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 b84fb6ce2ea527a9126e0a1f5b47b4c967dc43d9 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 02/10] 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 // >From 1fe1d5bc7944da1d3ab5fa9c71cf297346536427 Mon Sep 17 00:00:00 2001 From: Tom <41913303+t-a-ja...@users.noreply.github.com> Date: Fri, 22 Aug 2025 09:40:44 +0100 Subject: [PATCH 03/10] Update clang-tools-extra/clang-tidy/bugprone/MethodHidingCheck.h Co-authored-by: EugeneZelenko <eugene.zele...@gmail.com> --- clang-tools-extra/clang-tidy/bugprone/MethodHidingCheck.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang-tools-extra/clang-tidy/bugprone/MethodHidingCheck.h b/clang-tools-extra/clang-tidy/bugprone/MethodHidingCheck.h index f00f46cdf72e2..1ea5d656a67b9 100644 --- a/clang-tools-extra/clang-tidy/bugprone/MethodHidingCheck.h +++ b/clang-tools-extra/clang-tidy/bugprone/MethodHidingCheck.h @@ -1,4 +1,4 @@ -//===--- MethodHidingCheck.h - clang-tidy --------------------*- C++ -*-===// +//===----------------------------------------------------------------------===// // // SPDX-FileCopyrightText: 2025 Siemens Corporation and/or its affiliates // >From 316053382d50bbb0328d1b6216c132cc044b347b Mon Sep 17 00:00:00 2001 From: Tom James <tom.ja...@siemens.com> Date: Tue, 26 Aug 2025 11:07:35 +0100 Subject: [PATCH 04/10] Auto-format BugproneTidyModule.cpp --- clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp index d1d1979794743..950a0410edfdb 100644 --- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp @@ -157,8 +157,7 @@ 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<MethodHidingCheck>("bugprone-method-hiding"); CheckFactories.registerCheck<UnintendedCharOstreamOutputCheck>( "bugprone-unintended-char-ostream-output"); CheckFactories.registerCheck<ReturnConstRefFromParameterCheck>( >From e67dedae954c96ffa485f595543741ddbb6becaa Mon Sep 17 00:00:00 2001 From: Tom James <tom.ja...@siemens.com> Date: Tue, 26 Aug 2025 11:16:56 +0100 Subject: [PATCH 05/10] Remove unwanted copyright messages --- .../clang-tidy/bugprone/BugproneTidyModule.cpp | 3 --- clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt | 6 ------ .../clang-tidy/bugprone/MethodHidingCheck.cpp | 2 -- .../clang-tidy/bugprone/MethodHidingCheck.h | 2 -- clang-tools-extra/docs/ReleaseNotes.rst | 6 ------ .../docs/clang-tidy/checks/bugprone/method-hiding.rst | 5 ----- clang-tools-extra/docs/clang-tidy/checks/list.rst | 6 ------ .../clang-tidy/checkers/bugprone/method-hiding.cpp | 11 ----------- 8 files changed, 41 deletions(-) diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp index 950a0410edfdb..ac59b049ef46a 100644 --- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp @@ -4,9 +4,6 @@ // 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" diff --git a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt index 5745654855108..e2a2d3397902f 100644 --- a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt @@ -1,9 +1,3 @@ -# 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 diff --git a/clang-tools-extra/clang-tidy/bugprone/MethodHidingCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/MethodHidingCheck.cpp index 52e2c4a9d2626..c5bc8ca24d617 100644 --- a/clang-tools-extra/clang-tidy/bugprone/MethodHidingCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/MethodHidingCheck.cpp @@ -1,7 +1,5 @@ //===----------------------------------------------------------------------===// // -// 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 diff --git a/clang-tools-extra/clang-tidy/bugprone/MethodHidingCheck.h b/clang-tools-extra/clang-tidy/bugprone/MethodHidingCheck.h index 1ea5d656a67b9..f813e6d3f8ba5 100644 --- a/clang-tools-extra/clang-tidy/bugprone/MethodHidingCheck.h +++ b/clang-tools-extra/clang-tidy/bugprone/MethodHidingCheck.h @@ -1,7 +1,5 @@ //===----------------------------------------------------------------------===// // -// 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 diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 8147aae4a0201..e7b02e51efc5a 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -1,12 +1,6 @@ .. 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| ==================================================== 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 index dd58ec29e60c2..56d0ff71130d8 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/bugprone/method-hiding.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/method-hiding.rst @@ -1,8 +1,3 @@ -.. 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 diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst index 3a942c18c80b0..79fb3d70684db 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -1,9 +1,3 @@ -.. 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 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 index 38511c8807611..8aa5557dcd192 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/method-hiding.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/method-hiding.cpp @@ -1,16 +1,5 @@ -//===--- 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(); >From bd53bccd126954a8a7ed6c652b33003330dc6536 Mon Sep 17 00:00:00 2001 From: Tom <41913303+t-a-ja...@users.noreply.github.com> Date: Tue, 26 Aug 2025 15:36:48 +0100 Subject: [PATCH 06/10] 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 c5bc8ca24d617..1e8aec337f47c 100644 --- a/clang-tools-extra/clang-tidy/bugprone/MethodHidingCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/MethodHidingCheck.cpp @@ -132,7 +132,7 @@ void MethodHidingCheck::check(const MatchFinder::MatchResult &Result) { auto const MethodName = ShadowingMethod->getNameInfo().getAsString(); auto const DerivedClassName = DerivedClass->getNameAsString(); - std::string BaseClassName = BaseMethod->getParent()->getNameAsString(); + const std::string BaseClassName = BaseMethod->getParent()->getNameAsString(); std::string Message; llvm::raw_string_ostream StringStream(Message); >From e9157961bc29754840826d7dece7fe908cae8a74 Mon Sep 17 00:00:00 2001 From: Tom James <tom.ja...@siemens.com> Date: Tue, 26 Aug 2025 15:46:41 +0100 Subject: [PATCH 07/10] Remove unused variables --- .../clang-tidy/bugprone/MethodHidingCheck.cpp | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/clang-tools-extra/clang-tidy/bugprone/MethodHidingCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/MethodHidingCheck.cpp index 1e8aec337f47c..3b27cd0637d12 100644 --- a/clang-tools-extra/clang-tidy/bugprone/MethodHidingCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/MethodHidingCheck.cpp @@ -130,15 +130,10 @@ void MethodHidingCheck::check(const MatchFinder::MatchResult &Result) { llvm_unreachable("Required binding not found"); } - auto const MethodName = ShadowingMethod->getNameInfo().getAsString(); - auto const DerivedClassName = DerivedClass->getNameAsString(); - const 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(ShadowingMethod->getBeginLoc(), + "'" + ShadowingMethod->getQualifiedNameAsString() + + "' hides same method in '" + + BaseMethod->getParent()->getNameAsString() + "'"); diag(BaseMethod->getBeginLoc(), "previous definition is here", DiagnosticIDs::Note); } >From d22bfa404e79e4f566fb27a975c66a2166c635d1 Mon Sep 17 00:00:00 2001 From: Tom James <tom.ja...@siemens.com> Date: Wed, 27 Aug 2025 09:00:38 +0100 Subject: [PATCH 08/10] Remove braces --- .../clang-tidy/bugprone/MethodHidingCheck.cpp | 37 +++++++------------ 1 file changed, 13 insertions(+), 24 deletions(-) diff --git a/clang-tools-extra/clang-tidy/bugprone/MethodHidingCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/MethodHidingCheck.cpp index 3b27cd0637d12..e96b74428ba94 100644 --- a/clang-tools-extra/clang-tidy/bugprone/MethodHidingCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/MethodHidingCheck.cpp @@ -30,31 +30,23 @@ bool sameBasicType(ParmVarDecl const *Lhs, ParmVarDecl const *Rhs) { } bool namesCollide(CXXMethodDecl const &Lhs, CXXMethodDecl const &Rhs) { - if (Lhs.getNameAsString() != Rhs.getNameAsString()) { + if (Lhs.getNameAsString() != Rhs.getNameAsString()) return false; - } - if (Lhs.isConst() != Rhs.isConst()) { + if (Lhs.isConst() != Rhs.isConst()) return false; - } - if (Lhs.getNumParams() != Rhs.getNumParams()) { + if (Lhs.getNumParams() != Rhs.getNumParams()) return false; - } - for (unsigned int It = 0; It < Lhs.getNumParams(); ++It) { - if (!sameBasicType(Lhs.getParamDecl(It), Rhs.getParamDecl(It))) { + 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()) { + if (Lhs.isTemplated() || Rhs.isTemplated()) return false; - } - if (Lhs.isTemplateInstantiation() || Rhs.isTemplateInstantiation()) { + if (Lhs.isTemplateInstantiation() || Rhs.isTemplateInstantiation()) return false; - } if (Lhs.isFunctionTemplateSpecialization() || - Rhs.isFunctionTemplateSpecialization()) { + Rhs.isFunctionTemplateSpecialization()) return false; - } return true; } @@ -68,15 +60,13 @@ AST_MATCHER(CXXMethodDecl, nameCollidesWithMethodInBase) { Stack.pop(); if (CurrentBaseSpec->getAccessSpecifier() == - clang::AccessSpecifier::AS_private) { + clang::AccessSpecifier::AS_private) continue; - } const auto *CurrentRecord = CurrentBaseSpec->getType()->getAsCXXRecordDecl(); - if (!CurrentRecord) { + if (!CurrentRecord) continue; - } for (auto const &BaseMethod : CurrentRecord->methods()) { if (namesCollide(*BaseMethod, Node)) { @@ -87,9 +77,9 @@ AST_MATCHER(CXXMethodDecl, nameCollidesWithMethodInBase) { } } - for (auto const &SubBase : CurrentRecord->bases()) { + for (auto const &SubBase : CurrentRecord->bases()) Stack.push(&SubBase); - } + } } return false; @@ -126,9 +116,8 @@ void MethodHidingCheck::check(const MatchFinder::MatchResult &Result) { Result.Nodes.getNodeAs<CXXRecordDecl>("derived_class"); auto const *BaseMethod = Result.Nodes.getNodeAs<CXXMethodDecl>("base_method"); - if (!ShadowingMethod || !DerivedClass || !BaseMethod) { + if (!ShadowingMethod || !DerivedClass || !BaseMethod) llvm_unreachable("Required binding not found"); - } diag(ShadowingMethod->getBeginLoc(), "'" + ShadowingMethod->getQualifiedNameAsString() + >From 19465b6b2c615f4f9e394b97f17118bf7afc9093 Mon Sep 17 00:00:00 2001 From: Tom James <tom.ja...@siemens.com> Date: Wed, 27 Aug 2025 09:48:30 +0100 Subject: [PATCH 09/10] Give an example in the documentation --- .../checks/bugprone/method-hiding.rst | 22 ++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) 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 index 56d0ff71130d8..a4a2436118b44 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/bugprone/method-hiding.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/method-hiding.rst @@ -7,4 +7,24 @@ 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 +Only checks public, non-templated methods. + +The below example is bugprone because consumers of the `Derived` class will +expect the `reset` method to do the work of `Base::reset()` in addition to extra +work required to reset the `Derived` class. Common fixes include: +- Making the `reset` method polymorphic +- Re-naming `Derived::reset` if it's not meant to intersect with `Base::reset` +- Using `using Base::reset` to change the access specifier + +This is also a violation of the Liskov Substitution Principle. + +.. code-block:: c++ + + class Base { + void reset() {/* reset the base class */}; + }; + + class Derived : public Base { + public: + void reset() {/* reset the derived class, but not the base class */}; + }; \ No newline at end of file >From d337e8206eb78d47d7122dde41310e3e1ba0277c Mon Sep 17 00:00:00 2001 From: Tom James <tom.ja...@siemens.com> Date: Wed, 27 Aug 2025 09:52:43 +0100 Subject: [PATCH 10/10] West const everywhere --- .../clang-tidy/bugprone/MethodHidingCheck.cpp | 23 +++++++++---------- 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/clang-tools-extra/clang-tidy/bugprone/MethodHidingCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/MethodHidingCheck.cpp index e96b74428ba94..edcd929c3e2e5 100644 --- a/clang-tools-extra/clang-tidy/bugprone/MethodHidingCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/MethodHidingCheck.cpp @@ -16,7 +16,7 @@ namespace clang::tidy::bugprone { namespace { -bool sameBasicType(ParmVarDecl const *Lhs, ParmVarDecl const *Rhs) { +bool sameBasicType(const ParmVarDecl *Lhs, const ParmVarDecl *Rhs) { if (Lhs && Rhs) { return Lhs->getType() .getCanonicalType() @@ -29,7 +29,7 @@ bool sameBasicType(ParmVarDecl const *Lhs, ParmVarDecl const *Rhs) { return false; } -bool namesCollide(CXXMethodDecl const &Lhs, CXXMethodDecl const &Rhs) { +bool namesCollide(const CXXMethodDecl &Lhs, const CXXMethodDecl &Rhs) { if (Lhs.getNameAsString() != Rhs.getNameAsString()) return false; if (Lhs.isConst() != Rhs.isConst()) @@ -51,12 +51,12 @@ bool namesCollide(CXXMethodDecl const &Lhs, CXXMethodDecl const &Rhs) { } AST_MATCHER(CXXMethodDecl, nameCollidesWithMethodInBase) { - CXXRecordDecl const *DerivedClass = Node.getParent(); - for (auto const &Base : DerivedClass->bases()) { - std::stack<CXXBaseSpecifier const *> Stack; + const CXXRecordDecl *DerivedClass = Node.getParent(); + for (const auto &Base : DerivedClass->bases()) { + std::stack<const CXXBaseSpecifier *> Stack; Stack.push(&Base); while (!Stack.empty()) { - CXXBaseSpecifier const *CurrentBaseSpec = Stack.top(); + const CXXBaseSpecifier *CurrentBaseSpec = Stack.top(); Stack.pop(); if (CurrentBaseSpec->getAccessSpecifier() == @@ -68,7 +68,7 @@ AST_MATCHER(CXXMethodDecl, nameCollidesWithMethodInBase) { if (!CurrentRecord) continue; - for (auto const &BaseMethod : CurrentRecord->methods()) { + for (const auto &BaseMethod : CurrentRecord->methods()) { if (namesCollide(*BaseMethod, Node)) { ast_matchers::internal::BoundNodesTreeBuilder Result(*Builder); Builder->setBinding("base_method", @@ -77,9 +77,8 @@ AST_MATCHER(CXXMethodDecl, nameCollidesWithMethodInBase) { } } - for (auto const &SubBase : CurrentRecord->bases()) + for (const auto &SubBase : CurrentRecord->bases()) Stack.push(&SubBase); - } } return false; @@ -110,11 +109,11 @@ void MethodHidingCheck::registerMatchers(MatchFinder *Finder) { } void MethodHidingCheck::check(const MatchFinder::MatchResult &Result) { - auto const *ShadowingMethod = + const auto *ShadowingMethod = Result.Nodes.getNodeAs<CXXMethodDecl>("shadowing_method"); - auto const *DerivedClass = + const auto *DerivedClass = Result.Nodes.getNodeAs<CXXRecordDecl>("derived_class"); - auto const *BaseMethod = Result.Nodes.getNodeAs<CXXMethodDecl>("base_method"); + const auto *BaseMethod = Result.Nodes.getNodeAs<CXXMethodDecl>("base_method"); if (!ShadowingMethod || !DerivedClass || !BaseMethod) llvm_unreachable("Required binding not found"); _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
