Page MenuHomePhabricator

[docs][llvm-readobj] Improve llvm-readobj documentation
ClosedPublic

Authored by jhenderson on Mon, Jun 24, 7:59 AM.

Details

Summary

There were a number of issues with the llvm-readobj documentation. The following points were raised in https://bugs.llvm.org/show_bug.cgi?id=42255, and have been fixed in this patch:

  1. The description section claimed "The tool and its output is primarily designed for use in FileCheck-based tests" which is not really the case any more.
  2. The documentation used single-dash long options for option names, but references in the help text to other options exclusively used double-dashes. Fixed by standardising on double-dashes for all long-form options.
  3. The majority of options available and in the help text were not present in the documentation. This patch adds them.
  4. Several aliases, both long and short, were missing, e.g. --relocs.

Additionally, this patch improves the documentation by:

  1. Splitting the options into categories based on the file format they are specific to.
  2. Updating the Exit Status section to correctly mention that errors lead to a non-zero exit code.
  3. Adding a See Also section referencing other similar LLVM tools.
  4. Improving/correcting some of the descriptions of options that did not quite match up with what llvm-readobj does.

Diff Detail

Event Timeline

jhenderson created this revision.Mon, Jun 24, 7:59 AM
Herald added a project: Restricted Project. · View Herald TranscriptMon, Jun 24, 7:59 AM

Thanks for doing this, this is a big improvement. One mild suggestion, but no objections from me.

docs/CommandGuide/llvm-readobj.rst
26 ↗(On Diff #206216)

It might be worth being less specific about what --all does due to the likelihood of it getting out of date as new options are added? I suspect that people wanting --all are those that want to dump all output to a text file in order to search interactively rather than need to know exactly what it does in order to parse it. Perhaps something like "Equivalent to adding the major output format options that are relevant to the file format."

I've not a strong opinion here, so feel free to ignore.

mtrent accepted this revision.Mon, Jun 24, 3:23 PM
This revision is now accepted and ready to land.Mon, Jun 24, 3:23 PM
MaskRay added inline comments.Mon, Jun 24, 11:35 PM
docs/CommandGuide/llvm-readobj.rst
80 ↗(On Diff #206216)

--sd if the intention is to standardize on double-dashes

(Say, at some point, for llvm-readelf alone, or both llvm-readelf and llvm-readobj, -sd becomes -s -d)

84 ↗(On Diff #206216)

--sr

134 ↗(On Diff #206216)

--dt

jhenderson marked 4 inline comments as done.

Make two-letter options use double-dashes, make the --all description less fragile to new options, and standardise on "display" instead of "print" for consistency throughout the document.

MaskRay accepted this revision.Tue, Jun 25, 4:17 AM

Thanks for the update LGTM too.

This revision was automatically updated to reflect the committed changes.