dougpuob added inline comments.
================
Comment at:
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:130
m(ObjcIvar) \
+ m(HungarianNotation) \
----------------
njames93 wrote:
> Is this line needed?
I will remove it. Thank you.
================
Comment at:
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:241
+ // Options
+ const static llvm::StringMap<std::string> Options = {
+ {"TreatStructAsClass", "false"}};
----------------
njames93 wrote:
> 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
Thank you. I will change it.
================
Comment at:
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:368
+
+ std::vector<std::string> HNOpts = {"TreatStructAsClass"};
+ for (auto const &Opt : HNOpts) {
----------------
njames93 wrote:
> 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
I will move `HNOpts` and `HNDerviedTypes` outward to the top of the
`getHungarianNotationDefaultConfig()` function. If the arrays are defined as
static, is there any difference btw inside or outside of function, or did I
misunderstand your meaning?
================
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, "");
----------------
njames93 wrote:
> 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
Good idea, thank you.
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
