vingeldal created this revision.
Herald added subscribers: cfe-commits, kbarton, xazax.hun, nemanjai.
Herald added a project: clang.
vingeldal added a comment.
Herald added a subscriber: wuzish.

After looking in to it I got less certain of where the error lies and 
eventually I got uncertain if this even is a false negative, so I started out 
with just posting a change where the unit test is updated to detect the issue.
This is just to have a start for a discussion of how this should actually work 
and what is the best way forward.

The purpose of the rule is to avoid code which causes hidden dependencies. The 
given example of a potential false positive is a free function with a static 
variable.
Now, wouldn't a static variable inside a function body make the function 
stateful thereby risking that the function gives different results for the same 
input, in ways that might look arbitrary or random to the caller.
I think that might actually be a good example of what the rule is meant to 
prevent so maybe this isn't a false positive after all?


There was a post merge comment about a possible false negative for this check:
https://reviews.llvm.org/D70265


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D77461

Files:
  
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
@@ -231,7 +231,8 @@
 // CHECKING AGAINST FALSE POSITIVES INSIDE FUNCTION SCOPE /////////////////////
 int main() {
   for (int i = 0; i < 3; ++i) {
+    static int staticNonConstLoopVariable = 42;
     int nonConstLoopVariable = 42;
-    nonConstInt = nonConstLoopVariable + i;
+    nonConstInt = nonConstLoopVariable + i + staticNonConstLoopVariable;
   }
 }


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
@@ -231,7 +231,8 @@
 // CHECKING AGAINST FALSE POSITIVES INSIDE FUNCTION SCOPE /////////////////////
 int main() {
   for (int i = 0; i < 3; ++i) {
+    static int staticNonConstLoopVariable = 42;
     int nonConstLoopVariable = 42;
-    nonConstInt = nonConstLoopVariable + i;
+    nonConstInt = nonConstLoopVariable + i + staticNonConstLoopVariable;
   }
 }
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to