diff --git a/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt b/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt --- a/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt @@ -26,6 +26,7 @@ ReturnBracedInitListCheck.cpp ShrinkToFitCheck.cpp UnaryStaticAssertCheck.cpp + UseAnonymousNamespaceCheck.cpp UseAutoCheck.cpp UseBoolLiteralsCheck.cpp UseDefaultMemberInitCheck.cpp diff --git a/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp b/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp --- a/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp @@ -27,6 +27,7 @@ #include "ReturnBracedInitListCheck.h" #include "ShrinkToFitCheck.h" #include "UnaryStaticAssertCheck.h" +#include "UseAnonymousNamespaceCheck.h" #include "UseAutoCheck.h" #include "UseBoolLiteralsCheck.h" #include "UseDefaultMemberInitCheck.h" @@ -79,6 +80,8 @@ CheckFactories.registerCheck("modernize-shrink-to-fit"); CheckFactories.registerCheck( "modernize-unary-static-assert"); + CheckFactories.registerCheck( + "modernize-use-anonymous-namespace"); CheckFactories.registerCheck("modernize-use-auto"); CheckFactories.registerCheck( "modernize-use-bool-literals"); diff --git a/clang-tools-extra/clang-tidy/modernize/UseAnonymousNamespaceCheck.h b/clang-tools-extra/clang-tidy/modernize/UseAnonymousNamespaceCheck.h new file mode 100644 --- /dev/null +++ b/clang-tools-extra/clang-tidy/modernize/UseAnonymousNamespaceCheck.h @@ -0,0 +1,39 @@ +//===--- 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_MODERNIZE_USEANONYMOUSNAMESPACECHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_USEANONYMOUSNAMESPACECHECK_H + +#include "../ClangTidyCheck.h" + +namespace clang { +namespace tidy { +namespace modernize { + +/// Warns when using 'static' functions or variables at global scope, and +/// suggests moving them to the anonymous namespace. It also suggests removing +/// 'static' if they are already inside the 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; +}; + +} // namespace modernize +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_USEANONYMOUSNAMESPACECHECK_H diff --git a/clang-tools-extra/clang-tidy/modernize/UseAnonymousNamespaceCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseAnonymousNamespaceCheck.cpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/clang-tidy/modernize/UseAnonymousNamespaceCheck.cpp @@ -0,0 +1,75 @@ +//===--- 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 modernize { +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 auto *DC = Decl->getDeclContext(); + if (DC && DC->isNamespace()) { + const auto *ND = llvm::cast(DC); + if (ND && ND->isAnonymousNamespace()) + return true; + } + return false; +} + +void UseAnonymousNamespaceCheck::registerMatchers(MatchFinder *Finder) { + Finder->addMatcher( + functionDecl(isStatic(), unless(isMemberFunction())).bind("x"), this); + Finder->addMatcher( + varDecl(isStatic(), unless(anyOf(isStaticLocal(), isStaticDataMember()))) + .bind("x"), + this); +} + +void UseAnonymousNamespaceCheck::check(const MatchFinder::MatchResult &Result) { + if (const auto *MatchedDecl = Result.Nodes.getNodeAs("x")) { + if (isInAnonymousNamespace(MatchedDecl)) + diag(MatchedDecl->getLocation(), "function %0 declared 'static' in " + "anonymous namespace, remove 'static'") + << MatchedDecl; + else + diag(MatchedDecl->getLocation(), + "function %0 declared 'static', move to anonymous namespace instead") + << MatchedDecl; + } + + if (const auto *MatchedDecl = Result.Nodes.getNodeAs("x")) { + if (isInAnonymousNamespace(MatchedDecl)) + diag(MatchedDecl->getLocation(), "variable %0 declared 'static' in " + "anonymous namespace, remove 'static'") + << MatchedDecl; + else + diag(MatchedDecl->getLocation(), + "variable %0 declared 'static', move to anonymous namespace instead") + << MatchedDecl; + } +} + +} // namespace modernize +} // 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:`modernize-use-anonymous-namespace + ` check. + + Warns when using ``static`` function or variables at global scope, and suggests + moving them to the 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 @@ -277,6 +277,7 @@ `modernize-return-braced-init-list `_, "Yes" `modernize-shrink-to-fit `_, "Yes" `modernize-unary-static-assert `_, "Yes" + `modernize-use-anonymous-namespace `_, `modernize-use-auto `_, "Yes" `modernize-use-bool-literals `_, "Yes" `modernize-use-default-member-init `_, "Yes" diff --git a/clang-tools-extra/docs/clang-tidy/checks/modernize/use-anonymous-namespace.rst b/clang-tools-extra/docs/clang-tidy/checks/modernize/use-anonymous-namespace.rst new file mode 100644 --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/modernize/use-anonymous-namespace.rst @@ -0,0 +1,44 @@ +.. title:: clang-tidy - modernize-use-anonymous-namespace + +modernize-use-anonymous-namespace +================================= + +Finds instances of ``static`` functions or variables declared at global scope +that could instead be moved into the anonymous namespace. It also detects +instances moved to the 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/modernize/use-anonymous-namespace.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-anonymous-namespace.cpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-anonymous-namespace.cpp @@ -0,0 +1,59 @@ +// RUN: %check_clang_tidy %s modernize-use-anonymous-namespace %t + +static void f1(); +// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function 'f1' declared 'static', move to anonymous namespace instead [modernize-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 [modernize-use-anonymous-namespace] + static int v3; + // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: variable 'v3' declared 'static', move to anonymous namespace instead [modernize-use-anonymous-namespace] +} + +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; +}