[PATCH] D66449: [llvm-objcopy] Accept MachO formats in commad-line parsing. NFC.

Tue Aug 27 20:33:04 PDT 2019

seiya marked an inline comment as done.
seiya added a comment.

In D66449#1646539 <https://reviews.llvm.org/D66449#1646539>, @jhenderson wrote:

> Test cases for the new code?

I forgot to add NFC in the title. This patch does not introduce new features thus we don't need new tests for it I think.

================
Comment at: llvm/tools/llvm-objcopy/CopyConfig.cpp:271
+    {"sparcel", {ELF::EM_SPARC, 0, 0, false, true}},
+    {"x86-64",
+     {ELF::EM_X86_64, MachO::CPU_TYPE_X86_64, MachO::CPU_SUBTYPE_X86_64_ALL,
----------------
jhenderson wrote:
> seiya wrote:
> > rupprecht wrote:
> > > It looks like GNU objcopy doesn't even accept `-B x86-64`, I wonder if we should consider dropping it. The arch used for that case is actually `i386:x86-64`.
> > I didn't noticed that. Personally, `x86-64` looks intuitive to me but I think it should be removed for the compatibility with GNU objcopy. I'll write a patch for it when I have time.
> I don't think we should drop it. I agree that `x86-64` is more intuitive, so if people are in a constrained environment with only the LLVM tools available to them, it's more useful. It's good to add stuff beyond what GNU supports for this reason, if there's a use case.
> It's good to add stuff beyond what GNU supports for this reason, if there's a use case.
Makes much sense.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66449