[PATCH] D63239: [llvm-objcopy] Refactor output target parsing

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

Could you explain a bit more the motivation behind this change, please? If I'm not mistaken, it's to allow using input and output format with non-ELF (e.g. COFF and Mach-O) targets?



================
Comment at: llvm/tools/llvm-objcopy/CopyConfig.cpp:279
 // FIXME: consolidate with the bfd parsing used by lld.
-static const StringMap<MachineInfo> OutputFormatMap{
-    // Name, {EMachine, 64bit, LittleEndian}
+static const StringMap<std::pair<FileFormat, MachineInfo>> FileFormatMap{
+    // Name, {FileFormat, {EMachine, 64bit, LittleEndian}}
----------------
seiya wrote:
> compnerd wrote:
> > I think it would be nicer to use a tuple with an enumeration to give the values names rather than "first" and "second"
> What is an elegant way to get an element from tuple using enum? We need to cast the enum values so it should be `std::get<static_cast<std::size_t>(Foo::Bar)>(Tuple)`. It looks verbose a bit to me.
> I think it would be nicer to use a tuple with an enumeration to give the values names rather than "first" and "second"
Shouldn't it should just be a `struct` then?

```
struct {
  FileFormat Format;
  MachineInfo Machine;
};


================
Comment at: llvm/tools/llvm-objcopy/llvm-objcopy.cpp:32
 #include "llvm/Support/Casting.h"
+#include "llvm/Support/Errc.h"
 #include "llvm/Support/Error.h"
----------------
Why has this been added?


================
Comment at: llvm/tools/llvm-objcopy/llvm-objcopy.cpp:135
+  // output format is binary/ihex or it's not given. This behavior differs from
+  // GNU objcopy. Refer https://bugs.llvm.org/show_bug.cgi?id=42171
+  case FileFormat::Binary:
----------------
Refer -> See


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

https://reviews.llvm.org/D63239