diff --git a/clang-tools-extra/clang-tidy/misc/CMakeLists.txt b/clang-tools-extra/clang-tidy/misc/CMakeLists.txt --- a/clang-tools-extra/clang-tidy/misc/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/misc/CMakeLists.txt @@ -47,6 +47,7 @@ UnusedAliasDeclsCheck.cpp UnusedParametersCheck.cpp UnusedUsingDeclsCheck.cpp + UseAnonymousNamespaceCheck.cpp LINK_LIBS clangAnalysis diff --git a/clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp b/clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp --- a/clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp @@ -27,6 +27,7 @@ #include "UnusedAliasDeclsCheck.h" #include "UnusedParametersCheck.h" #include "UnusedUsingDeclsCheck.h" +#include "UseAnonymousNamespaceCheck.h" namespace clang { namespace tidy { @@ -68,6 +69,8 @@ "misc-unused-parameters"); CheckFactories.registerCheck( "misc-unused-using-decls"); + CheckFactories.registerCheck( + "misc-use-anonymous-namespace"); } }; diff --git a/clang-tools-extra/clang-tidy/misc/UseAnonymousNamespaceCheck.h b/clang-tools-extra/clang-tidy/misc/UseAnonymousNamespaceCheck.h new file mode 100644 --- /dev/null +++ b/clang-tools-extra/clang-tidy/misc/UseAnonymousNamespaceCheck.h @@ -0,0 +1,42 @@ +//===--- UseAnonymousNamespaceCheck.h - clang-tidy --------------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_USEANONYMOUSNAMESPACECHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_USEANONYMOUSNAMESPACECHECK_H + +#include "../ClangTidyCheck.h" + +namespace clang { +namespace tidy { +namespace misc { + +/// Warns when using 'static' functions or variables at global scope, and +/// suggests moving them to an anonymous namespace. It also suggests removing +/// 'static' if they are already inside an anonymous namespace. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/modernize/use-anonymous-namespace.html +class UseAnonymousNamespaceCheck : public ClangTidyCheck { +public: + UseAnonymousNamespaceCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + bool isLanguageVersionSupported(const LangOptions &LangOpts) const override { + return LangOpts.CPlusPlus; + } + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + +private: + template void processMatch(const T *MatchedDecl); +}; + +} // namespace misc +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_USEANONYMOUSNAMESPACECHECK_H diff --git a/clang-tools-extra/clang-tidy/misc/UseAnonymousNamespaceCheck.cpp b/clang-tools-extra/clang-tidy/misc/UseAnonymousNamespaceCheck.cpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/clang-tidy/misc/UseAnonymousNamespaceCheck.cpp @@ -0,0 +1,72 @@ +//===--- UseAnonymousNamespaceCheck.cpp - clang-tidy ----------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "UseAnonymousNamespaceCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" + +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { +namespace misc { +namespace { +AST_POLYMORPHIC_MATCHER(isStatic, AST_POLYMORPHIC_SUPPORTED_TYPES(FunctionDecl, + VarDecl)) { + return Node.getStorageClass() == SC_Static; +} + +AST_MATCHER(FunctionDecl, isMemberFunction) { + return llvm::isa(&Node); +} +AST_MATCHER(VarDecl, isStaticDataMember) { return Node.isStaticDataMember(); } +} // namespace + +static bool isInAnonymousNamespace(const Decl *Decl) { + const DeclContext *DC = Decl->getDeclContext(); + if (DC && DC->isNamespace()) { + const auto *ND = llvm::cast(DC); + if (ND && ND->isAnonymousNamespace()) + return true; + } + return false; +} + +template +void UseAnonymousNamespaceCheck::processMatch(const T *MatchedDecl) { + StringRef Type = llvm::isa(MatchedDecl) ? "variable" : "function"; + if (isInAnonymousNamespace(MatchedDecl)) + diag(MatchedDecl->getLocation(), "%0 %1 declared 'static' in " + "anonymous namespace, remove 'static'") + << Type << MatchedDecl; + else + diag(MatchedDecl->getLocation(), + "%0 %1 declared 'static', move to anonymous namespace instead") + << Type << MatchedDecl; +} + +void UseAnonymousNamespaceCheck::registerMatchers(MatchFinder *Finder) { + Finder->addMatcher( + functionDecl(isStatic(), unless(isMemberFunction())).bind("func"), this); + Finder->addMatcher( + varDecl(isStatic(), unless(anyOf(isStaticLocal(), isStaticDataMember()))) + .bind("var"), + this); +} + +void UseAnonymousNamespaceCheck::check(const MatchFinder::MatchResult &Result) { + if (const auto *MatchedDecl = Result.Nodes.getNodeAs("func")) + processMatch(MatchedDecl); + + if (const auto *MatchedDecl = Result.Nodes.getNodeAs("var")) + processMatch(MatchedDecl); +} + +} // namespace misc +} // 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 @@ -115,6 +115,12 @@ Warns when using ``do-while`` loops. +- New :doc:`misc-use-anonymous-namespace + ` check. + + Warns when using ``static`` function or variables at global scope, and suggests + moving them into an anonymous namespace. + New check aliases ^^^^^^^^^^^^^^^^^ diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -259,6 +259,7 @@ `misc-unused-alias-decls `_, "Yes" `misc-unused-parameters `_, "Yes" `misc-unused-using-decls `_, "Yes" + `misc-use-anonymous-namespace `_, `modernize-avoid-bind `_, "Yes" `modernize-avoid-c-arrays `_, `modernize-concat-nested-namespaces `_, "Yes" diff --git a/clang-tools-extra/docs/clang-tidy/checks/misc/use-anonymous-namespace.rst b/clang-tools-extra/docs/clang-tidy/checks/misc/use-anonymous-namespace.rst new file mode 100644 --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/misc/use-anonymous-namespace.rst @@ -0,0 +1,44 @@ +.. title:: clang-tidy - misc-use-anonymous-namespace + +misc-use-anonymous-namespace +============================ + +Finds instances of ``static`` functions or variables declared at global scope +that could instead be moved into an anonymous namespace. It also detects +instances moved to an anonymous namespace that still keep the redundant +``static``. + +Anonymous namespaces are the "superior alternative" according to the C++ +Standard. ``static`` was proposed for deprecation, but later un-deprecated to +keep C compatibility [1]. ``static`` is an overloaded term with different meanings in +different contexts, so it can create confusion. + +Examples: + +.. code-block:: c++ + + // Bad + static void foo(); + static int x; + + // Good + namespace { + void foo(); + int x; + } // namespace + +.. code-block:: c++ + + // Bad + namespace { + static void foo(); + static int x; + } + + // Good + namespace { + void foo(); + int x; + } // namespace + +[1] `Undeprecating static `_ diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc/use-anonymous-namespace.cpp b/clang-tools-extra/test/clang-tidy/checkers/misc/use-anonymous-namespace.cpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/misc/use-anonymous-namespace.cpp @@ -0,0 +1,59 @@ +// RUN: %check_clang_tidy %s misc-use-anonymous-namespace %t + +static void f1(); +// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function 'f1' declared 'static', move to anonymous namespace instead [misc-use-anonymous-namespace] +static int v1; +// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: variable 'v1' declared 'static', move to anonymous namespace instead + +namespace { + static void f2(); + // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: function 'f2' declared 'static' in anonymous namespace, remove 'static' + static int v2; + // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: variable 'v2' declared 'static' in anonymous namespace, remove 'static' +} + +namespace a { + static void f3(); + // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: function 'f3' declared 'static', move to anonymous namespace instead + static int v3; + // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: variable 'v3' declared 'static', move to anonymous namespace instead +} + +namespace a { +namespace { + static void f4(); + // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: function 'f4' declared 'static' in anonymous namespace, remove 'static' + static int v4; + // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: variable 'v4' declared 'static' in anonymous namespace, remove 'static' +} +} + +// OK +void f5(); +int v5; + +// OK +namespace { + void f6(); + int v6; +} + +// OK +namespace a { +namespace { + void f7(); + int v7; +} +} + +// OK +struct Foo { + static void f(); + static int x; +}; + +// OK +void foo() +{ + static int x; +}