[PATCH] D57680: [llvm-objdump] Implement `-Mreg-names-raw`/`-std` options.

[George Rimar via Phabricator via llvm-commits]

grimar added inline comments.


================
Comment at: lib/MC/MCInstPrinter.cpp:33
 
+bool MCInstPrinter::applyTargetSpecificCLOption(StringRef Opt) { return false; }
+
----------------
Maybe inline it?


================
Comment at: lib/Target/ARM/InstPrinter/ARMInstPrinter.cpp:83
+  } else
+    return false;
+}
----------------
You can avoid using `else` after `return`:

```
if (Opt == "reg-names-std") {
  DefaultAltIdx = ARM::NoRegAltName;
  return true;
} 
if (Opt == "reg-names-raw") {
  DefaultAltIdx = ARM::RegNamesRaw;
  return true;
} 
return false;
```


================
Comment at: tools/llvm-objdump/llvm-objdump.cpp:1474
+      error("Unrecognized disassembler option: " + Opt);
+  }
+
----------------
You do not need curly bracers here:

```
  for (const auto &Opt : DisassemblerOptions)
    if (!IP->applyTargetSpecificCLOption(Opt))
      error("Unrecognized disassembler option: " + Opt);
```

I would also don't use `auto` because the type is not obvious.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D57680