diff --git a/clang-tools-extra/clang-tidy/llvm/CMakeLists.txt b/clang-tools-extra/clang-tidy/llvm/CMakeLists.txt index 79c58a19aedac..3232f6e2cafe5 100644 --- a/clang-tools-extra/clang-tidy/llvm/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/llvm/CMakeLists.txt @@ -9,6 +9,7 @@ add_clang_library(clangTidyLLVMModule STATIC LLVMTidyModule.cpp PreferIsaOrDynCastInConditionalsCheck.cpp PreferRegisterOverUnsignedCheck.cpp + PreferStaticOverAnonymousNamespaceCheck.cpp TwineLocalCheck.cpp LINK_LIBS diff --git a/clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp b/clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp index ceebde1595e7f..075453046f0a1 100644 --- a/clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp @@ -16,6 +16,7 @@ #include "IncludeOrderCheck.h" #include "PreferIsaOrDynCastInConditionalsCheck.h" #include "PreferRegisterOverUnsignedCheck.h" +#include "PreferStaticOverAnonymousNamespaceCheck.h" #include "TwineLocalCheck.h" namespace clang::tidy { @@ -34,6 +35,8 @@ class LLVMModule : public ClangTidyModule { "llvm-prefer-isa-or-dyn-cast-in-conditionals"); CheckFactories.registerCheck( "llvm-prefer-register-over-unsigned"); + CheckFactories.registerCheck( + "llvm-prefer-static-over-anonymous-namespace"); CheckFactories.registerCheck( "llvm-qualified-auto"); CheckFactories.registerCheck("llvm-twine-local"); diff --git a/clang-tools-extra/clang-tidy/llvm/PreferStaticOverAnonymousNamespaceCheck.cpp b/clang-tools-extra/clang-tidy/llvm/PreferStaticOverAnonymousNamespaceCheck.cpp new file mode 100644 index 0000000000000..592f0986292f2 --- /dev/null +++ b/clang-tools-extra/clang-tidy/llvm/PreferStaticOverAnonymousNamespaceCheck.cpp @@ -0,0 +1,100 @@ +//===--- PreferStaticOverAnonymousNamespaceCheck.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 "PreferStaticOverAnonymousNamespaceCheck.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::llvm_check { + +namespace { + +AST_MATCHER(NamedDecl, isInMacro) { + return Node.getBeginLoc().isMacroID() || Node.getEndLoc().isMacroID(); +} + +AST_MATCHER(VarDecl, isLocalVariable) { return Node.isLocalVarDecl(); } + +} // namespace + +PreferStaticOverAnonymousNamespaceCheck:: + PreferStaticOverAnonymousNamespaceCheck(StringRef Name, + ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), + AllowVariableDeclarations(Options.get("AllowVariableDeclarations", true)), + AllowMemberFunctionsInClass( + Options.get("AllowMemberFunctionsInClass", true)) {} + +void PreferStaticOverAnonymousNamespaceCheck::storeOptions( + ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "AllowVariableDeclarations", AllowVariableDeclarations); + Options.store(Opts, "AllowMemberFunctionsInClass", + AllowMemberFunctionsInClass); +} + +void PreferStaticOverAnonymousNamespaceCheck::registerMatchers( + MatchFinder *Finder) { + const auto IsDefinitionInAnonymousNamespace = + allOf(unless(isExpansionInSystemHeader()), isInAnonymousNamespace(), + unless(isInMacro()), isDefinition()); + + if (AllowMemberFunctionsInClass) { + Finder->addMatcher( + functionDecl(IsDefinitionInAnonymousNamespace, + unless(anyOf(hasParent(cxxRecordDecl()), + hasParent(functionTemplateDecl( + hasParent(cxxRecordDecl())))))) + .bind("function"), + this); + } else { + Finder->addMatcher( + functionDecl(IsDefinitionInAnonymousNamespace).bind("function"), this); + } + + if (!AllowVariableDeclarations) + Finder->addMatcher(varDecl(IsDefinitionInAnonymousNamespace, + unless(isLocalVariable()), unless(parmVarDecl())) + .bind("var"), + this); +} + +void PreferStaticOverAnonymousNamespaceCheck::check( + const MatchFinder::MatchResult &Result) { + + if (const auto *Func = Result.Nodes.getNodeAs("function")) { + if (Func->isCXXClassMember()) + diag(Func->getLocation(), + "place definition of method %0 outside of an anonymous namespace") + << Func; + else if (Func->isStatic()) + diag(Func->getLocation(), + "place static function %0 outside of an anonymous namespace") + << Func; + else + diag(Func->getLocation(), + "function %0 is declared in an anonymous namespace; " + "prefer using 'static' for restricting visibility") + << Func; + return; + } + + if (const auto *Var = Result.Nodes.getNodeAs("var")) { + if (Var->getStorageClass() == SC_Static) + diag(Var->getLocation(), + "place static variable %0 outside of an anonymous namespace") + << Var; + else + diag(Var->getLocation(), + "variable %0 is declared in an anonymous namespace; " + "prefer using 'static' for restricting visibility") + << Var; + } +} + +} // namespace clang::tidy::llvm_check diff --git a/clang-tools-extra/clang-tidy/llvm/PreferStaticOverAnonymousNamespaceCheck.h b/clang-tools-extra/clang-tidy/llvm/PreferStaticOverAnonymousNamespaceCheck.h new file mode 100644 index 0000000000000..ca0245e1d3031 --- /dev/null +++ b/clang-tools-extra/clang-tidy/llvm/PreferStaticOverAnonymousNamespaceCheck.h @@ -0,0 +1,42 @@ +//===--- PreferStaticOverAnonymousNamespaceCheck.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_LLVM_PREFERSTATICOVERANONYMOUSNAMESPACECHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LLVM_PREFERSTATICOVERANONYMOUSNAMESPACECHECK_H + +#include "../ClangTidyCheck.h" + +namespace clang::tidy::llvm_check { + +/// Finds function and variable declarations inside anonymous namespace and +/// suggests replacing them with ``static`` declarations. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/llvm/prefer-static-over-anonymous-namespace.html +class PreferStaticOverAnonymousNamespaceCheck : public ClangTidyCheck { +public: + PreferStaticOverAnonymousNamespaceCheck(StringRef Name, + ClangTidyContext *Context); + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + void storeOptions(ClangTidyOptions::OptionMap &Opts) override; + bool isLanguageVersionSupported(const LangOptions &LangOpts) const override { + return LangOpts.CPlusPlus; + } + std::optional getCheckTraversalKind() const override { + return TK_IgnoreUnlessSpelledInSource; + } + +private: + const bool AllowVariableDeclarations; + const bool AllowMemberFunctionsInClass; +}; + +} // namespace clang::tidy::llvm_check + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LLVM_PREFERSTATICOVERANONYMOUSNAMESPACECHECK_H diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index e021d6350694e..599a22ac73c41 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -142,6 +142,12 @@ New checks Finds unscoped (non-class) ``enum`` declarations and suggests using ``enum class`` instead. +- New :doc:`llvm-prefer-static-over-anonymous-namespace + ` check. + + Finds function and variable declarations inside anonymous namespace and + suggests replacing them with ``static`` declarations. + - New :doc:`modernize-use-scoped-lock ` check. diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst index 5098582d0c42b..9b62c87664693 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -250,6 +250,7 @@ Clang-Tidy Checks :doc:`llvm-namespace-comment `, :doc:`llvm-prefer-isa-or-dyn-cast-in-conditionals `, "Yes" :doc:`llvm-prefer-register-over-unsigned `, "Yes" + :doc:`llvm-prefer-static-over-anonymous-namespace `, :doc:`llvm-twine-local `, "Yes" :doc:`llvmlibc-callee-namespace `, :doc:`llvmlibc-implementation-in-namespace `, diff --git a/clang-tools-extra/docs/clang-tidy/checks/llvm/prefer-static-over-anonymous-namespace.rst b/clang-tools-extra/docs/clang-tidy/checks/llvm/prefer-static-over-anonymous-namespace.rst new file mode 100644 index 0000000000000..85579ca676a68 --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/llvm/prefer-static-over-anonymous-namespace.rst @@ -0,0 +1,73 @@ +.. title:: clang-tidy - llvm-prefer-static-over-anonymous-namespace + +llvm-prefer-static-over-anonymous-namespace +=========================================== + +Finds function and variable declarations inside anonymous namespace and +suggests replacing them with ``static`` declarations. + +The `LLVM Coding Standards `_ +recommend keeping anonymous namespaces as small as possible and only use them +for class declarations. For functions and variables the ``static`` specifier +should be preferred for restricting visibility. + +For example non-compliant code: + +.. code-block:: c++ + + namespace { + + class StringSort { + public: + StringSort(...) + bool operator<(const char *RHS) const; + }; + + // warning: place method definition outside of an anonymous namespace + bool StringSort::operator<(const char *RHS) const {} + + // warning: prefer using 'static' for restricting visibility + void runHelper() {} + + // warning: prefer using 'static' for restricting visibility + int myVariable = 42; + + } + +Should become: + +.. code-block:: c++ + + // Small anonymous namespace for class declaration + namespace { + + class StringSort { + public: + StringSort(...) + bool operator<(const char *RHS) const; + }; + + } + + // placed method definition outside of the anonymous namespace + bool StringSort::operator<(const char *RHS) const {} + + // used 'static' instead of an anonymous namespace + static void runHelper() {} + + // used 'static' instead of an anonymous namespace + static int myVariable = 42; + + +Options +------- + +.. option:: AllowVariableDeclarations + + When `true`, allow variable declarations to be in anonymous namespace. + Default value is `true`. + +.. option:: AllowMemberFunctionsInClass + + When `true`, only methods defined in anonymous namespace outside of the + corresponding class will be warned. Default value is `true`. \ No newline at end of file diff --git a/clang-tools-extra/test/clang-tidy/checkers/llvm/prefer-static-over-anonymous-namespace.cpp b/clang-tools-extra/test/clang-tidy/checkers/llvm/prefer-static-over-anonymous-namespace.cpp new file mode 100644 index 0000000000000..f0ffafcf18e67 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/llvm/prefer-static-over-anonymous-namespace.cpp @@ -0,0 +1,201 @@ +// RUN: %check_clang_tidy %s llvm-prefer-static-over-anonymous-namespace %t -- -- -fno-delayed-template-parsing +// RUN: %check_clang_tidy -check-suffixes=,VAR %s llvm-prefer-static-over-anonymous-namespace %t -- \ +// RUN: -config="{CheckOptions: { \ +// RUN: llvm-prefer-static-over-anonymous-namespace.AllowVariableDeclarations: false }, \ +// RUN: }" -- -fno-delayed-template-parsing +// RUN: %check_clang_tidy -check-suffixes=,MEM %s llvm-prefer-static-over-anonymous-namespace %t -- \ +// RUN: -config="{CheckOptions: { \ +// RUN: llvm-prefer-static-over-anonymous-namespace.AllowMemberFunctionsInClass: false }, \ +// RUN: }" -- -fno-delayed-template-parsing + +namespace { + +void regularFunction() { +// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'regularFunction' is declared in an anonymous namespace; prefer using 'static' for restricting visibility [llvm-prefer-static-over-anonymous-namespace] + + int Variable = 42; + auto Lambda = []() { return 42; }; + static int StaticVariable = 42; +} + +void declaredFunction(); + +static void staticFunction() {} +// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: place static function 'staticFunction' outside of an anonymous namespace + +int globalVariable = 42; +// CHECK-MESSAGES-VAR: :[[@LINE-1]]:5: warning: variable 'globalVariable' is declared in an anonymous namespace; + +static int staticVariable = 42; +// CHECK-MESSAGES-VAR: :[[@LINE-1]]:12: warning: place static variable 'staticVariable' outside of an anonymous namespace + +typedef int MyInt; +const MyInt myGlobalVariable = 42; +// CHECK-MESSAGES-VAR: :[[@LINE-1]]:13: warning: variable 'myGlobalVariable' is declared in an anonymous namespace; + +template +constexpr T Pi = T(3.1415926); +// CHECK-MESSAGES-VAR: :[[@LINE-1]]:13: warning: variable 'Pi' is declared in an anonymous namespace; + +void (*funcPtr)() = nullptr; +// CHECK-MESSAGES-VAR: :[[@LINE-1]]:8: warning: variable 'funcPtr' is declared in an anonymous namespace; + +auto lambda = []() { return 42; }; +// CHECK-MESSAGES-VAR: :[[@LINE-1]]:6: warning: variable 'lambda' is declared in an anonymous namespace; + +class InstanceClass { + int member; +}; + +InstanceClass instance; +// CHECK-MESSAGES-VAR: :[[@LINE-1]]:15: warning: variable 'instance' is declared in an anonymous namespace; + +InstanceClass* instancePtr = nullptr; +// CHECK-MESSAGES-VAR: :[[@LINE-1]]:16: warning: variable 'instancePtr' is declared in an anonymous namespace; + +InstanceClass& instanceRef = instance; +// CHECK-MESSAGES-VAR: :[[@LINE-1]]:16: warning: variable 'instanceRef' is declared in an anonymous namespace; + +class MyClass { +public: + MyClass(); + MyClass(const MyClass&) {} + // CHECK-MESSAGES-MEM: :[[@LINE-1]]:3: warning: place definition of method 'MyClass' outside of an anonymous namespace + MyClass(MyClass&&) = default; + // CHECK-MESSAGES-MEM: :[[@LINE-1]]:3: warning: place definition of method 'MyClass' outside of an anonymous namespace + MyClass& operator=(const MyClass&); + MyClass& operator=(MyClass&&); + bool operator<(const MyClass&) const; + void memberFunction(); + static void staticMemberFunction(); + void memberDefinedInClass() {} + // CHECK-MESSAGES-MEM: :[[@LINE-1]]:8: warning: place definition of method 'memberDefinedInClass' outside of an anonymous namespace + static void staticMemberDefinedInClass() {} + // CHECK-MESSAGES-MEM: :[[@LINE-1]]:15: warning: place definition of method 'staticMemberDefinedInClass' outside of an anonymous namespace + template + void templateFunction(); + template + void templateFunctionInClass() {} + // CHECK-MESSAGES-MEM: :[[@LINE-1]]:8: warning: place definition of method 'templateFunctionInClass' outside of an anonymous namespace +}; + +MyClass::MyClass() {} +// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: place definition of method 'MyClass' outside of an anonymous namespace + +MyClass& MyClass::operator=(const MyClass&) { return *this; } +// CHECK-MESSAGES: :[[@LINE-1]]:19: warning: place definition of method 'operator=' outside of an anonymous namespace + +MyClass& MyClass::operator=(MyClass&&) = default; +// CHECK-MESSAGES: :[[@LINE-1]]:19: warning: place definition of method 'operator=' outside of an anonymous namespace + +bool MyClass::operator<(const MyClass&) const { return true; } +// CHECK-MESSAGES: :[[@LINE-1]]:15: warning: place definition of method 'operator<' outside of an anonymous namespace + +void MyClass::memberFunction() {} +// CHECK-MESSAGES: :[[@LINE-1]]:15: warning: place definition of method 'memberFunction' outside of an anonymous namespace + +void MyClass::staticMemberFunction() {} +// CHECK-MESSAGES: :[[@LINE-1]]:15: warning: place definition of method 'staticMemberFunction' outside of an anonymous namespace + +template +void MyClass::templateFunction() {} +// CHECK-MESSAGES: :[[@LINE-1]]:15: warning: place definition of method 'templateFunction' outside of an anonymous namespace + +template +void templateFunction(T Value) {} +// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'templateFunction' is declared in an anonymous namespace; prefer using 'static' for restricting visibility + +template<> +void templateFunction(int Value) {} +// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'templateFunction' is declared in an anonymous namespace; prefer using 'static' for restricting visibility + +template +class TemplateClass { +public: + TemplateClass(); + TemplateClass(const TemplateClass&) {} + // CHECK-MESSAGES-MEM: :[[@LINE-1]]:3: warning: place definition of method 'TemplateClass' outside of an anonymous namespace + TemplateClass(TemplateClass&&) = default; + // CHECK-MESSAGES-MEM: :[[@LINE-1]]:3: warning: place definition of method 'TemplateClass' outside of an anonymous namespace + TemplateClass& operator=(const TemplateClass&); + TemplateClass& operator=(TemplateClass&&); + bool operator<(const TemplateClass&) const; + void memberFunc(); + T getValue() const; + void memberDefinedInClass() {} + // CHECK-MESSAGES-MEM: :[[@LINE-1]]:8: warning: place definition of method 'memberDefinedInClass' outside of an anonymous namespace + static void staticMemberDefinedInClass() {} + // CHECK-MESSAGES-MEM: :[[@LINE-1]]:15: warning: place definition of method 'staticMemberDefinedInClass' outside of an anonymous namespace + template + void templateMethodInTemplateClass() {} + // CHECK-MESSAGES-MEM: :[[@LINE-1]]:8: warning: place definition of method 'templateMethodInTemplateClass' outside of an anonymous namespace +private: + T Value; +}; + +template +TemplateClass::TemplateClass() {} +// CHECK-MESSAGES: :[[@LINE-1]]:19: warning: place definition of method 'TemplateClass' outside of an anonymous namespace + +template +TemplateClass& TemplateClass::operator=(const TemplateClass&) { return *this; } +// CHECK-MESSAGES: :[[@LINE-1]]:37: warning: place definition of method 'operator=' outside of an anonymous namespace + +template +TemplateClass& TemplateClass::operator=(TemplateClass&&) = default; +// CHECK-MESSAGES: :[[@LINE-1]]:37: warning: place definition of method 'operator=' outside of an anonymous namespace + +template +bool TemplateClass::operator<(const TemplateClass&) const { return true; } +// CHECK-MESSAGES: :[[@LINE-1]]:24: warning: place definition of method 'operator<' outside of an anonymous namespace + +template +void TemplateClass::memberFunc() {} +// CHECK-MESSAGES: :[[@LINE-1]]:24: warning: place definition of method 'memberFunc' outside of an anonymous namespace + +template +T TemplateClass::getValue() const { return Value; } +// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: place definition of method 'getValue' outside of an anonymous namespace + +inline void inlineFunction() {} +// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function 'inlineFunction' is declared in an anonymous namespace; prefer using 'static' for restricting visibility + +auto autoReturnFunction() -> int { return 42; } +// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'autoReturnFunction' is declared in an anonymous namespace; prefer using 'static' for restricting visibility + +class OuterClass { +public: + class NestedClass { + public: + void nestedMemberFunc(); + void nestedMemberDefinedInClass() {} + // CHECK-MESSAGES-MEM: :[[@LINE-1]]:10: warning: place definition of method 'nestedMemberDefinedInClass' outside of an anonymous namespace + }; +}; + +void OuterClass::NestedClass::nestedMemberFunc() {} +// CHECK-MESSAGES: :[[@LINE-1]]:31: warning: place definition of method 'nestedMemberFunc' outside of an anonymous namespace + +} // namespace + +#define DEFINE_FUNCTION(name) \ + namespace { \ + void name() {} \ + } + +DEFINE_FUNCTION(macroDefinedFunction) + +#define DECLARE_VAR(type, name, value) \ + namespace { \ + type name = value; \ + } + +DECLARE_VAR(int, macroVariable, 42) + +namespace { + +#define INTERNAL_FUNC void internalMacroFunc() {} + +INTERNAL_FUNC + +} // namespace