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

Mon Apr 13 05:20:45 PDT 2020

aaron.ballman added a comment.

In D77461#1977503 <https://reviews.llvm.org/D77461#1977503>, @vingeldal wrote:

> In D77461#1963166 <https://reviews.llvm.org/D77461#1963166>, @lebedev.ri wrote:
>
> > https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#Ri-global is in "Interfaces" section, it only covers inter-procedural stuff.
> >  Diagnosing function-local static non-const variable is a plain false-positive diagnostic.
>
>
> I don't follow your train of thought. Could you please elaborate on why you think this must be a false positive?

I think it must be a false positive because the rule is about global variables where "global" refers to the scope of the variable. This is a locally scoped variable. Also, the rule's enforcement section is pretty clear that this does not apply to local statics.

> My reason for hesitating to  call this a false positive is that this pattern does cause a hidden dependency between users of the function, hence it clearly goes against the short and simple rationale given for this rule:
>  "Non-const global variables hide dependencies and make the dependencies subject to unpredictable changes."

I don't see it hiding a dependency between users of the function. The local static could be swapped out for something else (an actual global, linker magic, etc) and the caller would be unaware. The same is not true for a globally scoped object where the identifier is exposed because someone else could be trying to link to that symbol (and they would break if the symbol changed).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77461