Izaron updated this revision to Diff 466660.
Izaron added a comment.
Rebased onto main (after a long long time).
Thanks to @LegalizeAdulthood for instructions!
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D118743/new/
https://reviews.llvm.org/D118743
Files:
clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp
clang-tools-extra/clang-tidy/modernize/UseInlineConstVariablesInHeadersCheck.cpp
clang-tools-extra/clang-tidy/modernize/UseInlineConstVariablesInHeadersCheck.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
clang-tools-extra/docs/clang-tidy/checks/modernize/use-inline-const-variables-in-headers.rst
clang-tools-extra/test/clang-tidy/checkers/modernize/use-inline-const-variables-in-headers.hpp
Index: clang-tools-extra/test/clang-tidy/checkers/modernize/use-inline-const-variables-in-headers.hpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/modernize/use-inline-const-variables-in-headers.hpp
@@ -0,0 +1,43 @@
+// RUN: %check_clang_tidy %s -std=c++17 modernize-use-inline-const-variables-in-headers %t
+
+const int ConstFoo = 1;
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: global constant 'ConstFoo' should be marked as 'inline' [modernize-use-inline-const-variables-in-headers]
+// CHECK-FIXES: {{^}}inline const int ConstFoo = 1;{{$}}
+
+namespace N {
+constexpr int NamespaceFoo = 1;
+// CHECK-MESSAGES: :[[@LINE-1]]:15: warning: global constant 'NamespaceFoo' should be marked as 'inline' [modernize-use-inline-const-variables-in-headers]
+// CHECK-FIXES: {{^}}inline constexpr int NamespaceFoo = 1;{{$}}
+} // namespace N
+
+extern const int ExternFoo;
+// CHECK-MESSAGES: :[[@LINE-1]]:18: warning: global constant 'ExternFoo' should be converted to C++17 'inline variable' [modernize-use-inline-const-variables-in-headers]
+
+struct S {
+ // no warning: the variable is not at file scope
+ static const int StructFoo = 1;
+};
+
+// no warning: non-const global variables have external linkage
+int NonConstFoo = 1;
+
+// no warning: volatile global variables have external linkage
+const volatile int VolatileFoo = 1;
+
+// no warning: templates and their instantiations have external linkage
+template <typename T>
+const auto TemplateFoo = sizeof(T);
+
+// no warning: already fixed
+inline const int InlineFoo = 1;
+
+// no warning: C has no 'inline variables'
+extern "C" {
+const int CFoo0 = 1;
+}
+extern "C" const int CFoo1 = 1;
+
+// no warning: 'inline' is invisible when within an unnamed namespace
+namespace {
+const int AnonNamespaceFoo = 1;
+} // namespace
Index: clang-tools-extra/docs/clang-tidy/checks/modernize/use-inline-const-variables-in-headers.rst
===================================================================
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/modernize/use-inline-const-variables-in-headers.rst
@@ -0,0 +1,69 @@
+.. title:: clang-tidy - modernize-use-inline-const-variables-in-headers
+
+modernize-use-inline-const-variables-in-headers
+===============================================
+
+
+Suggests switching to C++17 ``inline variables`` for non-inline const
+variable definitions and extern const variable declarations in header files.
+Non-inline const variables make a separate instance of the variable for each
+translation unit that includes the header, which can lead to subtle violation
+of the ODR. Extern const variables are a deprecated way to define a constant
+since C++17.
+
+.. code-block:: c++
+
+ // Foo.h
+ const int ConstFoo = 1; // Warning: should be marked as 'inline'
+
+ namespace N {
+ constexpr int NamespaceFoo = 1; // Warning: should be marked as 'inline'
+ }
+
+ extern const int ExternFoo; // Warning: should be converted to C++17 'inline variable'
+
+ struct S {
+ static const int StructFoo = 1; // OK: the variable is not at file scope
+ };
+
+ int NonConstFoo = 1; // No warning: non-const global variables have external linkage
+
+ const volatile int VolatileFoo = 1; // No warning: volatile global variables have external linkage
+
+ template<typename T>
+ const auto TemplateFoo = sizeof(T); // OK: templates and their instantiations have external linkage
+
+ inline const int InlineFoo = 1; // no warning: already fixed
+
+ // No warning: C has no 'inline variables'
+ extern "C" {
+ const int CFoo0 = 1;
+ }
+ extern "C" const int CFoo1 = 1;
+
+ // No warning: 'inline' is invisible when within an unnamed namespace
+ namespace {
+ const int AnonNamespaceFoo = 1;
+ }
+
+Options
+-------
+
+.. option:: HeaderFileExtensions
+
+ A comma-separated list of filename extensions of header files (the filename
+ extensions should not include "." prefix). Default is `h,hh,hpp,hxx`.
+ For header files without an extension, use an empty string (if there are no
+ other desired extensions) or leave an empty element in the list. E.g.,
+ `h,hh,hpp,hxx,` (note the trailing comma).
+
+.. option:: CheckNonInline
+
+ When `true`, the check will find non-inline const variable definitions in
+ header files and suggest fixes for them. Default is `true`.
+
+.. option:: CheckExtern
+
+ When `true`, the check will find extern const variable declarations in
+ header files. But the checker won't fix it as the definition may be located
+ in another translation unit. Default is `true`.
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
@@ -283,6 +283,7 @@
`modernize-use-emplace <modernize/use-emplace.html>`_, "Yes"
`modernize-use-equals-default <modernize/use-equals-default.html>`_, "Yes"
`modernize-use-equals-delete <modernize/use-equals-delete.html>`_, "Yes"
+ `modernize-use-inline-const-variables-in-headers <modernize/use-inline-const-variables-in-headers.html>`_, "Yes"
`modernize-use-nodiscard <modernize/use-nodiscard.html>`_, "Yes"
`modernize-use-noexcept <modernize/use-noexcept.html>`_, "Yes"
`modernize-use-nullptr <modernize/use-nullptr.html>`_, "Yes"
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -115,6 +115,12 @@
Warns when using ``do-while`` loops.
+- New :doc:`modernize-use-inline-const-variables-in-headers
+ <clang-tidy/checks/modernize/use-inline-const-variables-in-headers>` check.
+
+ Suggests switching to C++17 ``inline variables`` for non-inline const
+ variable definitions and extern const variable declarations in header files.
+
New check aliases
^^^^^^^^^^^^^^^^^
Index: clang-tools-extra/clang-tidy/modernize/UseInlineConstVariablesInHeadersCheck.h
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/modernize/UseInlineConstVariablesInHeadersCheck.h
@@ -0,0 +1,55 @@
+//===--- UseInlineConstVariablesInHeadersCheck.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_USEINLINECONSTVARIABLESINHEADERSCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_USEINLINECONSTVARIABLESINHEADERSCHECK_H
+
+#include "../ClangTidyCheck.h"
+#include "../utils/FileExtensionsUtils.h"
+
+namespace clang {
+namespace tidy {
+namespace modernize {
+
+/// Suggests switching to C++17 ``inline variables`` for non-inline const
+/// variable definitions and extern const variable declarations in header files.
+///
+/// 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.
+/// - `CheckNonInline`: Whether to check and fix non-inline const variable
+/// definitions in header files. True by default.
+/// - `CheckExtern`: Whether to check extern const variable
+/// declarations in header files. True by default.
+class UseInlineConstVariablesInHeadersCheck : public ClangTidyCheck {
+public:
+ UseInlineConstVariablesInHeadersCheck(StringRef Name,
+ ClangTidyContext *Context);
+ bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
+ return LangOpts.CPlusPlus17;
+ }
+ void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+
+private:
+ const std::string RawStringHeaderFileExtensions;
+ const bool CheckNonInline;
+ const bool CheckExtern;
+ utils::FileExtensionsSet HeaderFileExtensions;
+};
+
+} // namespace modernize
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_USEINLINECONSTVARIABLESINHEADERSCHECK_H
Index: clang-tools-extra/clang-tidy/modernize/UseInlineConstVariablesInHeadersCheck.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/modernize/UseInlineConstVariablesInHeadersCheck.cpp
@@ -0,0 +1,107 @@
+//===--- UseInlineConstVariablesInHeadersCheck.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 "UseInlineConstVariablesInHeadersCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace modernize {
+
+namespace {
+
+AST_MATCHER(NamedDecl, isInAnonymousNamespace) {
+ return Node.isInAnonymousNamespace();
+}
+
+AST_MATCHER(VarDecl, isTemplateVariable) {
+ return Node.isTemplated() || isa<VarTemplateSpecializationDecl>(Node);
+}
+
+AST_MATCHER(VarDecl, isExternallyVisible) { return Node.isExternallyVisible(); }
+
+} // namespace
+
+UseInlineConstVariablesInHeadersCheck::UseInlineConstVariablesInHeadersCheck(
+ StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context),
+ RawStringHeaderFileExtensions(Options.getLocalOrGlobal(
+ "HeaderFileExtensions", utils::defaultHeaderFileExtensions())),
+ CheckNonInline(Options.getLocalOrGlobal("CheckNonInline", true)),
+ CheckExtern(Options.getLocalOrGlobal("CheckExtern", true)) {
+ if (!utils::parseFileExtensions(RawStringHeaderFileExtensions,
+ HeaderFileExtensions,
+ utils::defaultFileExtensionDelimiters())) {
+ this->configurationDiag("Invalid header file extension: '%0'")
+ << RawStringHeaderFileExtensions;
+ }
+}
+
+void UseInlineConstVariablesInHeadersCheck::storeOptions(
+ ClangTidyOptions::OptionMap &Opts) {
+ Options.store(Opts, "HeaderFileExtensions", RawStringHeaderFileExtensions);
+ Options.store(Opts, "CheckNonInline", CheckNonInline);
+ Options.store(Opts, "CheckExtern", CheckExtern);
+}
+
+void UseInlineConstVariablesInHeadersCheck::registerMatchers(
+ MatchFinder *Finder) {
+ auto NonInlineConstVarDecl =
+ varDecl(hasGlobalStorage(),
+ hasDeclContext(anyOf(translationUnitDecl(),
+ namespaceDecl())), // is at file scope
+ hasType(isConstQualified()), // const-qualified
+ unless(anyOf(
+ isInAnonymousNamespace(), // not within an anonymous namespace
+ isTemplateVariable(), // non-template
+ isInline(), // non-inline
+ hasType(isVolatileQualified()), // non-volatile
+ isExternC() // not "extern C" variable
+ )));
+
+ if (CheckNonInline)
+ Finder->addMatcher(varDecl(NonInlineConstVarDecl, isDefinition(),
+ unless(isExternallyVisible()))
+ .bind("non-inline-var-definition"),
+ this);
+ if (CheckExtern)
+ Finder->addMatcher(varDecl(NonInlineConstVarDecl, isExternallyVisible())
+ .bind("extern-var-declaration"),
+ this);
+}
+
+void UseInlineConstVariablesInHeadersCheck::check(
+ const MatchFinder::MatchResult &Result) {
+ const VarDecl *D = nullptr;
+ StringRef Msg;
+ bool InsertInlineKeyword = false;
+
+ if ((D = Result.Nodes.getNodeAs<VarDecl>("non-inline-var-definition"))) {
+ Msg = "global constant %0 should be marked as 'inline'";
+ InsertInlineKeyword = true;
+ } else {
+ D = Result.Nodes.getNodeAs<VarDecl>("extern-var-declaration");
+ Msg = "global constant %0 should be converted to C++17 'inline variable'";
+ }
+
+ // Ignore if it comes not from a header
+ if (!utils::isSpellingLocInHeaderFile(D->getBeginLoc(), *Result.SourceManager,
+ HeaderFileExtensions))
+ return;
+
+ DiagnosticBuilder Diag = diag(D->getLocation(), Msg) << D;
+ if (InsertInlineKeyword)
+ Diag << FixItHint::CreateInsertion(D->getBeginLoc(), "inline ");
+}
+
+} // namespace modernize
+} // namespace tidy
+} // namespace clang
Index: clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp
===================================================================
--- clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp
+++ clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp
@@ -33,6 +33,7 @@
#include "UseEmplaceCheck.h"
#include "UseEqualsDefaultCheck.h"
#include "UseEqualsDeleteCheck.h"
+#include "UseInlineConstVariablesInHeadersCheck.h"
#include "UseNodiscardCheck.h"
#include "UseNoexceptCheck.h"
#include "UseNullptrCheck.h"
@@ -88,6 +89,8 @@
CheckFactories.registerCheck<UseEqualsDefaultCheck>("modernize-use-equals-default");
CheckFactories.registerCheck<UseEqualsDeleteCheck>(
"modernize-use-equals-delete");
+ CheckFactories.registerCheck<UseInlineConstVariablesInHeadersCheck>(
+ "modernize-use-inline-const-variables-in-headers");
CheckFactories.registerCheck<UseNodiscardCheck>(
"modernize-use-nodiscard");
CheckFactories.registerCheck<UseNoexceptCheck>("modernize-use-noexcept");
Index: clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
===================================================================
--- clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
+++ clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
@@ -32,6 +32,7 @@
UseEmplaceCheck.cpp
UseEqualsDefaultCheck.cpp
UseEqualsDeleteCheck.cpp
+ UseInlineConstVariablesInHeadersCheck.cpp
UseNodiscardCheck.cpp
UseNoexceptCheck.cpp
UseNullptrCheck.cpp
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
