[PATCH] D86671: [clang-tidy] Add new case type to check variables with Hungarian notation

Nathan James via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Dec 15 13:49:59 PST 2020


njames93 added a comment.

One last point, is there a way to validate that these options are only set where it makes sense. If someone tries to use say `MacroDefinitionHungarianPrefix` That could be warning worthy?



================
Comment at: clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:435-462
+    std::string HPrefixKey = (StyleString + "HungarianPrefix").str();
+    using HungarianPrefixType = IdentifierNamingCheck::HungarianPrefixType;
+    auto HPTVal = HungarianPrefixType::HPT_Off;
+    std::string HPrefixVal = Options.get(HPrefixKey, "");
+    if (!HPrefixVal.empty()) {
+      if (auto HPTypeVal = Options.get<HungarianPrefixType>(HPrefixKey))
+        HPTVal = HPTypeVal.get();
----------------



================
Comment at: clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:398-404
+  std::string OptionVal = StrMap.lookup(OptionKey);
+  llvm::transform(OptionVal, OptionVal.begin(), toupper);
+
+  if (OptionVal == "1" || OptionVal == "TRUE" || OptionVal == "ON")
+    return true;
+
+  return false;
----------------
njames93 wrote:
> There's no need to build a string and transform it here.
Scratch this, in D92756 support was added for all boolean options in clang-tidy to use the full YAML boolean spec, I'd advise calling the same function here to keep everything consistent.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D86671/new/

https://reviews.llvm.org/D86671



More information about the cfe-commits mailing list