[PATCH] D72213: [clang-tidy] Add `bugprone-reserved-identifier` to flag usages of reserved C++ names

logan-5 added a comment. I'm taking @JonasToth's suggestion and splitting this into two patches, one for the refactor, followed by one for the new check. The refactor patch is here: https://reviews.llvm.org/D72284. Thanks everyone for your feedback so far. CHANGES SINCE LAST ACTION

[PATCH] D72213: [clang-tidy] Add `bugprone-reserved-identifier` to flag usages of reserved C++ names

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] == '_'; +}

[PATCH] D72213: [clang-tidy] Add `bugprone-reserved-identifier` to flag usages of reserved C++ names

logan-5 updated this revision to Diff 236216. logan-5 added a comment. Change double-underscore fix-it finding algorithm to correctly collapse any number of >=2 underscores in a row, not just exactly 2 (or multiples of 2). CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72213/new/

[PATCH] D72213: [clang-tidy] Add `bugprone-reserved-identifier` to flag usages of reserved C++ names

logan-5 marked an inline comment as done. logan-5 added inline comments. Comment at: clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.h:59 + struct FailureInfo { +std::string KindName; // Tag or misc info to be used as derived classes need +std::string Fixup;

[PATCH] D72213: [clang-tidy] Add `bugprone-reserved-identifier` to flag usages of reserved C++ names

logan-5 updated this revision to Diff 236210. logan-5 marked 39 inline comments as done. logan-5 added a comment. Addressed formatting issues, and tweaked handling of the names `__` and `::_`: the check now flags these but doesn't suggest a fix. CHANGES SINCE LAST ACTION

[PATCH] D72213: [clang-tidy] Add `bugprone-reserved-identifier` to flag usages of reserved C++ names

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 +

[PATCH] D72213: [clang-tidy] Add `bugprone-reserved-identifier` to flag usages of reserved C++ names

JonasToth added a comment. i think it would be easier to review if there are two patches, one refactoring and one new check. Comment at: clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.h:59 + struct FailureInfo { +std::string KindName; // Tag or misc info to be

[PATCH] D72213: [clang-tidy] Add `bugprone-reserved-identifier` to flag usages of reserved C++ names

Eugene.Zelenko added a comment. In D72213#1804447 , @Mordante wrote: > @eugene `Please don't use auto when type is spelled in same statement or > iterator.` do you mean `type is not spelled` ? Yes, you are correct. Sorry for mistake. Repository: rG

[PATCH] D72213: [clang-tidy] Add `bugprone-reserved-identifier` to flag usages of reserved C++ names

logan-5 added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/bugprone-reserved-identifier-invert.cpp:21 +// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: declaration uses identifier 'Helper', which is not a reserved identifier [bugprone-reserved-identifier]

[PATCH] D72213: [clang-tidy] Add `bugprone-reserved-identifier` to flag usages of reserved C++ names

logan-5 added a comment. @Eugene.Zelenko +1 to @Mordante 's question. Otherwise, happy to address most of those nits, though I think I will gently push back on a couple of them (in the inline comments). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D72213: [clang-tidy] Add `bugprone-reserved-identifier` to flag usages of reserved C++ names

logan-5 added a comment. In D72213#1804394 , @Mordante wrote: > I wonder what happens if the project already contains a suggested fix, for > example: > > #define __FOO(X) ... > #define _FOO(X) __FOO(X) > #define FOO(X) _FOO(X) > > > will it

[PATCH] D72213: [clang-tidy] Add `bugprone-reserved-identifier` to flag usages of reserved C++ names

Mordante added a subscriber: eugene. Mordante added a comment. @eugene `Please don't use auto when type is spelled in same statement or iterator.` do you mean `type is not spelled` ? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72213/new/

[PATCH] D72213: [clang-tidy] Add `bugprone-reserved-identifier` to flag usages of reserved C++ names

Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp:49 +} +static Optional getDoubleUnderscoreFixup(StringRef Name) { + if (hasDoubleUnderscore(Name)) { Please separate with empty line.

[PATCH] D72213: [clang-tidy] Add `bugprone-reserved-identifier` to flag usages of reserved C++ names

Mordante added a comment. I like this change, but I don't feel qualified to fully review the patch. I wonder what happens if the project already contains a suggested fix, for example: #define __FOO(X) ... #define _FOO(X) __FOO(X) #define FOO(X) _FOO(X) will it suggest: #define FOO(X)