[PATCH] D46029: [llvm-objcopy] Implement --redefine-sym option

[Jake Ehrlich via Phabricator via llvm-commits]

jakehehrlich added inline comments.


================
Comment at: tools/llvm-objcopy/Opts.td:30
                       HelpText<"Remove <section>">;
+def redefine_symbol : Separate<["--"], "redefine-sym">,
+                       MetaVarName<"old=new">,
----------------
alexshap wrote:
> jakehehrlich wrote:
> > Can you use Eq here instead of Separate?
> if i use Eq it will allow smb to type --redefine-sym=a=b which looks weird and doesn't match binutils objcopy, am i missing smth ?
I don't think it looks weird TBH but that's an opinion. Lots of options in llvm support flag formats not supported by GNU versions. LLD does the same thing (I stole it from them): https://github.com/llvm-mirror/lld/blob/master/ELF/Options.td. The point is to be a bit more accepting of various command line options. Using Eq is more consistent. This said LLD is the only tool I know of that a) uses TableGen for arguments and b) allows this. I copied LLD hence all the other ones being Eq. Additionally all tools not using TableGen and instead using the command line library support this. Most tools in the llvm repo don't use TableGen so this makes llvm-objcopy consistent with them as well. I believe llvm-ar is the only llvm copy of a GNU tool that doesn't support "=" in the llvm repo. That said Clang is a major project that doesn't support "=" everywhere as far as I am aware. So we can a) break with llvm and LLD but if we do that we should also go back and change all the existing Eq to be consistent with GNU objcopy. I personally support just using Eq.


Repository:
  rL LLVM

https://reviews.llvm.org/D46029