[PATCH] D77461: [clang-tidy] Remove false positive in AvoidNonConstGlobalVariables

[Kim Viggedal via Phabricator via cfe-commits]

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;
   }
 }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D77461.255037.patch
Type: text/x-patch
Size: 737 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20200404/a2d967b6/attachment.bin>