njames93 updated this revision to Diff 456170.
njames93 marked an inline comment as done.
njames93 added a comment.
Fix documentation.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D132804/new/
https://reviews.llvm.org/D132804
Files:
clang-tools-extra/clang-tidy/readability/ConvertMemberFunctionsToStatic.cpp
clang-tools-extra/clang-tidy/readability/ConvertMemberFunctionsToStatic.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/readability/convert-member-functions-to-static.rst
clang-tools-extra/test/clang-tidy/checkers/readability/convert-member-functions-to-static-crtp.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/readability/convert-member-functions-to-static-crtp.cpp
===================================================================
--- /dev/null
+++ 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<typename T>
+class CRTP{
+ bool foo();
+ bool bar();
+};
+
+class CRTPInstance : public CRTP<CRTPInstance> {
+ // 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; }
+};
Index: clang-tools-extra/docs/clang-tidy/checks/readability/convert-member-functions-to-static.rst
===================================================================
--- clang-tools-extra/docs/clang-tidy/checks/readability/convert-member-functions-to-static.rst
+++ 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 <typename T>
+ struct CRTPBase {
+ bool shouldTraverse();
+ };
+
+ struct Derived : CRTPBase<Derived> {
+ bool shouldTraverse() { return false; }
+ };
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ 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
+ <clang-tidy/checks/readability/convert-member-functions-to-static>` check.
+
+ This check can now ignore CRTP pseudooverrides.
+
Removed checks
^^^^^^^^^^^^^^
Index: clang-tools-extra/clang-tidy/readability/ConvertMemberFunctionsToStatic.h
===================================================================
--- clang-tools-extra/clang-tidy/readability/ConvertMemberFunctionsToStatic.h
+++ 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
Index: clang-tools-extra/clang-tidy/readability/ConvertMemberFunctionsToStatic.cpp
===================================================================
--- clang-tools-extra/clang-tidy/readability/ConvertMemberFunctionsToStatic.cpp
+++ 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
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
