This is an archive of the discontinued LLVM Phabricator instance.

[llvm-nm] Support the -V option, print that the tool is compatible with GNU nm
ClosedPublic

Authored by mstorsjo on May 12 2021, 2:48 AM.

Diff Detail

Event Timeline

mstorsjo created this revision.May 12 2021, 2:48 AM
mstorsjo requested review of this revision.May 12 2021, 2:48 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 12 2021, 2:48 AM

llvm/docs/CommandGuide/llvm-nm.rst needs update.

llvm/test/tools/llvm-nm/libtool-version.test
4

Move the file level comment above the commands.

llvm/tools/llvm-nm/llvm-nm.cpp
2241

Does libtool detect the substring GNU nm? If not, the notice should be dropped.

For example, ld.lld prints compatible with GNU linkers just because libtool detects GNU (D31199).

2241

If yes, there needs to be a comment.

llvm/docs/CommandGuide/llvm-nm.rst needs update.

Ok, will do

llvm/test/tools/llvm-nm/libtool-version.test
4

Sure, will do

llvm/tools/llvm-nm/llvm-nm.cpp
2241

Libtool detects ”GNU” only, but not necessarily “GNU nm”. Is this wording ok (while adding a comment explaining why) or do you want it written differently?

MaskRay added inline comments.May 12 2021, 10:33 AM
llvm/tools/llvm-nm/llvm-nm.cpp
2241

"llvm-nm, compatible with GNU nm\n" is ok, we just need to document the fact.

mstorsjo updated this revision to Diff 344884.May 12 2021, 11:32 AM

Addressed the feedback.

MaskRay accepted this revision.May 12 2021, 11:43 AM

Looks great! Please wait for @jhenderson

llvm/test/tools/llvm-nm/libtool-version.test
6

Add CHECK: LLVM version

(See some similar --version tests)

This revision is now accepted and ready to land.May 12 2021, 11:43 AM

Looks good, with a couple of nits.

llvm/docs/CommandGuide/llvm-nm.rst
235–236

Perhaps change the end of the description here to something like "... executable, then exit." The exiting aspect I think deserves describing, since llvm-nm doesn't need any other commands to still do useful work on an object normally.

llvm/test/tools/llvm-nm/libtool-version.test
1–2

I know they're not strictly necessary here, but I'd add some comment markers to make the comments stand out from the rest of the test.

Thanks for the reviews!

llvm/docs/CommandGuide/llvm-nm.rst
235–236

Ok, amending that.

llvm/test/tools/llvm-nm/libtool-version.test
1–2

Sure, will do