[clang-tidy] Allow thread-local variables in avoid-non-const-global-variables #164442
+29
−1
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-clang-tidy @llvm/pr-subscribers-clang-tools-extra Author: Sirui Mu (Lancern) ChangesThis patch adds an option named Full diff: https://github.com/llvm/llvm-project/pull/164442.diff 5 Files Affected:
diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidNonConstGlobalVariablesCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidNonConstGlobalVariablesCheck.cpp
index f0e66e44690b2..2c0baa5716954 100644
--- a/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidNonConstGlobalVariablesCheck.cpp
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidNonConstGlobalVariablesCheck.cpp
@@ -17,7 +17,8 @@ namespace clang::tidy::cppcoreguidelines {
AvoidNonConstGlobalVariablesCheck::AvoidNonConstGlobalVariablesCheck(
StringRef Name, ClangTidyContext *Context)
: ClangTidyCheck(Name, Context),
- AllowInternalLinkage(Options.get("AllowInternalLinkage", false)) {}
+ AllowInternalLinkage(Options.get("AllowInternalLinkage", false)),
+ AllowThreadLocal(Options.get("AllowThreadLocal", false)) {}
void AvoidNonConstGlobalVariablesCheck::registerMatchers(MatchFinder *Finder) {
auto NamespaceMatcher = AllowInternalLinkage
@@ -31,6 +32,8 @@ void AvoidNonConstGlobalVariablesCheck::registerMatchers(MatchFinder *Finder) {
GlobalContext,
AllowInternalLinkage ? varDecl(unless(isStaticStorageClass()))
: varDecl(),
+ AllowThreadLocal ? varDecl(unless(hasThreadStorageDuration()))
+ : varDecl(),
unless(anyOf(
isConstexpr(), hasType(isConstQualified()),
hasType(referenceType())))); // References can't be changed, only the
diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidNonConstGlobalVariablesCheck.h b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidNonConstGlobalVariablesCheck.h
index 5e7c968b12f97..d8f2a733e3b01 100644
--- a/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidNonConstGlobalVariablesCheck.h
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidNonConstGlobalVariablesCheck.h
@@ -27,6 +27,7 @@ class AvoidNonConstGlobalVariablesCheck : public ClangTidyCheck {
private:
const bool AllowInternalLinkage;
+ const bool AllowThreadLocal;
};
} // namespace clang::tidy::cppcoreguidelines
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 8a0151f567c24..58bcf92a75834 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -316,6 +316,11 @@ Changes in existing checks
an additional matcher that generalizes the copy-and-swap idiom pattern
detection.
+- Improved :doc:`cppcoreguidelines-avoid-non-const-global-variables
+ <clang-tidy/checks/cppcoreguidelines/avoid-non-const-global-variables>` check
+ by adding a new option ``AllowThreadLocal`` that suppresses warnings on
+ non-const global variables with thread-local storage duration.
+
- Improved :doc:`cppcoreguidelines-init-variables
<clang-tidy/checks/cppcoreguidelines/init-variables>` check by fixing the
insertion location for function pointers with multiple parameters.
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 8da284ca13e3d..3d5fef3a07dca 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
@@ -49,3 +49,8 @@ Options
When set to `true`, static non-const variables and variables in anonymous
namespaces will not generate a warning. The default value is `false`.
+
+.. option:: AllowThreadLocal
+
+ When set to `true`, non-const global variables with thread-local storage
+ duration 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 334332def216f..30bdd68a21b84 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,6 +1,8 @@
// 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'}}"
+// RUN: %check_clang_tidy %s -check-suffixes=,THREAD-LOCAL cppcoreguidelines-avoid-non-const-global-variables %t -- \
+// RUN: -config="{CheckOptions: {cppcoreguidelines-avoid-non-const-global-variables.AllowThreadLocal : '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]
@@ -42,14 +44,23 @@ namespace {
int nonConstAnonymousNamespaceInt = 0;
// 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]
+// CHECK-MESSAGES-THREAD-LOCAL: :[[@LINE-3]]: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]
+// CHECK-MESSAGES-THREAD-LOCAL: :[[@LINE-3]]: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;
+thread_local int threadLocalInt = 0;
+// CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:18: warning: variable 'threadLocalInt' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]
+// CHECK-MESSAGES-INTERNAL-LINKAGE: :[[@LINE-2]]:18: warning: variable 'threadLocalInt' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]
+// CHECK-MESSAGES-THREAD-LOCAL-NOT: :[[@LINE-3]]:18: warning: variable 'threadLocalInt' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]
+
+thread_local const int threadLocalConstInt = 0;
+
class DummyClass {
public:
int nonConstPublicMemberVariable = 0;
@@ -137,6 +148,7 @@ DummyEnum nonConstAnonymousNamespaceEnumInstance = DummyEnum::first;
}
// 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]
+// CHECK-MESSAGES-THREAD-LOCAL: :[[@LINE-4]]: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 {
@@ -181,6 +193,7 @@ DummyStruct nonConstAnonymousNamespaceStructInstance;
}
// 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]
+// CHECK-MESSAGES-THREAD-LOCAL: :[[@LINE-4]]:13: warning: variable 'nonConstAnonymousNamespaceStructInstance' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]
// CHECKING FOR NON-CONST GLOBAL UNION ////////////////////////////////////////
union DummyUnion {
@@ -222,6 +235,7 @@ DummyUnion nonConstAnonymousNamespaceUnionInstance = {0x0};
}
// 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]
+// CHECK-MESSAGES-THREAD-LOCAL: :[[@LINE-4]]: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() {
|
…ariables This patch adds an option named `AllowThreadLocal` to the `cppcoreguidelines-avoid-non-const-global-variables` check. When set to true, the option suppresses warnings generated for non-const global variables with thread-local storage duration. By default, the option is set to false.
Lancern
force-pushed
the
clang-tidy/non-const-thread-local
branch
from
0e5e21f to
b3586be
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This patch adds an option named
AllowThreadLocalto thecppcoreguidelines-avoid-non-const-global-variablescheck. When set to true, the option suppresses warnings generated for non-const global variables with thread-local storage duration. By default, the option is set to false.