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( "readability-redundant-member-init"); + CheckFactories.registerCheck( + "readability-static-accessed-through-instance"); CheckFactories.registerCheck( "readability-static-definition-in-anonymous-namespace"); CheckFactories.registerCheck( 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()) { + 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("memberExpression"); + + if (MemberExpression->getLocStart().isMacroID()) + return; + + const Expr *BaseExpr = MemberExpression->getBase(); + + // Do not warn for overlaoded -> operators. + if (isa(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/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 + `_ 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/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 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/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/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 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 T CT::x; + +template struct CCT { + T foo(); + T x; +}; + +typedef C D; + +using E = D; + +#define FOO(c) c.foo() +#define X(c) c.x + +template 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 struct S { static int x; }; + +template <> struct S<0> { int x; }; + +template void h() { + S 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 ct; + ct.foo(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: static member + // CHECK-FIXES: {{^}} CT::foo();{{$}} + ct.x; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: static member + // CHECK-FIXES: {{^}} CT::x;{{$}} + ct.nsx; // OK, nsx is a non-static member + + CCT 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; // +}