carlosgalvezp updated this revision to Diff 483981.
carlosgalvezp marked an inline comment as done.
carlosgalvezp added a comment.
Add description in release notes.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D140290/new/
https://reviews.llvm.org/D140290
Files:
clang-tools-extra/clang-tidy/misc/CMakeLists.txt
clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
clang-tools-extra/clang-tidy/misc/StaticDeclarationInHeaderCheck.cpp
clang-tools-extra/clang-tidy/misc/StaticDeclarationInHeaderCheck.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
clang-tools-extra/docs/clang-tidy/checks/misc/static-declaration-in-header.rst
clang-tools-extra/test/clang-tidy/checkers/misc/static-declaration-in-header.hpp
Index: clang-tools-extra/test/clang-tidy/checkers/misc/static-declaration-in-header.hpp
===================================================================
--- /dev/null
+++ 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;
+}
Index: clang-tools-extra/docs/clang-tidy/checks/misc/static-declaration-in-header.rst
===================================================================
--- /dev/null
+++ 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<clang-tidy/checks/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.
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===================================================================
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -254,6 +254,7 @@
`misc-non-private-member-variables-in-classes <misc/non-private-member-variables-in-classes.html>`_,
`misc-redundant-expression <misc/redundant-expression.html>`_, "Yes"
`misc-static-assert <misc/static-assert.html>`_, "Yes"
+ `misc-static-declaration-in-header <misc/static-declaration-in-header.html>`_, "Yes"
`misc-throw-by-value-catch-by-reference <misc/throw-by-value-catch-by-reference.html>`_,
`misc-unconventional-assign-operator <misc/unconventional-assign-operator.html>`_,
`misc-uniqueptr-reset-release <misc/uniqueptr-reset-release.html>`_, "Yes"
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -118,6 +118,12 @@
Warns when using ``do-while`` loops.
+- New :doc:`misc-static-declaration-in-header
+ <clang-tidy/checks/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
<clang-tidy/checks/misc/use-anonymous-namespace>` check.
Index: clang-tools-extra/clang-tidy/misc/StaticDeclarationInHeaderCheck.h
===================================================================
--- /dev/null
+++ 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
Index: clang-tools-extra/clang-tidy/misc/StaticDeclarationInHeaderCheck.cpp
===================================================================
--- /dev/null
+++ 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<CXXMethodDecl>(&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<NamedDecl>("x")) {
+ diag(MatchedDecl->getLocation(),
+ "static declaration in header; remove 'static'")
+ << FixItHint::CreateRemoval(
+ getStaticSourceRange(MatchedDecl, *Result.SourceManager));
+ }
+}
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang
Index: clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
===================================================================
--- clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
+++ 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<RedundantExpressionCheck>(
"misc-redundant-expression");
CheckFactories.registerCheck<StaticAssertCheck>("misc-static-assert");
+ CheckFactories.registerCheck<StaticDeclarationInHeaderCheck>(
+ "misc-static-declaration-in-header");
CheckFactories.registerCheck<ThrowByValueCatchByReferenceCheck>(
"misc-throw-by-value-catch-by-reference");
CheckFactories.registerCheck<UnconventionalAssignOperatorCheck>(
Index: clang-tools-extra/clang-tidy/misc/CMakeLists.txt
===================================================================
--- clang-tools-extra/clang-tidy/misc/CMakeLists.txt
+++ 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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
