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 @@ -41,6 +41,7 @@ NonPrivateMemberVariablesInClassesCheck.cpp RedundantExpressionCheck.cpp StaticAssertCheck.cpp + StaticDeclarationInHeaderCheck.cpp ThrowByValueCatchByReferenceCheck.cpp UnconventionalAssignOperatorCheck.cpp UniqueptrResetReleaseCheck.cpp 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 @@ -21,6 +21,7 @@ #include "NonPrivateMemberVariablesInClassesCheck.h" #include "RedundantExpressionCheck.h" #include "StaticAssertCheck.h" +#include "StaticDeclarationInHeaderCheck.h" #include "ThrowByValueCatchByReferenceCheck.h" #include "UnconventionalAssignOperatorCheck.h" #include "UniqueptrResetReleaseCheck.h" @@ -57,6 +58,8 @@ CheckFactories.registerCheck( "misc-redundant-expression"); CheckFactories.registerCheck("misc-static-assert"); + CheckFactories.registerCheck( + "misc-static-declaration-in-header"); CheckFactories.registerCheck( "misc-throw-by-value-catch-by-reference"); CheckFactories.registerCheck( diff --git a/clang-tools-extra/clang-tidy/misc/StaticDeclarationInHeaderCheck.h b/clang-tools-extra/clang-tidy/misc/StaticDeclarationInHeaderCheck.h new file mode 100644 --- /dev/null +++ b/clang-tools-extra/clang-tidy/misc/StaticDeclarationInHeaderCheck.h @@ -0,0 +1,45 @@ +//===--- StaticDeclarationInHeaderCheck.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_STATICDECLARATIONINHEADERCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_STATICDECLARATIONINHEADERCHECK_H + +#include "../ClangTidyCheck.h" +#include "../utils/FileExtensionsUtils.h" + +namespace clang { +namespace tidy { +namespace misc { + +/// Warns about ``static`` variable and function declarations at file or +/// namespace scope in header files. Provides fix-it hint to remove ``static``. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/misc/static-declaration-in-header.html +class StaticDeclarationInHeaderCheck : public ClangTidyCheck { +public: + StaticDeclarationInHeaderCheck(StringRef Name, ClangTidyContext *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; + void storeOptions(ClangTidyOptions::OptionMap &Opts) override; + +private: + const StringRef RawStringHeaderFileExtensions; + utils::FileExtensionsSet HeaderFileExtensions; + SourceRange getStaticSourceRange(NamedDecl const *MatchedDecl, + const SourceManager &SM); +}; + +} // namespace misc +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_STATICDECLARATIONINHEADERCHECK_H diff --git a/clang-tools-extra/clang-tidy/misc/StaticDeclarationInHeaderCheck.cpp b/clang-tools-extra/clang-tidy/misc/StaticDeclarationInHeaderCheck.cpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/clang-tidy/misc/StaticDeclarationInHeaderCheck.cpp @@ -0,0 +1,103 @@ +//===--- StaticDeclarationInHeaderCheck.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 "StaticDeclarationInHeaderCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Lex/Lexer.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(); } + +AST_POLYMORPHIC_MATCHER_P(isInHeaderFile, + AST_POLYMORPHIC_SUPPORTED_TYPES(FunctionDecl, + VarDecl), + utils::FileExtensionsSet, HeaderFileExtensions) { + return utils::isExpansionLocInHeaderFile( + Node.getBeginLoc(), Finder->getASTContext().getSourceManager(), + HeaderFileExtensions); +} + +} // namespace + +StaticDeclarationInHeaderCheck::StaticDeclarationInHeaderCheck( + 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; + } +} + +SourceRange StaticDeclarationInHeaderCheck::getStaticSourceRange( + NamedDecl const *MatchedDecl, const SourceManager &SM) { + Token Tok; + SourceLocation Loc = MatchedDecl->getSourceRange().getBegin(); + while (Loc < MatchedDecl->getSourceRange().getEnd() && + !Lexer::getRawToken(Loc, Tok, SM, getLangOpts(), true)) { + SourceRange TokenRange(Tok.getLocation(), Tok.getEndLoc()); + StringRef SourceText = Lexer::getSourceText( + CharSourceRange::getTokenRange(TokenRange), SM, getLangOpts()); + if (SourceText == "static") { + return TokenRange; + } + Loc = Tok.getEndLoc(); + } + llvm_unreachable("Could not find 'static' token"); +} + +void StaticDeclarationInHeaderCheck::storeOptions( + ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "HeaderFileExtensions", RawStringHeaderFileExtensions); +} + +void StaticDeclarationInHeaderCheck::registerMatchers(MatchFinder *Finder) { + Finder->addMatcher(functionDecl(isStatic(), + isInHeaderFile(HeaderFileExtensions), + unless(isMemberFunction())) + .bind("x"), + this); + Finder->addMatcher( + varDecl(isStatic(), isInHeaderFile(HeaderFileExtensions), + unless(anyOf(isStaticLocal(), isStaticDataMember()))) + .bind("x"), + this); +} + +void StaticDeclarationInHeaderCheck::check( + const MatchFinder::MatchResult &Result) { + if (const auto *MatchedDecl = Result.Nodes.getNodeAs("x")) { + diag(MatchedDecl->getLocation(), + "static declaration in header; remove 'static'") + << FixItHint::CreateRemoval( + getStaticSourceRange(MatchedDecl, *Result.SourceManager)); + } +} + +} // 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 @@ -118,6 +118,12 @@ Warns when using ``do-while`` loops. +- New :doc:`misc-static-declaration-in-header + ` check. + + Warns about ``static`` variable and function declarations at file or + namespace scope in header files. + - New :doc:`misc-use-anonymous-namespace ` check. 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 @@ -254,6 +254,7 @@ `misc-non-private-member-variables-in-classes `_, `misc-redundant-expression `_, "Yes" `misc-static-assert `_, "Yes" + `misc-static-declaration-in-header `_, "Yes" `misc-throw-by-value-catch-by-reference `_, `misc-unconventional-assign-operator `_, `misc-uniqueptr-reset-release `_, "Yes" diff --git a/clang-tools-extra/docs/clang-tidy/checks/misc/static-declaration-in-header.rst b/clang-tools-extra/docs/clang-tidy/checks/misc/static-declaration-in-header.rst new file mode 100644 --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/misc/static-declaration-in-header.rst @@ -0,0 +1,41 @@ +.. title:: clang-tidy - misc-static-declaration-in-header + +misc-static-declaration-in-header +================================= + +Warns about ``static`` variable and function declarations at file or namespace +scope in header files. Provides fix-it hint to remove ``static``. + +Header files are used to share code among different translation units. ``static``, +on the other hand, can be used to express that a given declaration has internal +linkage. Using ``static`` in header files leads to each translation unit creating +their own internal copy of the function or variable at hand, which is problematic. +This can cause unexpected results, bloat the resulting executable or even trigger +one-definition-rule (ODR) violations. + +This problem is similar to having anonymous namespaces in header files, already +covered by :doc:`google-build-namespaces`. + +Example of problematic code: + +.. code-block:: c++ + + // foo.h + + // Bad + static int x = 123; + static void f() {} + + // Good + int x = 123; + void f() {} + +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. diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc/static-declaration-in-header.hpp b/clang-tools-extra/test/clang-tidy/checkers/misc/static-declaration-in-header.hpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/misc/static-declaration-in-header.hpp @@ -0,0 +1,48 @@ +// RUN: %check_clang_tidy %s misc-static-declaration-in-header %t + +static int v1 = 123; +// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: static declaration in header; remove 'static' [misc-static-declaration-in-header] +// CHECK-FIXES: {{^}}int v1 = 123; + +static const int c1 = 123; +// CHECK-MESSAGES: :[[@LINE-1]]:18: warning: static declaration in header; remove 'static' +// CHECK-FIXES: {{^}}const int c1 = 123; + +static constexpr int c2 = 123; +// CHECK-MESSAGES: :[[@LINE-1]]:22: warning: static declaration in header; remove 'static' +// CHECK-FIXES: {{^}}constexpr int c2 = 123; + +static void f1(){} +// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: static declaration in header; remove 'static' +// CHECK-FIXES: {{^}}void f1(){} + +namespace foo { + static int v2 = 123; + // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: static declaration in header; remove 'static' + // CHECK-FIXES: {{^}} int v2 = 123; + + static int f2(){} + // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: static declaration in header; remove 'static' + // CHECK-FIXES: {{^}} int f2(){} +} + +namespace { + static int v3 = 123; + // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: static declaration in header; remove 'static' + // CHECK-FIXES: {{^}} int v3 = 123; + + static int f3(){} + // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: static declaration in header; remove 'static' + // CHECK-FIXES: {{^}} int f3(){} +} + +// OK +struct Foo { + static const int v4 = 123; + static void f4(){} +}; + +// OK +void f5() { + static int v5; +}