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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to