[PATCH] D50404: [lld-link] Generalize handling of /debug and /debug:{none, full, fastlink, ghash, symtab}

[Rui Ueyama via Phabricator via llvm-commits]

ruiu added inline comments.


================
Comment at: COFF/Driver.cpp:520
+
+  auto V = StringRef(A->getValue()).lower();
+  DebugKind DK = StringSwitch<DebugKind>(V)
----------------
Please avoid using `auto` unless its actual type is obvious.


================
Comment at: COFF/Driver.cpp:522
+  DebugKind DK = StringSwitch<DebugKind>(V)
+                     .Case("none", DebugKind::None)
+                     .Case("full", DebugKind::Full)
----------------
StringSwitch has `CaseLower`, so you can use that instead of calling `lower` beforehand.


================
Comment at: COFF/Driver.cpp:533
+    warn("/debug:fastlink unsupported; using /debug:full");
+    DK = DebugKind::Full;
+  } else if (DK == DebugKind::Unknown) {
----------------
Just return `DebugKind::Full`.


================
Comment at: COFF/Driver.cpp:536
+    error("/debug: unknown option: " + Twine(A->getValue()));
+    DK = DebugKind::None;
+  }
----------------
Ditto


================
Comment at: COFF/Driver.cpp:547
+
+    for (StringRef Type : Types)
+      DebugTypes |= StringSwitch<unsigned>(Type.lower())
----------------
You probably should warn on unknown string.


================
Comment at: COFF/Driver.cpp:552
+                        .Case("fixup", static_cast<unsigned>(DebugType::Fixup))
+                        .Default(0);
+  } else {
----------------
Return early.


================
Comment at: COFF/Driver.cpp:928
   // Handle /debug
-  if (Args.hasArg(OPT_debug, OPT_debug_dwarf, OPT_debug_ghash)) {
+  DebugKind DK = parseDebugKind(Args);
+
----------------
I'd name DK Debug. We don't usually use acronym for variable names.


Repository:
  rLLD LLVM Linker

https://reviews.llvm.org/D50404