[PATCH] D46064: [llvm-objcopy] Add --localize-symbol option

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: test/tools/llvm-objcopy/localize.test:1
+# RUN: yaml2obj %s > %t
+# RUN: llvm-objcopy --localize-symbol defaultGlobal  %t %t2
----------------
Why do we need all the extra symbols in this test?

Could we also get tests for localizing a symbol that is already a local, and also localizing a weak symbol, please, and for localizing multiple symbols at the same time (maybe these could be all one test).

I think we've also tended to test both short and long options.


================
Comment at: tools/llvm-objcopy/Opts.td:61
+                 MetaVarName<"symbol">,
+                 HelpText<"Localize <symbol>">;
+def L : JoinedOrSeparate<["-"], "L">,
----------------
Let's use a slightly more meaningful help message, that doesn't just repeat the switch name, e.g. "Mark <symbol> as local" (see also --localize-hidden).


Repository:
  rL LLVM

https://reviews.llvm.org/D46064