njames93 added inline comments.
================
Comment at:
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:130
m(ObjcIvar) \
+ m(HungarianNotation) \
----------------
Is this line needed?
================
Comment at:
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:241
+ // Options
+ const static llvm::StringMap<std::string> Options = {
+ {"TreatStructAsClass", "false"}};
----------------
As you never use map like access for this, shouldn't it be an array.
The same goes for all the other StringMaps in this function
================
Comment at:
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:366
+ IdentifierNamingCheck::HungarianNotationOption &HNOption) {
+ std::string Section = "HungarianNotation.";
+
----------------
================
Comment at:
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:368
+
+ std::vector<std::string> HNOpts = {"TreatStructAsClass"};
+ for (auto const &Opt : HNOpts) {
----------------
However for this I can see that its mapping the same options as `Options` in
`getHungarianNotationDefaultConfig()`.
Maybe `HNOpts` should be removed from here, `Option` from
`getHungarianNotationDefaultConfig()` taken out of function scope and iterate
over that array below.
A similar approach could be made with HNDerviedTypes
================
Comment at:
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:369-370
+ std::vector<std::string> HNOpts = {"TreatStructAsClass"};
+ for (auto const &Opt : HNOpts) {
+ std::string Key = Section + "General." + Opt;
+ std::string Val = Options.get(Key, "");
----------------
Building these temporary strings is expensive. Better off having a SmallString
contsructed outside the loop and fill the string for each iteration, saved on
allocations.
The same buffer can be reused below for the other loops
================
Comment at:
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:424
+static bool
+isHungarianNotationOptionEnabled(const std::string OptionKey,
+ llvm::StringMap<std::string> StrMap) {
----------------
================
Comment at:
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:718
+
+ const static std::list<std::string> Keywords = {
+ // Constexpr specifiers
----------------
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D86671/new/
https://reviews.llvm.org/D86671
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
