This is an archive of the discontinued LLVM Phabricator instance.

Differential D101483

[llvm-objdump] add -v alias for --version
ClosedPublic

Authored by nickdesaulniers on Apr 28 2021, 1:10 PM.

Details

Reviewers
MaskRay
nathanchance
srhines
jhenderson
rupprecht
Commits
rGdde24a87c55f: [llvm-objdump] add -v alias for --version
Summary

Used by the Linux kernel's CONFIG_X86_DECODER_SELFTEST.

Link: https://github.com/ClangBuiltLinux/linux/issues/1130

Diff Detail

Event Timeline

nickdesaulniers created this revision.Apr 28 2021, 1:10 PM
Herald added a reviewer: jhenderson. · View Herald TranscriptApr 28 2021, 1:10 PM
Herald added subscribers: pengfei, rupprecht. · View Herald Transcript
nickdesaulniers requested review of this revision.Apr 28 2021, 1:10 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 28 2021, 1:10 PM
Herald added a subscriber: llvm-commits. · View Herald Transcript
MaskRay accepted this revision.Apr 28 2021, 1:39 PM

Looks great! Please wait for @jhenderson, though.

llvm/test/tools/llvm-objdump/version.test
4

CHECK: LLVM version

(While llvm-readobj/basic.test says the "LLVM" can be changed by cmake PACKAGE_NAME, I used "LLVM version" for llvm-symbolizer and it seems fine so far. )

This revision is now accepted and ready to land.Apr 28 2021, 1:39 PM
MaskRay added a comment.Apr 28 2021, 1:40 PM

a -v alias for --version

a -> Add

rupprecht accepted this revision.Apr 28 2021, 2:48 PM
rupprecht added inline comments.
llvm/test/tools/llvm-objdump/version.test
4

"LLVM version" is fine. Just don't go further than that -- e.g. don't check "LLVM version XX.Y"

Harbormaster completed remote builds in B101487: Diff 341298.Apr 28 2021, 3:52 PM
nickdesaulniers retitled this revision from [llvm-objdump] a -v alias for --version to [llvm-objdump] add -v alias for --version.Apr 28 2021, 5:01 PM
nickdesaulniers marked 2 inline comments as done.
nickdesaulniers added inline comments.
llvm/test/tools/llvm-objdump/version.test
4

My full version string looks like:

LLVM (http://llvm.org/):
  LLVM version 13.0.0git
...

so this is intentionally matching the initial LLVM; I don't think matching the word version adds any value here.

jhenderson added a comment.Apr 29 2021, 12:29 AM

Please update the CommandGuide docs (llvm/docs/CommandGuide) for llvm-objdump to include the new alias.

llvm/test/tools/llvm-objdump/version.test
4

I'd think it not unlikely for other output from the tool to incldue the LLVM string personally, so there's a (probably small) risk of this being a false positive.

nickdesaulniers updated this revision to Diff 341568.Apr 29 2021, 10:17 AM
  • update command guide, add "version" to test
Harbormaster completed remote builds in B101676: Diff 341568.Apr 29 2021, 11:34 AM
MaskRay accepted this revision.Apr 29 2021, 1:52 PM
jhenderson accepted this revision.Apr 30 2021, 1:11 AM

LGTM!

Closed by commit rGdde24a87c55f: [llvm-objdump] add -v alias for --version (authored by nickdesaulniers). · Explain WhyApr 30 2021, 11:37 AM
This revision was automatically updated to reflect the committed changes.
nickdesaulniers added a commit: rGdde24a87c55f: [llvm-objdump] add -v alias for --version.

Revision Contents

PathSize
llvm/
test/
tools/
llvm-objdump/
5 lines
tools/
llvm-objdump/
2 lines

Diff 341298

llvm/test/tools/llvm-objdump/version.test

  • This file was added.
RUN: llvm-objdump --version | FileCheck %s
RUN: llvm-objdump -v | FileCheck %s
CHECK: LLVM
MaskRayUnsubmitted
Done
ReplyInline Actions

CHECK: LLVM version

(While llvm-readobj/basic.test says the "LLVM" can be changed by cmake PACKAGE_NAME, I used "LLVM version" for llvm-symbolizer and it seems fine so far. )

MaskRay: `CHECK: LLVM version` (While llvm-readobj/basic.test says the "LLVM" can be changed by cmake…
rupprechtUnsubmitted
Done
ReplyInline Actions

"LLVM version" is fine. Just don't go further than that -- e.g. don't check "LLVM version XX.Y"

rupprecht: "LLVM version" is fine. Just don't go further than that -- e.g. don't check "LLVM version XX.Y"
nickdesaulniersAuthorUnsubmitted
Done
ReplyInline Actions

My full version string looks like:

LLVM (http://llvm.org/):
  LLVM version 13.0.0git
...

so this is intentionally matching the initial LLVM; I don't think matching the word version adds any value here.

nickdesaulniers: My full version string looks like: ``` LLVM (http://llvm.org/): LLVM version 13.0.0git ...
jhendersonUnsubmitted
Not Done
ReplyInline Actions

I'd think it not unlikely for other output from the tool to incldue the LLVM string personally, so there's a (probably small) risk of this being a false positive.

jhenderson: I'd think it not unlikely for other output from the tool to incldue the `LLVM` string…
CHECK: Registered Targets:

llvm/tools/llvm-objdump/ObjdumpOpts.td

include "llvm/Option/OptParser.td" include "llvm/Option/OptParser.td"
def grp_generic : OptionGroup<"kind">, HelpText<"Generic Options">; def grp_generic : OptionGroup<"kind">, HelpText<"Generic Options">;
def help : Flag<["--"], "help">, def help : Flag<["--"], "help">,
HelpText<"Display available options (--help-hidden for more)">, HelpText<"Display available options (--help-hidden for more)">,
Group<grp_generic>; Group<grp_generic>;
def help_hidden : Flag<["--"], "help-hidden">, def help_hidden : Flag<["--"], "help-hidden">,
Flags<[HelpHidden]>, Flags<[HelpHidden]>,
HelpText<"Display all available options">, HelpText<"Display all available options">,
Group<grp_generic>; Group<grp_generic>;
def version : Flag<["--"], "version">, def version : Flag<["--"], "version">,
HelpText<"Display the version of this program">, HelpText<"Display the version of this program">,
Group<grp_generic>; Group<grp_generic>;
def : Flag<["-"], "v">, Alias<version>, HelpText<"Alias for --version">,
Group<grp_generic>;
def grp_objdump : OptionGroup<"objdump">, HelpText<"llvm-objdump Options">; def grp_objdump : OptionGroup<"objdump">, HelpText<"llvm-objdump Options">;
def adjust_vma_EQ : Joined<["--"], "adjust-vma=">, def adjust_vma_EQ : Joined<["--"], "adjust-vma=">,
MetaVarName<"offset">, MetaVarName<"offset">,
HelpText<"Increase the displayed address by the specified offset">, HelpText<"Increase the displayed address by the specified offset">,
▲ Show 20 Lines • Show All 403 Lines • Show Last 20 Lines