[PATCH] D50117: [llvm-objcopy] NFC: Refactor main objcopy method that takes an ELFReader to a generic Reader.

[Alexander Shaposhnikov via Phabricator via llvm-commits]

alexshap added inline comments.


================
Comment at: tools/llvm-objcopy/llvm-objcopy.cpp:291
 
+static Optional<ElfType> GetOutputElfType(const CopyConfig &Config,
+                                          const Reader &Reader) {
----------------
rupprecht wrote:
> alexshap wrote:
> > it looks like now or even in the future None  here is not "recoverable" anyway
> > (down the line we exit with an error),
> > maybe it would be better to return the enum directly instead ?
> > (and if the output format is "binary" it will error("Unsupported output format ...")).
> The unrecoverable error that's thrown when this optional doesn't have a value comes immediately after a check that Config.OutputFormat == "binary". So returning None is valid in that case.
yes, that's exactly what i was saying -  but the point was different - wouldn't returning enum directly make the code a bit simpler ?


Repository:
  rL LLVM

https://reviews.llvm.org/D50117