Revert "Remove false positive in AvoidNonConstGlobalVariables."

[vingeldal]

Summary

There was concernes about a false positive in clang-tidy check
AvoidNonConstGlobalVariables after it was merged and a fix for
assumed false positive was submitted in:
https://reviews.llvm.org/D77461

There was also some discussion regarding if this was actually a
false positive or a true positive. An issue was submitted to the
C++ Core Guidelines asking for a clarification in the matter:
https://github.com/isocpp/CppCoreGuidelines/issues/1599

The answer from the C++ Core Guidelines project seems to go
against the previous assumption that this was a matter of a
false positive, so this patch is suggestion to revert the
previous fix of the assumed false positive.

This reverts commit b2d8c89ea48beb83e0392b1f00c9eafa33c09ca8.

[aaron.ballman]

I only see a comment on the issue, but not that the issue was resolved in favor of that comment (unless I've missed something). If the issue gets resolved in the direction that comment is going, then the revert makes sense (though we should add an explicit test case showing that we want the diagnostic).

[vingeldal]

In D79113#2016294, @aaron.ballman wrote:

I only see a comment on the issue, but not that the issue was resolved in favor of that comment (unless I've missed something). If the issue gets resolved in the direction that comment is going, then the revert makes sense (though we should add an explicit test case showing that we want the diagnostic).

No, your not missing anything. The issue isn't actually closed yet. I'll remember to update tests as appropriate if the issue is resolved in favor of this revert. Until the issue is resolved I'll just wait.

[JonasToth]

The issue is resolved now in the CppCoreGuidelines.

[lebedev.ri]

This review seems to be stuck/dead, consider abandoning if no longer relevant.

clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidNonConstGlobalVariablesCheck.cpp

 #include "clang/ASTMatchers/ASTMatchers.h"
 
 using namespace clang::ast_matchers;
 
 namespace clang {
 namespace tidy {
 namespace cppcoreguidelines {
 
-namespace {
-AST_MATCHER(VarDecl, isLocalVarDecl) { return Node.isLocalVarDecl(); }
-} // namespace
-
 void AvoidNonConstGlobalVariablesCheck::registerMatchers(MatchFinder *Finder) {
   auto GlobalVariable = varDecl(
       hasGlobalStorage(),
       unless(anyOf(
-          isLocalVarDecl(), isConstexpr(), hasType(isConstQualified()),
+          isConstexpr(), hasType(isConstQualified()),
           hasType(referenceType())))); // References can't be changed, only the
                                        // data they reference can be changed.
 
   auto GlobalReferenceToNonConst =
       varDecl(hasGlobalStorage(), hasType(referenceType()),
               unless(hasType(references(qualType(isConstQualified())))));
 
   auto GlobalPointerToNonConst =

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

 
 // CHECKING FOR NON-CONST GLOBAL TEMPLATE VARIABLE ////////////////////////////
 template <class T>
 constexpr T templateVariable = T(0L);
 
 // 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 + staticNonConstLoopVariable;
+    nonConstInt = nonConstLoopVariable + i;
   }
 }