This is an archive of the discontinued LLVM Phabricator instance.

[llvm-objdump] - Show aliases in -help.
ClosedPublic

Authored by grimar on Jan 17 2019, 7:04 AM.

Details

Summary

Currently llvm-objdump is inconsistent.

When -help is specified it shows no aliases except two.
Aliases are shown with -help-hidden though.
GNU objdump also prints them by default.

This patch does a change to always show all aliases when -help is given.

I included no test case because seems there is no good way to test it:

  • Does not seem we want to test the whole -help output. It seems an overkill.
  • Does not seem we want to test only lines with the aliases, because it is brittle

and would be easy to forget to update such test case.

  • In LLD we test a single option:

https://github.com/llvm-mirror/lld/blob/master/test/ELF/help.s
IMO it is not very useful actually. It effectively tests that we print at least one alias.
It does not save from any mistakes.

Diff Detail

Event Timeline

grimar created this revision.Jan 17 2019, 7:04 AM
ruiu accepted this revision.Jan 17 2019, 10:16 AM
ruiu added a subscriber: ruiu.

LGTM

I think this is a good change, and I agree with you that we don't need to test each alias.

Speaking of the --help message, I found there are other weird stuff there. The options in the --help message are categorized as "Color Options", "General options" and "Generic Options". There is only one option under the "Color Options" category, which is --color. Most options are under "General Options". --help and --version are under "Generic Options". Frankly, these categories doesn't make sense at all to me. (what's the difference between "Generic" and "General"?)

Maybe we should remove the categories and display all options just asciibetically.

This revision is now accepted and ready to land.Jan 17 2019, 10:16 AM

LGTM

I think this is a good change, and I agree with you that we don't need to test each alias.

Speaking of the --help message, I found there are other weird stuff there. The options in the --help message are categorized as "Color Options", "General options" and "Generic Options". There is only one option under the "Color Options" category, which is --color. Most options are under "General Options". --help and --version are under "Generic Options". Frankly, these categories doesn't make sense at all to me. (what's the difference between "Generic" and "General"?)

Maybe we should remove the categories and display all options just asciibetically.

Yeah, I also noticed that. I *guess* it somehow can be related with GNU objdump which seems to divide options into two groups (required and optional):

At least one of the following switches must be given:

-a, --archive-headers    Display archive header information
-f, --file-headers       Display the contents of the overall file header
-p, --private-headers    Display object format specific file header contents

....

The following switches are optional:

-b, --target=BFDNAME           Specify the target object format as BFDNAME
-m, --architecture=MACHINE     Specify the target architecture as MACHINE
-j, --section=NAME             Only display information for section NAME
-M, --disassembler-options=OPT Pass text OPT on to the disassembler

....

Perhaps the intention was to do something like that, but something went wrong. I agree that just listing them asciibetically would be better than what is done now.
I'll try to suggest a patch on the next week if nobody do that earlier.

Thanks for working on this! I agree that testing for this is somewhat tricky, and I don't have any suggestions for how to improve it.

I have a feeling that the option categorisation is more general to LLVM, rather than specifically llvm-objdump, from memory, but I haven't dug into this recently. IIRC, the "Generic" options category is not provided by llvm-objdump at all, and is instead somewhere in the CommandLine library.

This revision was automatically updated to reflect the committed changes.

I have a feeling that the option categorisation is more general to LLVM, rather than specifically llvm-objdump, from memory, but I haven't dug into this recently. IIRC, the "Generic" options category is not provided by llvm-objdump at all, and is instead somewhere in the CommandLine library.

Yes. And the current behavior seems intentional (it is documented): to print command line options as an uncategorized list, -help-list can be used (https://llvm.org/docs/CommandLine.html#grouping-options-into-categories).
FTR, LLD, llvm-objcopy and few other tools use a different approach based on llvm::opt::OptTable for parsing options and we have no strange categories there.

Seems if we want to change this for llvm-objdump we probably might want to reimplement it in the same way I guess.