[PATCH] D46819: [llvm-objcopy] Add --keep-symbol (-K) option

Jake Ehrlich via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon May 14 16:15:10 PDT 2018


jakehehrlich accepted this revision.
jakehehrlich added a comment.

LGTM



================
Comment at: test/tools/llvm-objcopy/keep-symbol-remove-section.test:2
+# RUN: yaml2obj %s > %t
+# RUN: llvm-objcopy --remove-section .text -K foo %t %t2
+# RUN: llvm-readobj -symbols %t2 | FileCheck %s
----------------
jhenderson wrote:
> Nit: I'd suggest using only long options in this sort of test, to make it a little clearer.
I'd like to +1. It's good to test that the short options work but I can never remember what the aliases are. I had to go look them up for keep-symbol.test


================
Comment at: test/tools/llvm-objcopy/keep-symbol.test:2
+# RUN: yaml2obj %s > %t
+# RUN: llvm-objcopy -x -K foo --keep-symbol bar %t %t2
+# RUN: llvm-objcopy -K foo -N foo -N bar --keep-symbol bar -N baz %t %t3
----------------
nit: Can you use --discard-all following James recommendation above. Since --discard-all isn't being tested (nor any related option really) here it shouldn't use an alias.


Repository:
  rL LLVM

https://reviews.llvm.org/D46819





More information about the llvm-commits mailing list