diff --git a/clang-tools-extra/clang-tidy/readability/ConvertMemberFunctionsToStatic.h b/clang-tools-extra/clang-tidy/readability/ConvertMemberFunctionsToStatic.h --- a/clang-tools-extra/clang-tidy/readability/ConvertMemberFunctionsToStatic.h +++ b/clang-tools-extra/clang-tidy/readability/ConvertMemberFunctionsToStatic.h @@ -23,10 +23,13 @@ /// readability-convert-member-functions-to-static.html class ConvertMemberFunctionsToStatic : public ClangTidyCheck { public: - ConvertMemberFunctionsToStatic(StringRef Name, ClangTidyContext *Context) - : ClangTidyCheck(Name, Context) {} + ConvertMemberFunctionsToStatic(StringRef Name, ClangTidyContext *Context); void registerMatchers(ast_matchers::MatchFinder *Finder) override; void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + void storeOptions(ClangTidyOptions::OptionMap &Options) override; + +private: + const bool IgnorePseudoOverrides; }; } // namespace readability diff --git a/clang-tools-extra/clang-tidy/readability/ConvertMemberFunctionsToStatic.cpp b/clang-tools-extra/clang-tidy/readability/ConvertMemberFunctionsToStatic.cpp --- a/clang-tools-extra/clang-tidy/readability/ConvertMemberFunctionsToStatic.cpp +++ b/clang-tools-extra/clang-tidy/readability/ConvertMemberFunctionsToStatic.cpp @@ -71,6 +71,15 @@ return UsageOfThis.Used; } +AST_MATCHER(CXXMethodDecl, potentialCTRPOverride) { + for (const CXXBaseSpecifier &Base : Node.getParent()->bases()) { + if (const auto *RD = Base.getType()->getAsCXXRecordDecl()) + if (RD->hasMemberName(Node.getDeclName())) + return true; + } + return false; +} + void ConvertMemberFunctionsToStatic::registerMatchers(MatchFinder *Finder) { Finder->addMatcher( cxxMethodDecl( @@ -85,6 +94,8 @@ hasAnyDependentBases()) // Method might become virtual // depending on template base class. ), + IgnorePseudoOverrides ? potentialCTRPOverride() + : unless(anything()), isInsideMacroDefinition(), hasCanonicalDecl(isInsideMacroDefinition()), usesThis()))) .bind("x"), @@ -167,6 +178,15 @@ Diag << FixItHint::CreateInsertion(Declaration->getBeginLoc(), "static "); } +ConvertMemberFunctionsToStatic::ConvertMemberFunctionsToStatic( + StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), + IgnorePseudoOverrides(Options.get("IgnorePseudoOverrides", false)) {} + +void ConvertMemberFunctionsToStatic::storeOptions( + ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "IgnorePseudoOverrides", IgnorePseudoOverrides); +} } // namespace readability } // namespace tidy } // namespace clang diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -141,6 +141,11 @@ The check now skips unions since in this case a default constructor with empty body is not equivalent to the explicitly defaulted one. +- Improved :doc:`readability-convert-member-functions-to-static + ` check. + + This check can now ignore CRTP pseudooverrides. + Removed checks ^^^^^^^^^^^^^^ diff --git a/clang-tools-extra/docs/clang-tidy/checks/readability/convert-member-functions-to-static.rst b/clang-tools-extra/docs/clang-tidy/checks/readability/convert-member-functions-to-static.rst --- a/clang-tools-extra/docs/clang-tidy/checks/readability/convert-member-functions-to-static.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/readability/convert-member-functions-to-static.rst @@ -12,3 +12,26 @@ After making a member function ``static``, you might want to run the check `readability-static-accessed-through-instance <../readability/static-accessed-through-instance.html>`_ to replace calls like ``Instance.method()`` by ``Class::method()``. + +Options +------- + +.. option:: IgnorePseudoOverrides + + When `true`, if there is a base class with a method with the same name, no + warning will be emitted. Often occurs with CRTP like classes. + For the case below a warning would only be emitted for + ``Derived::shouldTraverse`` if this is set to `false`. + + Default value is `false` + + .. code-block:: c++ + + template + struct CRTPBase { + bool shouldTraverse(); + }; + + struct Derived : CRTPBase { + bool shouldTraverse() { return false; } + }; diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/convert-member-functions-to-static-crtp.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/convert-member-functions-to-static-crtp.cpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/readability/convert-member-functions-to-static-crtp.cpp @@ -0,0 +1,19 @@ +// If the option is true, expect no warnigs, however the check_clang_tidy script can't handle that case. +// RUN: clang-tidy %s -checks=-*,readability-convert-member-functions-to-static \ +// RUN: -config='{CheckOptions: {readability-convert-member-functions-to-static.IgnorePseudoOverrides: true}}' \ +// RUN: -- -std=c++14 | count 0 + +// RUN: %check_clang_tidy %s readability-convert-member-functions-to-static %t -- \ +// RUN: -config='{CheckOptions: {readability-convert-member-functions-to-static.IgnorePseudoOverrides: false}}' +template +class CRTP{ + bool foo(); + bool bar(); +}; + +class CRTPInstance : public CRTP { + // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: method 'foo' can be made static + bool foo() { return false; } + // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: method 'bar' can be made static + void bar() { return; } +};