This revision was automatically updated to reflect the committed changes.
Closed by commit rL310371: [clang-tidy] Add new readability non-idiomatic
static access check (authored by xazax).
Changed prior to commit:
https://reviews.llvm.org/D35937?vs=110175&id=110209#toc
Repository:
rL LLVM
https://reviews.llvm.org/D35937
Files:
clang-tools-extra/trunk/clang-tidy/readability/CMakeLists.txt
clang-tools-extra/trunk/clang-tidy/readability/ReadabilityTidyModule.cpp
clang-tools-extra/trunk/clang-tidy/readability/StaticAccessedThroughInstanceCheck.cpp
clang-tools-extra/trunk/clang-tidy/readability/StaticAccessedThroughInstanceCheck.h
clang-tools-extra/trunk/docs/ReleaseNotes.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/readability-static-accessed-through-instance.rst
clang-tools-extra/trunk/test/clang-tidy/readability-static-accessed-through-instance-nesting-threshold.cpp
clang-tools-extra/trunk/test/clang-tidy/readability-static-accessed-through-instance.cpp
Index: clang-tools-extra/trunk/test/clang-tidy/readability-static-accessed-through-instance.cpp
===================================================================
--- clang-tools-extra/trunk/test/clang-tidy/readability-static-accessed-through-instance.cpp
+++ clang-tools-extra/trunk/test/clang-tidy/readability-static-accessed-through-instance.cpp
@@ -0,0 +1,222 @@
+// RUN: %check_clang_tidy %s readability-static-accessed-through-instance %t
+
+struct C {
+ static void foo();
+ static int x;
+ int nsx;
+ void mf() {
+ (void)&x; // OK, x is accessed inside the struct.
+ (void)&C::x; // OK, x is accessed using a qualified-id.
+ foo(); // OK, foo() is accessed inside the struct.
+ }
+ void ns() const;
+};
+
+int C::x = 0;
+
+struct CC {
+ void foo();
+ int x;
+};
+
+template <typename T> struct CT {
+ static T foo();
+ static T x;
+ int nsx;
+ void mf() {
+ (void)&x; // OK, x is accessed inside the struct.
+ (void)&C::x; // OK, x is accessed using a qualified-id.
+ foo(); // OK, foo() is accessed inside the struct.
+ }
+};
+
+// Expressions with side effects
+C &f(int, int, int, int);
+void g() {
+ f(1, 2, 3, 4).x;
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: static member accessed through instance [readability-static-accessed-through-instance]
+ // CHECK-FIXES: {{^}} f(1, 2, 3, 4).x;{{$}}
+}
+
+int i(int &);
+void j(int);
+C h();
+bool a();
+int k(bool);
+
+void f(C c) {
+ j(i(h().x));
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: static member
+ // CHECK-FIXES: {{^}} j(i(h().x));{{$}}
+
+ // The execution of h() depends on the return value of a().
+ j(k(a() && h().x));
+ // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: static member
+ // CHECK-FIXES: {{^}} j(k(a() && h().x));{{$}}
+
+ if ([c]() {
+ c.ns();
+ return c;
+ }().x == 15)
+ ;
+ // CHECK-MESSAGES: :[[@LINE-5]]:7: warning: static member
+ // CHECK-FIXES: {{^}} if ([c]() {{{$}}
+}
+
+// Nested specifiers
+namespace N {
+struct V {
+ static int v;
+ struct T {
+ static int t;
+ struct U {
+ static int u;
+ };
+ };
+};
+}
+
+void f(N::V::T::U u) {
+ N::V v;
+ v.v = 12;
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: static member
+ // CHECK-FIXES: {{^}} N::V::v = 12;{{$}}
+
+ N::V::T w;
+ w.t = 12;
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: static member
+ // CHECK-FIXES: {{^}} N::V::T::t = 12;{{$}}
+
+ // u.u is not changed to N::V::T::U::u; because the nesting level is over 3.
+ u.u = 12;
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: static member
+ // CHECK-FIXES: {{^}} u.u = 12;{{$}}
+
+ using B = N::V::T::U;
+ B b;
+ b.u;
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: static member
+ // CHECK-FIXES: {{^}} B::u;{{$}}
+}
+
+// Templates
+template <typename T> T CT<T>::x;
+
+template <typename T> struct CCT {
+ T foo();
+ T x;
+};
+
+typedef C D;
+
+using E = D;
+
+#define FOO(c) c.foo()
+#define X(c) c.x
+
+template <typename T> void f(T t, C c) {
+ t.x; // OK, t is a template parameter.
+ c.x; // 1
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: static member
+ // CHECK-FIXES: {{^}} C::x; // 1{{$}}
+}
+
+template <int N> struct S { static int x; };
+
+template <> struct S<0> { int x; };
+
+template <int N> void h() {
+ S<N> sN;
+ sN.x; // OK, value of N affects whether x is static or not.
+
+ S<2> s2;
+ s2.x;
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: static member
+ // CHECK-FIXES: {{^}} S<2>::x;{{$}}
+}
+
+void static_through_instance() {
+ C *c1 = new C();
+ c1->foo(); // 1
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: static member
+ // CHECK-FIXES: {{^}} C::foo(); // 1{{$}}
+ c1->x; // 2
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: static member
+ // CHECK-FIXES: {{^}} C::x; // 2{{$}}
+ c1->nsx; // OK, nsx is a non-static member.
+
+ const C *c2 = new C();
+ c2->foo(); // 2
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: static member
+ // CHECK-FIXES: {{^}} C::foo(); // 2{{$}}
+
+ C::foo(); // OK, foo() is accessed using a qualified-id.
+ C::x; // OK, x is accessed using a qualified-id.
+
+ D d;
+ d.foo();
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: static member
+ // CHECK-FIXES: {{^}} D::foo();{{$}}
+ d.x;
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: static member
+ // CHECK-FIXES: {{^}} D::x;{{$}}
+
+ E e;
+ e.foo();
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: static member
+ // CHECK-FIXES: {{^}} E::foo();{{$}}
+ e.x;
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: static member
+ // CHECK-FIXES: {{^}} E::x;{{$}}
+
+ CC *cc = new CC;
+
+ f(*c1, *c1);
+ f(*cc, *c1);
+
+ // Macros: OK, macros are not checked.
+ FOO((*c1));
+ X((*c1));
+ FOO((*cc));
+ X((*cc));
+
+ // Templates
+ CT<int> ct;
+ ct.foo();
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: static member
+ // CHECK-FIXES: {{^}} CT<int>::foo();{{$}}
+ ct.x;
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: static member
+ // CHECK-FIXES: {{^}} CT<int>::x;{{$}}
+ ct.nsx; // OK, nsx is a non-static member
+
+ CCT<int> cct;
+ cct.foo(); // OK, CCT has no static members.
+ cct.x; // OK, CCT has no static members.
+
+ h<4>();
+}
+
+// Overloaded member access operator
+struct Q {
+ static int K;
+ int y = 0;
+};
+
+int Q::K = 0;
+
+struct Qptr {
+ Q *q;
+
+ explicit Qptr(Q *qq) : q(qq) {}
+
+ Q *operator->() {
+ ++q->y;
+ return q;
+ }
+};
+
+int func(Qptr qp) {
+ qp->y = 10; // OK, the overloaded operator might have side-effects.
+ qp->K = 10; //
+}
Index: clang-tools-extra/trunk/test/clang-tidy/readability-static-accessed-through-instance-nesting-threshold.cpp
===================================================================
--- clang-tools-extra/trunk/test/clang-tidy/readability-static-accessed-through-instance-nesting-threshold.cpp
+++ clang-tools-extra/trunk/test/clang-tidy/readability-static-accessed-through-instance-nesting-threshold.cpp
@@ -0,0 +1,33 @@
+// RUN: %check_clang_tidy %s readability-static-accessed-through-instance %t -- -config="{CheckOptions: [{key: readability-static-accessed-through-instance.NameSpecifierNestingThreshold, value: 4}]}" --
+
+// Nested specifiers
+namespace M {
+namespace N {
+struct V {
+ static int v;
+ struct T {
+ static int t;
+ struct U {
+ static int u;
+ };
+ };
+};
+}
+}
+
+void f(M::N::V::T::U u) {
+ M::N::V v;
+ v.v = 12;
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: static member
+ // CHECK-FIXES: {{^}} M::N::V::v = 12;{{$}}
+
+ M::N::V::T w;
+ w.t = 12;
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: static member
+ // CHECK-FIXES: {{^}} M::N::V::T::t = 12;{{$}}
+
+ // u.u is not changed, because the nesting level is over 4
+ u.u = 12;
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: static member
+ // CHECK-FIXES: {{^}} u.u = 12;{{$}}
+}
Index: clang-tools-extra/trunk/clang-tidy/readability/StaticAccessedThroughInstanceCheck.h
===================================================================
--- clang-tools-extra/trunk/clang-tidy/readability/StaticAccessedThroughInstanceCheck.h
+++ clang-tools-extra/trunk/clang-tidy/readability/StaticAccessedThroughInstanceCheck.h
@@ -0,0 +1,43 @@
+//===--- StaticAccessedThroughInstanceCheck.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_STATIC_ACCESSED_THROUGH_INSTANCE_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_STATIC_ACCESSED_THROUGH_INSTANCE_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace readability {
+
+/// \@brief Checks for member expressions that access static members through
+/// instances and replaces them with uses of the appropriate qualified-id.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/readability-static-accessed-through-instance.html
+class StaticAccessedThroughInstanceCheck : public ClangTidyCheck {
+public:
+ StaticAccessedThroughInstanceCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context),
+ NameSpecifierNestingThreshold(
+ Options.get("NameSpecifierNestingThreshold", 3)) {}
+
+ void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+
+private:
+ const unsigned NameSpecifierNestingThreshold;
+};
+
+} // namespace readability
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_STATIC_ACCESSED_THROUGH_INSTANCE_H
Index: clang-tools-extra/trunk/clang-tidy/readability/StaticAccessedThroughInstanceCheck.cpp
===================================================================
--- clang-tools-extra/trunk/clang-tidy/readability/StaticAccessedThroughInstanceCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/readability/StaticAccessedThroughInstanceCheck.cpp
@@ -0,0 +1,90 @@
+//===--- StaticAccessedThroughInstanceCheck.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 "StaticAccessedThroughInstanceCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace readability {
+
+static unsigned getNameSpecifierNestingLevel(const QualType &QType) {
+ if (const ElaboratedType *ElType = QType->getAs<ElaboratedType>()) {
+ const NestedNameSpecifier *NestedSpecifiers = ElType->getQualifier();
+ unsigned NameSpecifierNestingLevel = 1;
+ do {
+ NameSpecifierNestingLevel++;
+ NestedSpecifiers = NestedSpecifiers->getPrefix();
+ } while (NestedSpecifiers);
+
+ return NameSpecifierNestingLevel;
+ }
+ return 0;
+}
+
+void StaticAccessedThroughInstanceCheck::storeOptions(
+ ClangTidyOptions::OptionMap &Opts) {
+ Options.store(Opts, "NameSpecifierNestingThreshold",
+ NameSpecifierNestingThreshold);
+}
+
+void StaticAccessedThroughInstanceCheck::registerMatchers(MatchFinder *Finder) {
+ Finder->addMatcher(
+ memberExpr(hasDeclaration(anyOf(cxxMethodDecl(isStaticStorageClass()),
+ varDecl(hasStaticStorageDuration()))),
+ unless(isInTemplateInstantiation()))
+ .bind("memberExpression"),
+ this);
+}
+
+void StaticAccessedThroughInstanceCheck::check(
+ const MatchFinder::MatchResult &Result) {
+ const auto *MemberExpression =
+ Result.Nodes.getNodeAs<MemberExpr>("memberExpression");
+
+ if (MemberExpression->getLocStart().isMacroID())
+ return;
+
+ const Expr *BaseExpr = MemberExpression->getBase();
+
+ // Do not warn for overlaoded -> operators.
+ if (isa<CXXOperatorCallExpr>(BaseExpr))
+ return;
+
+ QualType BaseType =
+ BaseExpr->getType()->isPointerType()
+ ? BaseExpr->getType()->getPointeeType().getUnqualifiedType()
+ : BaseExpr->getType().getUnqualifiedType();
+
+ const ASTContext *AstContext = Result.Context;
+ PrintingPolicy PrintingPolicyWithSupressedTag(AstContext->getLangOpts());
+ PrintingPolicyWithSupressedTag.SuppressTagKeyword = true;
+ std::string BaseTypeName =
+ BaseType.getAsString(PrintingPolicyWithSupressedTag);
+
+ SourceLocation MemberExprStartLoc = MemberExpression->getLocStart();
+ auto Diag =
+ diag(MemberExprStartLoc, "static member accessed through instance");
+
+ if (BaseExpr->HasSideEffects(*AstContext) ||
+ getNameSpecifierNestingLevel(BaseType) > NameSpecifierNestingThreshold)
+ return;
+
+ Diag << FixItHint::CreateReplacement(
+ CharSourceRange::getCharRange(MemberExprStartLoc,
+ MemberExpression->getMemberLoc()),
+ BaseTypeName + "::");
+}
+
+} // namespace readability
+} // namespace tidy
+} // namespace clang
Index: clang-tools-extra/trunk/clang-tidy/readability/CMakeLists.txt
===================================================================
--- clang-tools-extra/trunk/clang-tidy/readability/CMakeLists.txt
+++ clang-tools-extra/trunk/clang-tidy/readability/CMakeLists.txt
@@ -25,6 +25,7 @@
RedundantSmartptrGetCheck.cpp
RedundantStringInitCheck.cpp
SimplifyBooleanExprCheck.cpp
+ StaticAccessedThroughInstanceCheck.cpp
StaticDefinitionInAnonymousNamespaceCheck.cpp
UniqueptrDeleteReleaseCheck.cpp
Index: clang-tools-extra/trunk/clang-tidy/readability/ReadabilityTidyModule.cpp
===================================================================
--- clang-tools-extra/trunk/clang-tidy/readability/ReadabilityTidyModule.cpp
+++ clang-tools-extra/trunk/clang-tidy/readability/ReadabilityTidyModule.cpp
@@ -32,6 +32,7 @@
#include "RedundantStringCStrCheck.h"
#include "RedundantStringInitCheck.h"
#include "SimplifyBooleanExprCheck.h"
+#include "StaticAccessedThroughInstanceCheck.h"
#include "StaticDefinitionInAnonymousNamespaceCheck.h"
#include "UniqueptrDeleteReleaseCheck.h"
@@ -70,6 +71,8 @@
"readability-redundant-function-ptr-dereference");
CheckFactories.registerCheck<RedundantMemberInitCheck>(
"readability-redundant-member-init");
+ CheckFactories.registerCheck<StaticAccessedThroughInstanceCheck>(
+ "readability-static-accessed-through-instance");
CheckFactories.registerCheck<StaticDefinitionInAnonymousNamespaceCheck>(
"readability-static-definition-in-anonymous-namespace");
CheckFactories.registerCheck<readability::NamedParameterCheck>(
Index: clang-tools-extra/trunk/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/trunk/docs/ReleaseNotes.rst
+++ clang-tools-extra/trunk/docs/ReleaseNotes.rst
@@ -71,6 +71,12 @@
``AllowConditionalPointerCasts`` -> ``AllowPointerConditions``.
+- New `readability-static-accessed-through-instance
+ <http://clang.llvm.org/extra/clang-tidy/checks/readability-static-accessed-through-instance.html>`_ check
+
+ Finds member expressions that access static members through instances and
+ replaces them with uses of the appropriate qualified-id.
+
Improvements to include-fixer
-----------------------------
Index: clang-tools-extra/trunk/docs/clang-tidy/checks/readability-static-accessed-through-instance.rst
===================================================================
--- clang-tools-extra/trunk/docs/clang-tidy/checks/readability-static-accessed-through-instance.rst
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/readability-static-accessed-through-instance.rst
@@ -0,0 +1,30 @@
+.. title:: clang-tidy - readability-static-accessed-through-instance
+
+readability-static-accessed-through-instance
+============================================
+
+Checks for member expressions that access static members through instances, and
+replaces them with uses of the appropriate qualified-id.
+
+Example:
+
+The following code:
+
+.. code-block:: c++
+
+ struct C {
+ static void foo();
+ static int x;
+ };
+
+ C *c1 = new C();
+ c1->foo();
+ c1->x;
+
+is changed to:
+
+.. code-block:: c++
+
+ C::foo();
+ C::x;
+
Index: clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
===================================================================
--- clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
@@ -178,5 +178,6 @@
readability-redundant-string-cstr
readability-redundant-string-init
readability-simplify-boolean-expr
+ readability-static-accessed-through-instance
readability-static-definition-in-anonymous-namespace
readability-uniqueptr-delete-release
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
