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