[PATCH] D141838: [clang-tidy] fix a false positive of `cppcoreguidelines-avoid-non-const-global-variables`

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

I'm not convinced this is the correct fix.
The guildline 

 states that tools should flag all variables declared at global or namespace 
scope.
Therefore we shouldn't be flagging any class static members, no matter if they 
are public, private or protected.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141838/new/

https://reviews.llvm.org/D141838

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D141838: [clang-tidy] fix a false positive of `cppcoreguidelines-avoid-non-const-global-variables`

[Vincent Hong via Phabricator via cfe-commits]

v1nh1shungry created this revision.
v1nh1shungry added reviewers: carlosgalvezp, gribozavr2.
Herald added subscribers: shchenz, kbarton, xazax.hun, nemanjai.
Herald added a reviewer: njames93.
Herald added a project: All.
v1nh1shungry requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

Currently this checker will report the non-const static
private/protected member of a class/struct.

Fixes https://github.com/llvm/llvm-project/issues/57919


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D141838

Files:
  
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidNonConstGlobalVariablesCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-non-const-global-variables.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-non-const-global-variables.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-non-const-global-variables.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-non-const-global-variables.cpp
@@ -228,6 +228,20 @@
 template 
 constexpr T templateVariable = T(0L);
 
+// CHECKING FOR NON-CONST STATIC PRIVATE/PROTECTED CLASS MEMBERS //
+class Foo {
+  static int nonConstStaticPrivateMember;
+protected:
+  static int nonConstStaticProtectedMember;
+};
+
+struct Bar {
+private:
+  static int nonConstStaticPrivateMember;
+protected:
+  static int nonConstStaticProtectedMember;
+};
+
 // CHECKING AGAINST FALSE POSITIVES INSIDE FUNCTION SCOPE /
 int main() {
   for (int i = 0; i < 3; ++i) {
Index: 
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidNonConstGlobalVariablesCheck.cpp
===
--- 
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidNonConstGlobalVariablesCheck.cpp
+++ 
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidNonConstGlobalVariablesCheck.cpp
@@ -24,6 +24,7 @@
   hasGlobalStorage(),
   unless(anyOf(
   isLocalVarDecl(), isConstexpr(), hasType(isConstQualified()),
+  isPrivate(), isProtected(),
   hasType(referenceType(); // References can't be changed, only the
// data they reference can be changed.
 


Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-non-const-global-variables.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-non-const-global-variables.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-non-const-global-variables.cpp
@@ -228,6 +228,20 @@
 template 
 constexpr T templateVariable = T(0L);
 
+// CHECKING FOR NON-CONST STATIC PRIVATE/PROTECTED CLASS MEMBERS //
+class Foo {
+  static int nonConstStaticPrivateMember;
+protected:
+  static int nonConstStaticProtectedMember;
+};
+
+struct Bar {
+private:
+  static int nonConstStaticPrivateMember;
+protected:
+  static int nonConstStaticProtectedMember;
+};
+
 // CHECKING AGAINST FALSE POSITIVES INSIDE FUNCTION SCOPE /
 int main() {
   for (int i = 0; i < 3; ++i) {
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidNonConstGlobalVariablesCheck.cpp
===
--- clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidNonConstGlobalVariablesCheck.cpp
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidNonConstGlobalVariablesCheck.cpp
@@ -24,6 +24,7 @@
   hasGlobalStorage(),
   unless(anyOf(
   isLocalVarDecl(), isConstexpr(), hasType(isConstQualified()),
+  isPrivate(), isProtected(),
   hasType(referenceType(); // References can't be changed, only the
// data they reference can be changed.
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits