[clang-tools-extra] r366265 - [clang-tidy] initial version of readability-convert-member-functions-to-static
Matthias Gehre via cfe-commits
cfe-commits at lists.llvm.org
Tue Jul 16 14:19:01 PDT 2019
Author: mgehre
Date: Tue Jul 16 14:19:00 2019
New Revision: 366265
URL: http://llvm.org/viewvc/llvm-project?rev=366265&view=rev
Log:
[clang-tidy] initial version of readability-convert-member-functions-to-static
Summary:
Finds non-static member functions that can be made ``static``.
I have run this check (repeatedly) over llvm-project. It made 1708 member functions
``static``. Out of those, I had to exclude 22 via ``NOLINT`` because their address
was taken and stored in a variable of pointer-to-member type (e.g. passed to
llvm::StringSwitch).
It also made 243 member functions ``const``. (This is currently very conservative
to have no false-positives and can hopefully be extended in the future.)
You can find the results here: https://github.com/mgehre/llvm-project/commits/static_const_eval
Reviewers: alexfh, aaron.ballman
Subscribers: mgorny, xazax.hun, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D61749
Added:
clang-tools-extra/trunk/clang-tidy/readability/ConvertMemberFunctionsToStatic.cpp
clang-tools-extra/trunk/clang-tidy/readability/ConvertMemberFunctionsToStatic.h
clang-tools-extra/trunk/docs/clang-tidy/checks/readability-convert-member-functions-to-static.rst
clang-tools-extra/trunk/test/clang-tidy/readability-convert-member-functions-to-static.cpp
Modified:
clang-tools-extra/trunk/clang-tidy/readability/CMakeLists.txt
clang-tools-extra/trunk/clang-tidy/readability/ReadabilityTidyModule.cpp
clang-tools-extra/trunk/docs/ReleaseNotes.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
Modified: clang-tools-extra/trunk/clang-tidy/readability/CMakeLists.txt
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/readability/CMakeLists.txt?rev=366265&r1=366264&r2=366265&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/readability/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clang-tidy/readability/CMakeLists.txt Tue Jul 16 14:19:00 2019
@@ -5,6 +5,7 @@ add_clang_library(clangTidyReadabilityMo
BracesAroundStatementsCheck.cpp
ConstReturnTypeCheck.cpp
ContainerSizeEmptyCheck.cpp
+ ConvertMemberFunctionsToStatic.cpp
DeleteNullPointerCheck.cpp
DeletedDefaultCheck.cpp
ElseAfterReturnCheck.cpp
Added: clang-tools-extra/trunk/clang-tidy/readability/ConvertMemberFunctionsToStatic.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/readability/ConvertMemberFunctionsToStatic.cpp?rev=366265&view=auto
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/readability/ConvertMemberFunctionsToStatic.cpp (added)
+++ clang-tools-extra/trunk/clang-tidy/readability/ConvertMemberFunctionsToStatic.cpp Tue Jul 16 14:19:00 2019
@@ -0,0 +1,172 @@
+//===--- ConvertMemberFunctionsToStatic.cpp - clang-tidy ------------------===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "ConvertMemberFunctionsToStatic.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/DeclCXX.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Basic/SourceLocation.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace readability {
+
+AST_MATCHER(CXXMethodDecl, isStatic) { return Node.isStatic(); }
+
+AST_MATCHER(CXXMethodDecl, hasTrivialBody) { return Node.hasTrivialBody(); }
+
+AST_MATCHER(CXXMethodDecl, isOverloadedOperator) {
+ return Node.isOverloadedOperator();
+}
+
+AST_MATCHER(CXXRecordDecl, hasAnyDependentBases) {
+ return Node.hasAnyDependentBases();
+}
+
+AST_MATCHER(CXXMethodDecl, isTemplate) {
+ return Node.getTemplatedKind() != FunctionDecl::TK_NonTemplate;
+}
+
+AST_MATCHER(CXXMethodDecl, isDependentContext) {
+ return Node.isDependentContext();
+}
+
+AST_MATCHER(CXXMethodDecl, isInsideMacroDefinition) {
+ const ASTContext &Ctxt = Finder->getASTContext();
+ return clang::Lexer::makeFileCharRange(
+ clang::CharSourceRange::getCharRange(
+ Node.getTypeSourceInfo()->getTypeLoc().getSourceRange()),
+ Ctxt.getSourceManager(), Ctxt.getLangOpts())
+ .isInvalid();
+}
+
+AST_MATCHER_P(CXXMethodDecl, hasCanonicalDecl,
+ ast_matchers::internal::Matcher<CXXMethodDecl>, InnerMatcher) {
+ return InnerMatcher.matches(*Node.getCanonicalDecl(), Finder, Builder);
+}
+
+AST_MATCHER(CXXMethodDecl, usesThis) {
+ class FindUsageOfThis : public RecursiveASTVisitor<FindUsageOfThis> {
+ public:
+ bool Used = false;
+
+ bool VisitCXXThisExpr(const CXXThisExpr *E) {
+ Used = true;
+ return false; // Stop traversal.
+ }
+ } UsageOfThis;
+
+ // TraverseStmt does not modify its argument.
+ UsageOfThis.TraverseStmt(const_cast<Stmt *>(Node.getBody()));
+
+ return UsageOfThis.Used;
+}
+
+void ConvertMemberFunctionsToStatic::registerMatchers(MatchFinder *Finder) {
+ Finder->addMatcher(
+ cxxMethodDecl(
+ isDefinition(), isUserProvided(),
+ unless(anyOf(
+ isExpansionInSystemHeader(), isVirtual(), isStatic(),
+ hasTrivialBody(), isOverloadedOperator(), cxxConstructorDecl(),
+ cxxDestructorDecl(), cxxConversionDecl(), isTemplate(),
+ isDependentContext(),
+ ofClass(anyOf(
+ isLambda(),
+ hasAnyDependentBases()) // Method might become virtual
+ // depending on template base class.
+ ),
+ isInsideMacroDefinition(),
+ hasCanonicalDecl(isInsideMacroDefinition()), usesThis())))
+ .bind("x"),
+ this);
+}
+
+/// \brief Obtain the original source code text from a SourceRange.
+static StringRef getStringFromRange(SourceManager &SourceMgr,
+ const LangOptions &LangOpts,
+ SourceRange Range) {
+ if (SourceMgr.getFileID(Range.getBegin()) !=
+ SourceMgr.getFileID(Range.getEnd()))
+ return {};
+
+ return Lexer::getSourceText(CharSourceRange(Range, true), SourceMgr,
+ LangOpts);
+}
+
+static SourceRange getLocationOfConst(const TypeSourceInfo *TSI,
+ SourceManager &SourceMgr,
+ const LangOptions &LangOpts) {
+ assert(TSI);
+ const auto FTL = TSI->getTypeLoc().IgnoreParens().getAs<FunctionTypeLoc>();
+ assert(FTL);
+
+ SourceRange Range{FTL.getRParenLoc().getLocWithOffset(1),
+ FTL.getLocalRangeEnd()};
+ // Inside Range, there might be other keywords and trailing return types.
+ // Find the exact position of "const".
+ StringRef Text = getStringFromRange(SourceMgr, LangOpts, Range);
+ size_t Offset = Text.find("const");
+ if (Offset == StringRef::npos)
+ return {};
+
+ SourceLocation Start = Range.getBegin().getLocWithOffset(Offset);
+ return {Start, Start.getLocWithOffset(strlen("const") - 1)};
+}
+
+void ConvertMemberFunctionsToStatic::check(
+ const MatchFinder::MatchResult &Result) {
+ const auto *Definition = Result.Nodes.getNodeAs<CXXMethodDecl>("x");
+
+ // TODO: For out-of-line declarations, don't modify the source if the header
+ // is excluded by the -header-filter option.
+ DiagnosticBuilder Diag =
+ diag(Definition->getLocation(), "method %0 can be made static")
+ << Definition;
+
+ // TODO: Would need to remove those in a fix-it.
+ if (Definition->getMethodQualifiers().hasVolatile() ||
+ Definition->getMethodQualifiers().hasRestrict() ||
+ Definition->getRefQualifier() != RQ_None)
+ return;
+
+ const CXXMethodDecl *Declaration = Definition->getCanonicalDecl();
+
+ if (Definition->isConst()) {
+ // Make sure that we either remove 'const' on both declaration and
+ // definition or emit no fix-it at all.
+ SourceRange DefConst = getLocationOfConst(Definition->getTypeSourceInfo(),
+ *Result.SourceManager,
+ Result.Context->getLangOpts());
+
+ if (DefConst.isInvalid())
+ return;
+
+ if (Declaration != Definition) {
+ SourceRange DeclConst = getLocationOfConst(
+ Declaration->getTypeSourceInfo(), *Result.SourceManager,
+ Result.Context->getLangOpts());
+
+ if (DeclConst.isInvalid())
+ return;
+ Diag << FixItHint::CreateRemoval(DeclConst);
+ }
+
+ // Remove existing 'const' from both declaration and definition.
+ Diag << FixItHint::CreateRemoval(DefConst);
+ }
+ Diag << FixItHint::CreateInsertion(Declaration->getBeginLoc(), "static ");
+}
+
+} // namespace readability
+} // namespace tidy
+} // namespace clang
Added: clang-tools-extra/trunk/clang-tidy/readability/ConvertMemberFunctionsToStatic.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/readability/ConvertMemberFunctionsToStatic.h?rev=366265&view=auto
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/readability/ConvertMemberFunctionsToStatic.h (added)
+++ clang-tools-extra/trunk/clang-tidy/readability/ConvertMemberFunctionsToStatic.h Tue Jul 16 14:19:00 2019
@@ -0,0 +1,37 @@
+//===--- ConvertMemberFunctionsToStatic.h - clang-tidy ----------*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_CONVERTMEMFUNCTOSTATIC_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_CONVERTMEMFUNCTOSTATIC_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace readability {
+
+/// This check finds C++ class methods than can be made static
+/// because they don't use the 'this' pointer.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/
+/// readability-convert-member-functions-to-static.html
+class ConvertMemberFunctionsToStatic : public ClangTidyCheck {
+public:
+ ConvertMemberFunctionsToStatic(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context) {}
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace readability
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_CONVERTMEMFUNCTOSTATIC_H
Modified: clang-tools-extra/trunk/clang-tidy/readability/ReadabilityTidyModule.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/readability/ReadabilityTidyModule.cpp?rev=366265&r1=366264&r2=366265&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/readability/ReadabilityTidyModule.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/readability/ReadabilityTidyModule.cpp Tue Jul 16 14:19:00 2019
@@ -13,6 +13,7 @@
#include "BracesAroundStatementsCheck.h"
#include "ConstReturnTypeCheck.h"
#include "ContainerSizeEmptyCheck.h"
+#include "ConvertMemberFunctionsToStatic.h"
#include "DeleteNullPointerCheck.h"
#include "DeletedDefaultCheck.h"
#include "ElseAfterReturnCheck.h"
@@ -57,6 +58,8 @@ public:
"readability-const-return-type");
CheckFactories.registerCheck<ContainerSizeEmptyCheck>(
"readability-container-size-empty");
+ CheckFactories.registerCheck<ConvertMemberFunctionsToStatic>(
+ "readability-convert-member-functions-to-static");
CheckFactories.registerCheck<DeleteNullPointerCheck>(
"readability-delete-null-pointer");
CheckFactories.registerCheck<DeletedDefaultCheck>(
Modified: clang-tools-extra/trunk/docs/ReleaseNotes.rst
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/ReleaseNotes.rst?rev=366265&r1=366264&r2=366265&view=diff
==============================================================================
--- clang-tools-extra/trunk/docs/ReleaseNotes.rst (original)
+++ clang-tools-extra/trunk/docs/ReleaseNotes.rst Tue Jul 16 14:19:00 2019
@@ -230,6 +230,11 @@ Improvements to clang-tidy
If set to true, the check will provide fix-its with literal initializers
(``int i = 0;``) instead of curly braces (``int i{};``).
+- New :doc:`readability-convert-member-functions-to-static
+ <clang-tidy/checks/readability-convert-member-functions-to-static>` check.
+
+ Finds non-static member functions that can be made ``static``.
+
Improvements to include-fixer
-----------------------------
Modified: clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst?rev=366265&r1=366264&r2=366265&view=diff
==============================================================================
--- clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst (original)
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst Tue Jul 16 14:19:00 2019
@@ -257,6 +257,7 @@ Clang-Tidy Checks
readability-braces-around-statements
readability-const-return-type
readability-container-size-empty
+ readability-convert-member-functions-to-static
readability-delete-null-pointer
readability-deleted-default
readability-else-after-return
Added: clang-tools-extra/trunk/docs/clang-tidy/checks/readability-convert-member-functions-to-static.rst
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/readability-convert-member-functions-to-static.rst?rev=366265&view=auto
==============================================================================
--- clang-tools-extra/trunk/docs/clang-tidy/checks/readability-convert-member-functions-to-static.rst (added)
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/readability-convert-member-functions-to-static.rst Tue Jul 16 14:19:00 2019
@@ -0,0 +1,14 @@
+.. title:: clang-tidy - readability-convert-member-functions-to-static
+
+readability-convert-member-functions-to-static
+==============================================
+
+Finds non-static member functions that can be made ``static``
+because the functions don't use ``this``.
+
+After applying modifications as suggested by the check, runnnig the check again
+might find more opportunities to mark member functions ``static``.
+
+After making a member function ``static``, you might want to run the check
+`readability-static-accessed-through-instance` to replace calls like
+``Instance.method()`` by ``Class::method()``.
Added: clang-tools-extra/trunk/test/clang-tidy/readability-convert-member-functions-to-static.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/readability-convert-member-functions-to-static.cpp?rev=366265&view=auto
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/readability-convert-member-functions-to-static.cpp (added)
+++ clang-tools-extra/trunk/test/clang-tidy/readability-convert-member-functions-to-static.cpp Tue Jul 16 14:19:00 2019
@@ -0,0 +1,218 @@
+// RUN: %check_clang_tidy %s readability-convert-member-functions-to-static %t
+
+class DoNotMakeEmptyStatic {
+ void emptyMethod() {}
+ void empty_method_out_of_line();
+};
+
+void DoNotMakeEmptyStatic::empty_method_out_of_line() {}
+
+class A {
+ int field;
+ const int const_field;
+ static int static_field;
+
+ void no_use() {
+ // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: method 'no_use' can be made static
+ // CHECK-FIXES: {{^}} static void no_use() {
+ int i = 1;
+ }
+
+ int read_field() {
+ return field;
+ }
+
+ void write_field() {
+ field = 1;
+ }
+
+ int call_non_const_member() { return read_field(); }
+
+ int call_static_member() {
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: method 'call_static_member' can be made static
+ // CHECK-FIXES: {{^}} static int call_static_member() {
+ already_static();
+ }
+
+ int read_static() {
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: method 'read_static' can be made static
+ // CHECK-FIXES: {{^}} static int read_static() {
+ return static_field;
+ }
+ void write_static() {
+ // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: method 'write_static' can be made static
+ // CHECK-FIXES: {{^}} static void write_static() {
+ static_field = 1;
+ }
+
+ static int already_static() { return static_field; }
+
+ int already_const() const { return field; }
+
+ int already_const_convert_to_static() const {
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: method 'already_const_convert_to_static' can be made static
+ // CHECK-FIXES: {{^}} static int already_const_convert_to_static() {
+ return static_field;
+ }
+
+ static int out_of_line_already_static();
+
+ void out_of_line_call_static();
+ // CHECK-FIXES: {{^}} static void out_of_line_call_static();
+ int out_of_line_const_to_static() const;
+ // CHECK-FIXES: {{^}} static int out_of_line_const_to_static() ;
+};
+
+int A::out_of_line_already_static() { return 0; }
+
+void A::out_of_line_call_static() {
+ // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: method 'out_of_line_call_static' can be made static
+ // CHECK-FIXES: {{^}}void A::out_of_line_call_static() {
+ already_static();
+}
+
+int A::out_of_line_const_to_static() const {
+ // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: method 'out_of_line_const_to_static' can be made static
+ // CHECK-FIXES: {{^}}int A::out_of_line_const_to_static() {
+ return 0;
+}
+
+struct KeepVirtual {
+ virtual int f() { return 0; }
+ virtual int h() const { return 0; }
+};
+
+struct KeepVirtualDerived : public KeepVirtual {
+ int f() { return 0; }
+ int h() const override { return 0; }
+};
+
+// Don't add 'static' to special member functions and operators.
+struct KeepSpecial {
+ KeepSpecial() { int L = 0; }
+ ~KeepSpecial() { int L = 0; }
+ int operator+() { return 0; }
+ operator int() { return 0; }
+};
+
+void KeepLambdas() {
+ using FT = int (*)();
+ auto F = static_cast<FT>([]() { return 0; });
+ auto F2 = []() { return 0; };
+}
+
+template <class Base>
+struct KeepWithTemplateBase : public Base {
+ int i;
+ // We cannot make these methods static because they might need to override
+ // a function from Base.
+ int static_f() { return 0; }
+};
+
+template <class T>
+struct KeepTemplateClass {
+ int i;
+ // We cannot make these methods static because a specialization
+ // might use *this differently.
+ int static_f() { return 0; }
+};
+
+struct KeepTemplateMethod {
+ int i;
+ // We cannot make these methods static because a specialization
+ // might use *this differently.
+ template <class T>
+ static int static_f() { return 0; }
+};
+
+void instantiate() {
+ struct S {};
+ KeepWithTemplateBase<S> I1;
+ I1.static_f();
+
+ KeepTemplateClass<int> I2;
+ I2.static_f();
+
+ KeepTemplateMethod I3;
+ I3.static_f<int>();
+}
+
+struct Trailing {
+ auto g() const -> int {
+ // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: method 'g' can be made static
+ // CHECK-FIXES: {{^}} static auto g() -> int {
+ return 0;
+ }
+
+ void vol() volatile {
+ // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: method 'vol' can be made static
+ return;
+ }
+
+ void ref() const & {
+ // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: method 'ref' can be made static
+ return;
+ }
+ void refref() const && {
+ // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: method 'refref' can be made static
+ return;
+ }
+
+ void restr() __restrict {
+ // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: method 'restr' can be made static
+ return;
+ }
+};
+
+struct UnevaluatedContext {
+ void f() { sizeof(this); }
+
+ void noex() noexcept(noexcept(this));
+};
+
+struct LambdaCapturesThis {
+ int Field;
+
+ int explicitCapture() {
+ return [this]() { return Field; }();
+ }
+
+ int implicitCapture() {
+ return [&]() { return Field; }();
+ }
+};
+
+struct NoFixitInMacro {
+#define CONST const
+ int no_use_macro_const() CONST {
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: method 'no_use_macro_const' can be made static
+ return 0;
+ }
+
+#define ADD_CONST(F) F const
+ int ADD_CONST(no_use_macro2()) {
+ return 0;
+ }
+
+#define FUN no_use_macro()
+ int i;
+ int FUN {
+ return i;
+ }
+
+#define T(FunctionName, Keyword) \
+ Keyword int FunctionName() { return 0; }
+#define EMPTY
+ T(A, EMPTY)
+ T(B, static)
+
+#define T2(FunctionName) \
+ int FunctionName() { return 0; }
+ T2(A2)
+
+#define VOLATILE volatile
+ void volatileMacro() VOLATILE {
+ // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: method 'volatileMacro' can be made static
+ return;
+ }
+};
More information about the cfe-commits
mailing list