Author: Carlos Galvez Date: 2022-12-12T15:26:14Z New Revision: 35d9f873e3f21846de1b8f07271feedbbe8518ed
URL: https://github.com/llvm/llvm-project/commit/35d9f873e3f21846de1b8f07271feedbbe8518ed DIFF: https://github.com/llvm/llvm-project/commit/35d9f873e3f21846de1b8f07271feedbbe8518ed.diff LOG: [clang-tidy] Fix a couple additional cases in misc-use-anonymous-namespace only - Do not analyze header files, since we don't want to promote using anonymous namespaces there. - Do not warn about const/constexpr variables, those are implicitly static in C++ and they don't need to be moved to an anonymous namespace. Warning about redundant static in general could be implemented as a standalone check, moving away some of the functionality from this check. This check has been introduced in the current release, thus no mention of this change is needed in the Release Notes. Differential Revision: https://reviews.llvm.org/D139113 Added: clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/use-anonymous-namespace.h Modified: clang-tools-extra/clang-tidy/misc/UseAnonymousNamespaceCheck.cpp clang-tools-extra/clang-tidy/misc/UseAnonymousNamespaceCheck.h clang-tools-extra/docs/clang-tidy/checks/misc/use-anonymous-namespace.rst clang-tools-extra/test/clang-tidy/checkers/misc/use-anonymous-namespace.cpp Removed: ################################################################################ diff --git a/clang-tools-extra/clang-tidy/misc/UseAnonymousNamespaceCheck.cpp b/clang-tools-extra/clang-tidy/misc/UseAnonymousNamespaceCheck.cpp index fdf7828c8c0fd..b26632d107796 100644 --- a/clang-tools-extra/clang-tidy/misc/UseAnonymousNamespaceCheck.cpp +++ b/clang-tools-extra/clang-tidy/misc/UseAnonymousNamespaceCheck.cpp @@ -21,6 +21,15 @@ AST_POLYMORPHIC_MATCHER(isStatic, AST_POLYMORPHIC_SUPPORTED_TYPES(FunctionDecl, return Node.getStorageClass() == SC_Static; } +AST_POLYMORPHIC_MATCHER_P(isInHeaderFile, + AST_POLYMORPHIC_SUPPORTED_TYPES(FunctionDecl, + VarDecl), + utils::FileExtensionsSet, HeaderFileExtensions) { + return utils::isExpansionLocInHeaderFile( + Node.getBeginLoc(), Finder->getASTContext().getSourceManager(), + HeaderFileExtensions); +} + AST_MATCHER(FunctionDecl, isMemberFunction) { return llvm::isa<CXXMethodDecl>(&Node); } @@ -31,15 +40,36 @@ AST_MATCHER(Decl, isInAnonymousNamespace) { } } // namespace +UseAnonymousNamespaceCheck::UseAnonymousNamespaceCheck( + StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), + RawStringHeaderFileExtensions(Options.getLocalOrGlobal( + "HeaderFileExtensions", utils::defaultHeaderFileExtensions())) { + if (!utils::parseFileExtensions(RawStringHeaderFileExtensions, + HeaderFileExtensions, + utils::defaultFileExtensionDelimiters())) { + this->configurationDiag("Invalid header file extension: '%0'") + << RawStringHeaderFileExtensions; + } +} + +void UseAnonymousNamespaceCheck::storeOptions( + ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "HeaderFileExtensions", RawStringHeaderFileExtensions); +} + void UseAnonymousNamespaceCheck::registerMatchers(MatchFinder *Finder) { Finder->addMatcher( functionDecl(isStatic(), - unless(anyOf(isInAnonymousNamespace(), isMemberFunction()))) + unless(anyOf(isInHeaderFile(HeaderFileExtensions), + isInAnonymousNamespace(), isMemberFunction()))) .bind("x"), this); Finder->addMatcher( - varDecl(isStatic(), unless(anyOf(isInAnonymousNamespace(), - isStaticLocal(), isStaticDataMember()))) + varDecl(isStatic(), + unless(anyOf(isInHeaderFile(HeaderFileExtensions), + isInAnonymousNamespace(), isStaticLocal(), + isStaticDataMember(), hasType(isConstQualified())))) .bind("x"), this); } diff --git a/clang-tools-extra/clang-tidy/misc/UseAnonymousNamespaceCheck.h b/clang-tools-extra/clang-tidy/misc/UseAnonymousNamespaceCheck.h index 19d76bfc19f63..af4ac60cf9b25 100644 --- a/clang-tools-extra/clang-tidy/misc/UseAnonymousNamespaceCheck.h +++ b/clang-tools-extra/clang-tidy/misc/UseAnonymousNamespaceCheck.h @@ -10,6 +10,7 @@ #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_USEANONYMOUSNAMESPACECHECK_H #include "../ClangTidyCheck.h" +#include "../utils/FileExtensionsUtils.h" namespace clang { namespace tidy { @@ -18,17 +19,29 @@ namespace misc { /// Warns when using 'static' functions or variables at global scope, and /// suggests moving them to an anonymous namespace. /// +/// The check supports these options: +/// - `HeaderFileExtensions`: a semicolon-separated list of filename +/// extensions of header files (The filename extension should not contain +/// "." prefix). ";h;hh;hpp;hxx" by default. +/// +/// For extension-less header files, using an empty string or leaving an +/// empty string between ";" if there are other filename extensions. +/// /// 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) {} + UseAnonymousNamespaceCheck(StringRef Name, ClangTidyContext *Context); bool isLanguageVersionSupported(const LangOptions &LangOpts) const override { return LangOpts.CPlusPlus; } + void storeOptions(ClangTidyOptions::OptionMap &Opts) override; void registerMatchers(ast_matchers::MatchFinder *Finder) override; void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + +private: + const StringRef RawStringHeaderFileExtensions; + utils::FileExtensionsSet HeaderFileExtensions; }; } // namespace misc 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 index c116bfbdd3325..6b256707366cb 100644 --- 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 @@ -11,6 +11,14 @@ Standard. ``static`` was proposed for deprecation, but later un-deprecated to keep C compatibility [1]. ``static`` is an overloaded term with diff erent meanings in diff erent contexts, so it can create confusion. +The following uses of ``static`` will *not* be diagnosed: + +* Functions or variables in header files, since anonymous namespaces in headers + is considered an antipattern. Allowed header file extensions can be configured + via the `HeaderFileExtensions` option (see below). +* ``const`` or ``constexpr`` variables, since they already have implicit internal + linkage in C++. + Examples: .. code-block:: c++ @@ -25,4 +33,14 @@ Examples: int x; } // namespace +Options +------- + +.. option:: HeaderFileExtensions + + A semicolon-separated list of filename extensions of header files (the filename + extensions should not include "." prefix). Default is ";h;hh;hpp;hxx". + For extension-less header files, using an empty string or leaving an + empty string between ";" if there are other filename extensions. + [1] `Undeprecating static <https://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#1012>`_ diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/use-anonymous-namespace.h b/clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/use-anonymous-namespace.h new file mode 100644 index 0000000000000..59fdef193112b --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/use-anonymous-namespace.h @@ -0,0 +1,3 @@ +// Should not warn here, do not require anonymous namespaces in headers +static int gv{123}; +static void gf(){} 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 index 966310eef7f7c..14f84366e5341 100644 --- 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 @@ -1,4 +1,5 @@ -// RUN: %check_clang_tidy %s misc-use-anonymous-namespace %t +// RUN: %check_clang_tidy %s misc-use-anonymous-namespace %t -- -header-filter=.* -- -I%S/Inputs +#include "use-anonymous-namespace.h" static void f1(); // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function 'f1' declared 'static', move to anonymous namespace instead [misc-use-anonymous-namespace] @@ -41,3 +42,7 @@ void foo() { static int x; } + +// OK +static const int v8{123}; +static constexpr int v9{123}; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
