This is an archive of the discontinued LLVM Phabricator instance.

[llvm-objdump --macho] Rename --dyld_info to --dyld-info
ClosedPublic

Authored by thakis on Aug 15 2022, 8:17 AM.

Details

Summary

llvm-objdump takes foo-bar style flags, while llvm-otool takes foo_bar style
flags. dyld_info was the only exception to that.

Diff Detail

Event Timeline

thakis created this revision.Aug 15 2022, 8:17 AM
Herald added a reviewer: MaskRay. · View Herald Transcript
Herald added a project: Restricted Project. · View Herald Transcript
Herald added a subscriber: rupprecht. · View Herald Transcript
thakis requested review of this revision.Aug 15 2022, 8:17 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 15 2022, 8:17 AM
Herald added a subscriber: StephenFan. · View Herald Transcript
thakis updated this revision to Diff 452680.Aug 15 2022, 8:18 AM

update docs too

I picked this option name to match the system tools on macOS, which also use this spelling (but I agree it's inconsistent).

Right, we have llvm-otool which matches the otool flags. None of the other objdump flags match the cctools's option spellings.

MaskRay accepted this revision.Aug 15 2022, 9:51 PM
This revision is now accepted and ready to land.Aug 15 2022, 9:51 PM
jhenderson accepted this revision.Aug 16 2022, 12:11 AM

I've got nothing against this, although it probably deserves a release note, in case there are any existing users of that spelling and llvm-objdump.

thakis updated this revision to Diff 452980.Aug 16 2022, 6:11 AM

add -dyld_info flag to llvm-otool

aprantl: I added -dyld_info to llvm-otool instead. Does that work for you?

jhenderson: The flag currently doesn't really do anything, it calls PrintDyldInfo which calls one more function which internally casts everything to (void). So I don't think there are any actual non-test users of this flag, and maybe we can get away without a release notes entry in this case?

jhenderson: The flag currently doesn't really do anything, it calls PrintDyldInfo which calls one more function which internally casts everything to (void). So I don't think there are any actual non-test users of this flag, and maybe we can get away without a release notes entry in this case?

Up to you - if you don't think it's needed, that's fine (I don't know anything about its usage or behaviour).

Splitting it this way works for me (although it's still going to be confusing that we have two spellings).

Great, thanks!

This revision was landed with ongoing or failed builds.Aug 17 2022, 9:58 AM
This revision was automatically updated to reflect the committed changes.
llvm/test/Object/AArch64/chained-fixups-header.test