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