aaron.ballman added inline comments.

================
Comment at: 
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:149-151
+    auto CaseOptionName = (Name + "Case").str();
+    auto CaseValue = Options.get(CaseOptionName, "");
+    auto const caseOptional = fromString(CaseValue);
----------------
Some `auto`s that don't have the type spelled out (also, a top-level `const` 
that can be dropped).


================
Comment at: 
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:154
+      llvm::errs() << "warning: Invalid case style '" << CaseValue
+                   << "' for readability-identifier-naming." << CaseOptionName;
+      constexpr StringRef AcceptibleNames[] = {
----------------
njames93 wrote:
> Unsure about hardcoding the check-name as it could be ran under an alias. 
> However there is no way for a ClangTidyCheck to get the name its ran as. 
> `ClangTidyCheck::CheckName` is `private`, maybe a protected getter would be 
> wise.
I agree that we should not hardcode the name of the check. I actually wonder if 
a better approach here is to call `Context->diag()` to generate an actual 
diagnostic?


================
Comment at: 
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:155
+                   << "' for readability-identifier-naming." << CaseOptionName;
+      constexpr StringRef AcceptibleNames[] = {
+#define CASE(NAME, ENUM) NAME,
----------------
acceptible -> acceptable


================
Comment at: 
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:177
+      if (EditDistance < 3)
+        llvm::errs() << ": did you mean '" << Closest << "'\n";
+      else
----------------
njames93 wrote:
> I feel its safer not assuming the fix and instead letting the user modify 
> their configuration, WDYT?
Agreed -- we don't recover in Clang by assuming the fix to the command line 
option spelling, either.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76606



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to