dougpuob added inline comments.

================
Comment at: 
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:441-456
+static IdentifierNamingCheck::HungarianPrefixOption
+parseHungarianPrefix(std::string OptionVal) {
+  for (auto &C : OptionVal)
+    C = toupper(C);
+
+  if (std::string::npos != OptionVal.find("LOWER_CASE"))
+    return IdentifierNamingCheck::HungarianPrefixOption::HPO_LowerCase;
----------------
njames93 wrote:
> This isn't really needed if you have the mapping defined, `Options.get` works 
> with enums, just look at how CaseType is parsed and stored. If you want to 
> map multiple strings to a single enum constant that can also work by putting 
> both strings in the mapping.
> This method also validates inputs and will print out an error if a user 
> supplies a value that can't be converted.
Good idea. Thank you.


================
Comment at: 
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:460
 getNamingStyles(const ClangTidyCheck::OptionsView &Options) {
+  static IdentifierNamingCheck::HungarianNotationOption HNOption;
+  HNOption.clearAll();
----------------
njames93 wrote:
> This function can be called multiple times per translation unit when looking 
> through header files if `GetConfigPerFile` is enabled. Making this static 
> will mean that each file thats read could potentially alter other files style 
> configuration.
> Maybe a smarter way about this is rather than this function returning a 
> vector of naming styles, it returns a struct which contains the Hungarian 
> options and a vector of the styles. Doing this would probably also mean you 
> don't need to store a reference to this in the `NamingStyle`.
I removed the static variable from the function and a reference in 
`NamingStyle`.


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

Reply via email to