[PATCH] D64564: Loop pragma parsing. NFC.

[Michael Kruse via Phabricator via cfe-commits]

Meinersbur added inline comments.


================
Comment at: clang/lib/Parse/ParsePragma.cpp:1009
 static std::string PragmaLoopHintString(Token PragmaName, Token Option) {
-  std::string PragmaString;
-  if (PragmaName.getIdentifierInfo()->getName() == "loop") {
-    PragmaString = "clang loop ";
-    PragmaString += Option.getIdentifierInfo()->getName();
-  } else if (PragmaName.getIdentifierInfo()->getName() == "unroll_and_jam") {
-    PragmaString = "unroll_and_jam";
-  } else {
-    assert(PragmaName.getIdentifierInfo()->getName() == "unroll" &&
-           "Unexpected pragma name");
-    PragmaString = "unroll";
-  }
-  return PragmaString;
+  std::string Str = PragmaName.getIdentifierInfo()->getName();
+  Str = llvm::StringSwitch<std::string>(Str)
----------------
`getName()` returns `StringRef`. No need to use a `std::string` yet.


================
Comment at: clang/lib/Parse/ParsePragma.cpp:1011-1014
+               .Case("loop", std::string("clang loop ") + Str)
+               .Case("unroll_and_jam", Str)
+               .Case("unroll", Str)
+               .Default("");
----------------
This unconditionally creates (at least) 5 `std::string` objects.


================
Comment at: clang/lib/Parse/ParsePragma.cpp:1040
   // without an argument.
-  bool PragmaUnroll = PragmaNameInfo->getName() == "unroll";
-  bool PragmaNoUnroll = PragmaNameInfo->getName() == "nounroll";
-  bool PragmaUnrollAndJam = PragmaNameInfo->getName() == "unroll_and_jam";
-  bool PragmaNoUnrollAndJam = PragmaNameInfo->getName() == "nounroll_and_jam";
-  if (Toks.empty() && (PragmaUnroll || PragmaNoUnroll || PragmaUnrollAndJam ||
-                       PragmaNoUnrollAndJam)) {
+  const bool LoopHint = llvm::StringSwitch<bool>(PragmaNameInfo->getName())
+                            .Cases("unroll", "nounroll", "unroll_and_jam",
----------------
[style] We don't use `const` for local variables. Could also use `auto` here since the type is already explicit on the RHS.

[suggestion] `IsLoopHint` would indicate the meaning of the boolean variable.


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

https://reviews.llvm.org/D64564