logan-5 marked an inline comment as done. logan-5 added a comment. @Eugene.Zelenko never mind about pushing back on the nits; I had misread some of them. They all sound good, will address.
================ Comment at: clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp:70 + const bool IsInGlobalNamespace) { + return IsInGlobalNamespace && Name.size() > 1 && Name[0] == '_'; +} ---------------- 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. ================ Comment at: clang-tools-extra/test/clang-tidy/checkers/bugprone-reserved-identifier.cpp:68 + +} // namespace __ns + ---------------- 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? Repository: rG LLVM Github Monorepo 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