This is an archive of the discontinued LLVM Phabricator instance.

[llvm-objdump] Fix --prefix and --prefix-strip
ClosedPublic

Authored by gbreynoo on Sep 27 2021, 7:32 AM.

Details

Summary

In the command guide --prefix and --prefix-strip is used in the form --prefix=<prefix> however currently it is used in the form --prefix <prefix>. This change fixes these options to match the command guide.

Diff Detail

Event Timeline

gbreynoo created this revision.Sep 27 2021, 7:32 AM
gbreynoo requested review of this revision.Sep 27 2021, 7:32 AM
Herald added a project: Restricted Project. · View Herald TranscriptSep 27 2021, 7:32 AM

Looking at the other options in llvm-objdump, --option=<value> seems to be the consistent format. For this reason I changed the command line option rather than the user guide, if people disagree I'm happy to change the command guide instead.

What does GNU objdump allow? We should allow whatever GNU does.

gbreynoo updated this revision to Diff 375282.Sep 27 2021, 8:31 AM

Fixed non windows test.

Looking at the GNU docs its` --prefix=prefix`.

Many utilities supporting double-dash long options (not only GNU's) generally use the GNU style getopt_long which supports both --foo value and --foo=value.

We can use lld/llvm-objcopy style tablegen Eq to support both Joined and Separate.

gbreynoo updated this revision to Diff 375525.Sep 28 2021, 4:30 AM

Updated after Maskray's comment.

I suggest that you use a multiclass Eq to avoid verbosity for individual options.

gbreynoo updated this revision to Diff 376239.Sep 30 2021, 8:54 AM

Updated after Maskray's comment.

Thanks Maskray, that is cleaner.

jhenderson accepted this revision.Oct 1 2021, 12:33 AM

LGTM! Please wait for @MaskRay too.

This revision is now accepted and ready to land.Oct 1 2021, 12:33 AM
MaskRay accepted this revision.Oct 6 2021, 7:20 PM

Thanks! Sorry for the slow response. I was out of town for one week.

This revision was automatically updated to reflect the committed changes.