[PATCH] D81907: [llvm-objcopy] Fix help text for install-name-tool

James Henderson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jun 16 01:04:25 PDT 2020


jhenderson added a comment.

llvm-strip doesn't allow a positional argument output either, as it can take multiple input files. This looks like a regression in the help text since an older version I have kicking around. Could you fix it for both (and update the patch description), please? You should also expand the help message test for both llvm-strip and llvm-objcopy to test that the full string has been matched.



================
Comment at: llvm/tools/llvm-objcopy/CopyConfig.cpp:398
                       StringRef ToolName) {
-  OptTable.PrintHelp(OS, (ToolName + " input [output]").str().c_str(),
+  StringRef Output = ToolName == "llvm-install-name-tool" ? "" : " [output]";
+  OptTable.PrintHelp(OS, (ToolName + " input" + Output).str().c_str(),
----------------
Rather than relying on a string for the name, I wonder if this code would be more robust with an enum that represents the three different tools, given that there's always potential for more front ends to be added. You would probably still want to pass in the tool name, but that could just be whatever the executable is called, possibly. For example, if the tool was llvm_install_name_tool, it would print that as the name in the help text message, rather than the current llvm-install-name-tool.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81907





More information about the llvm-commits mailing list