[PATCH] D70265: [clang-tidy] Add CppCoreGuidelines I.2 "Avoid non-const global variables" check

[Kim Viggedal via Phabricator via cfe-commits]

vingeldal marked an inline comment as done.
vingeldal added inline comments.


================
Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidNonConstGlobalVariablesCheck.cpp:55-71
+  if (const auto *Reference =
+          Result.Nodes.getNodeAs<VarDecl>("reference_to_non-const")) {
+    diag(Reference->getLocation(),
+         "variable %0 provides global access to non-const type, consider "
+         "making the referenced data const")
+        << Reference; // FIXME: Add fix-it hint to Reference
+    return;
----------------
aaron.ballman wrote:
> I think these cases should be combined to avoid duplication.
> ```
> if (const auto *VD = Result.Nodes.getNodeAs<VarDecl>("whatever")) {
>   diag(VD->getLocation(), "variable %0 provides global access to a non-const object; considering making the %select{referenced|pointed-to}1 data 'const'") << VD << VD->getType()->isReferenceType();
>   return;
> }
> ```
> the matcher needs to be changed to bind to the same id for both cases.
> 
> Note, this rewords the diagnostic slightly to avoid type/object confusion (the variable provides access to an object of a non-const type, not to the type itself).
You mean just the pointer and reference cases right? That matcher seems to get much more complicated, I'm having some trouble accomplishing that. Are you sure that's necessary? What would the cases of duplication be?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70265