Mordante added inline comments.

================
Comment at: clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp:70
+                                      const bool IsInGlobalNamespace) {
+  return IsInGlobalNamespace && Name.size() > 1 && Name[0] == '_';
+}
----------------
logan-5 wrote:
> Mordante wrote:
> > `Name.size() > 1` doesn't catch `_` in the global namespace. Catching `_` 
> > will probably also cause issues with the automatic renaming.
> > What about renaming `__`?
> Good point. Perhaps the check should catch these but simply not propose a fix.
Agreed.


================
Comment at: 
clang-tools-extra/test/clang-tidy/checkers/bugprone-reserved-identifier.cpp:68
+
+} // namespace __ns
+
----------------
logan-5 wrote:
> Mordante wrote:
> > Should it not suggest to change this to `namespace ns` ?
> That's a fair point. I'll point out (again) that this is an existing case 
> missed by the logic factored out of readability-identifier-naming, so it 
> might be better suited for a separate patch?
Ok. I think a separate patch will be better.


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

https://reviews.llvm.org/D72213



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

Reply via email to