Author: Pascal Jungblut Date: 2024-07-01T21:12:42+02:00 New Revision: 7c50187b7d7a977144372ceff306d21d71e22e26
URL: https://github.com/llvm/llvm-project/commit/7c50187b7d7a977144372ceff306d21d71e22e26 DIFF: https://github.com/llvm/llvm-project/commit/7c50187b7d7a977144372ceff306d21d71e22e26.diff LOG: [clang-tidy] Option to ignore anonymous namespaces in avoid-non-const-global-variables (#93827) Add an option to ignore warnings for cppcoreguidelines avoid-non-const-global-variables. Understandably, the core guidelines discourage non const global variables, even at the TU level (see https://github.com/isocpp/CppCoreGuidelines/issues/2195). However, having a small TU with an interface that uses a non const variable from an anonymous namespace can be a valid choice. This adds an option that disables the warning just for anonymous namespaces, i.e. at the file level. The default is still to show a warning, just as before. Added: Modified: clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidNonConstGlobalVariablesCheck.cpp clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidNonConstGlobalVariablesCheck.h clang-tools-extra/docs/ReleaseNotes.rst clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-non-const-global-variables.rst clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-non-const-global-variables.cpp Removed: ################################################################################ diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidNonConstGlobalVariablesCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidNonConstGlobalVariablesCheck.cpp index ee17b0e014288..a97ec9fe3fe3d 100644 --- a/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidNonConstGlobalVariablesCheck.cpp +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidNonConstGlobalVariablesCheck.cpp @@ -7,7 +7,6 @@ //===----------------------------------------------------------------------===// #include "AvoidNonConstGlobalVariablesCheck.h" -#include "clang/AST/ASTContext.h" #include "clang/ASTMatchers/ASTMatchFinder.h" #include "clang/ASTMatchers/ASTMatchers.h" @@ -15,13 +14,23 @@ using namespace clang::ast_matchers; namespace clang::tidy::cppcoreguidelines { +AvoidNonConstGlobalVariablesCheck::AvoidNonConstGlobalVariablesCheck( + StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), + AllowInternalLinkage(Options.get("AllowInternalLinkage", false)) {} + void AvoidNonConstGlobalVariablesCheck::registerMatchers(MatchFinder *Finder) { + auto NamespaceMatcher = AllowInternalLinkage + ? namespaceDecl(unless(isAnonymous())) + : namespaceDecl(); auto GlobalContext = varDecl(hasGlobalStorage(), - hasDeclContext(anyOf(namespaceDecl(), translationUnitDecl()))); + hasDeclContext(anyOf(NamespaceMatcher, translationUnitDecl()))); auto GlobalVariable = varDecl( GlobalContext, + AllowInternalLinkage ? varDecl(unless(isStaticStorageClass())) + : varDecl(), unless(anyOf( isConstexpr(), hasType(isConstQualified()), hasType(referenceType())))); // References can't be changed, only the @@ -43,7 +52,6 @@ void AvoidNonConstGlobalVariablesCheck::registerMatchers(MatchFinder *Finder) { void AvoidNonConstGlobalVariablesCheck::check( const MatchFinder::MatchResult &Result) { - if (const auto *Variable = Result.Nodes.getNodeAs<VarDecl>("non-const_variable")) { diag(Variable->getLocation(), "variable %0 is non-const and globally " @@ -63,4 +71,9 @@ void AvoidNonConstGlobalVariablesCheck::check( } } +void AvoidNonConstGlobalVariablesCheck::storeOptions( + ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "AllowInternalLinkage", AllowInternalLinkage); +} + } // namespace clang::tidy::cppcoreguidelines diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidNonConstGlobalVariablesCheck.h b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidNonConstGlobalVariablesCheck.h index e816ca9b47d41..a912763489db9 100644 --- a/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidNonConstGlobalVariablesCheck.h +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidNonConstGlobalVariablesCheck.h @@ -20,10 +20,13 @@ namespace clang::tidy::cppcoreguidelines { /// http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines/avoid-non-const-global-variables.html class AvoidNonConstGlobalVariablesCheck : public ClangTidyCheck { public: - AvoidNonConstGlobalVariablesCheck(StringRef Name, ClangTidyContext *Context) - : ClangTidyCheck(Name, Context) {} + AvoidNonConstGlobalVariablesCheck(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; + +private: + const bool AllowInternalLinkage; }; } // namespace clang::tidy::cppcoreguidelines diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index d968100cd57a7..4d2cd0a9621ba 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -269,6 +269,11 @@ Changes in existing checks <clang-tidy/checks/bugprone/use-after-move>` check to also handle calls to ``std::forward``. +- Improved :doc:`cppcoreguidelines-avoid-non-const-global-variables + <clang-tidy/checks/cppcoreguidelines/avoid-non-const-global-variables>` check + with a new option `AllowInternalLinkage` to disable the warning for variables + with internal linkage. + - Improved :doc:`cppcoreguidelines-macro-usage <clang-tidy/checks/cppcoreguidelines/macro-usage>` check by ignoring macro with hash preprocessing token. diff --git a/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-non-const-global-variables.rst b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-non-const-global-variables.rst index 21c20af6e8107..8da284ca13e3d 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-non-const-global-variables.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-non-const-global-variables.rst @@ -41,3 +41,11 @@ The variables ``a``, ``c``, ``c_ptr1``, ``c_const_ptr`` and ``c_reference`` will all generate warnings since they are either a non-const globally accessible variable, a pointer or a reference providing global access to non-const data or both. + +Options +------- + +.. option:: AllowInternalLinkage + + When set to `true`, static non-const variables and variables in anonymous + namespaces will not generate a warning. The default value is `false`. diff --git a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-non-const-global-variables.cpp b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-non-const-global-variables.cpp index 3ca1029433d22..334332def216f 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-non-const-global-variables.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-non-const-global-variables.cpp @@ -1,4 +1,6 @@ -// RUN: %check_clang_tidy %s cppcoreguidelines-avoid-non-const-global-variables %t +// RUN: %check_clang_tidy %s -check-suffixes=,DEFAULT cppcoreguidelines-avoid-non-const-global-variables %t +// RUN: %check_clang_tidy %s -check-suffixes=,INTERNAL-LINKAGE cppcoreguidelines-avoid-non-const-global-variables %t -- \ +// RUN: -config="{CheckOptions: {cppcoreguidelines-avoid-non-const-global-variables.AllowInternalLinkage : 'true'}}" int nonConstInt = 0; // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: variable 'nonConstInt' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables] @@ -38,9 +40,16 @@ int function() { namespace { int nonConstAnonymousNamespaceInt = 0; -// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: variable 'nonConstAnonymousNamespaceInt' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables] +// CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:5: warning: variable 'nonConstAnonymousNamespaceInt' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables] +// CHECK-MESSAGES-INTERNAL-LINKAGE-NOT: :[[@LINE-2]]:5: warning: variable 'nonConstAnonymousNamespaceInt' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables] } // namespace +static int nonConstStaticInt = 0; +// CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:12: warning: variable 'nonConstStaticInt' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables] +// CHECK-MESSAGES-INTERNAL-LINKAGE-NOT: :[[@LINE-2]]:12: warning: variable 'nonConstStaticInt' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables] + +static const int constStaticInt = 0; + class DummyClass { public: int nonConstPublicMemberVariable = 0; @@ -126,7 +135,8 @@ const DummyEnum constNamespaceEnumInstance = DummyEnum::first; namespace { DummyEnum nonConstAnonymousNamespaceEnumInstance = DummyEnum::first; } -// CHECK-MESSAGES: :[[@LINE-2]]:11: warning: variable 'nonConstAnonymousNamespaceEnumInstance' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables] +// CHECK-MESSAGES-DEFAULT: :[[@LINE-2]]:11: warning: variable 'nonConstAnonymousNamespaceEnumInstance' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables] +// CHECK-MESSAGES-INTERNAL-LINKAGE-NOT: :[[@LINE-2]]:11: warning: variable 'nonConstAnonymousNamespaceEnumInstance' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables] // CHECKING FOR NON-CONST GLOBAL STRUCT /////////////////////////////////////// struct DummyStruct { @@ -169,7 +179,8 @@ const DummyStruct constNamespaceDummyStructInstance; namespace { DummyStruct nonConstAnonymousNamespaceStructInstance; } -// CHECK-MESSAGES: :[[@LINE-2]]:13: warning: variable 'nonConstAnonymousNamespaceStructInstance' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables] +// CHECK-MESSAGES-DEFAULT: :[[@LINE-2]]:13: warning: variable 'nonConstAnonymousNamespaceStructInstance' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables] +// CHECK-MESSAGES-INTERNAL-LINKAGE-NOT: :[[@LINE-2]]:11: warning: variable 'nonConstAnonymousNamespaceEnumInstance' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables] // CHECKING FOR NON-CONST GLOBAL UNION //////////////////////////////////////// union DummyUnion { @@ -209,7 +220,8 @@ const DummyUnion constNamespaceDummyUnionInstance = {0x0}; namespace { DummyUnion nonConstAnonymousNamespaceUnionInstance = {0x0}; } -// CHECK-MESSAGES: :[[@LINE-2]]:12: warning: variable 'nonConstAnonymousNamespaceUnionInstance' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables] +// CHECK-MESSAGES-DEFAULT: :[[@LINE-2]]:12: warning: variable 'nonConstAnonymousNamespaceUnionInstance' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables] +// CHECK-MESSAGES-INTERNAL-LINKAGE-NOT: :[[@LINE-3]]:12: warning: variable 'nonConstAnonymousNamespaceUnionInstance' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables] // CHECKING FOR NON-CONST GLOBAL FUNCTION POINTER ///////////////////////////// int dummyFunction() { _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
