[PATCH] D123349: [clang-tidy] Deal with keyword tokens in preprocessor conditions

[Nathan James via Phabricator via cfe-commits]

njames93 added inline comments.


================
Comment at: clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp:281
 
+inline StringRef getTokenName(const Token &Tok) {
+  return Tok.is(tok::raw_identifier) ? Tok.getRawIdentifier()
----------------
inline is pretty redundant here. Did you mean to make this static?


================
Comment at: clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp:287
 void MacroToEnumCallbacks::checkName(const Token &MacroNameTok) {
-  std::string Id;
-  if (MacroNameTok.is(tok::raw_identifier))
-    Id = MacroNameTok.getRawIdentifier().str();
-  else if (MacroNameTok.is(tok::identifier))
-    Id = MacroNameTok.getIdentifierInfo()->getName().str();
-  else {
-    assert(false && "Expected either an identifier or raw identifier token");
-    return;
-  }
+  std::string Id = getTokenName(MacroNameTok).str();
 
----------------
Building a std::string here seems unnecessary, Why can't we just keep the StringRef.


================
Comment at: clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp:291
     return llvm::any_of(MacroList, [&Id](const EnumMacro &Macro) {
-      return Macro.Name.getIdentifierInfo()->getName().str() == Id;
+      return getTokenName(Macro.Name).str() == Id;
     });
----------------
Again, building a string is unnecessary, the StringRef equality operators will do the job nicely.


================
Comment at: clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp:432
+              "macro '%0' defines an integral constant; prefer an enum instead")
+      << getTokenName(Macro.Name).str();
 }
----------------
ditto.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123349