[PATCH] D62706: [llvm-lipo] Add docs for llvm-lipo

Michael Trent via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Jun 6 15:31:08 PDT 2019


mtrent added a comment.

I had trouble getting recommonmark.parser to install on my system, so I hacked at the patch to get the html and groff formatters to run. Should not be relevant to this review.

Please consider breaking apart the required commands from the optional options. This isn't important yet, but as soon as you add "-arch" you're going to have to deal with arguments that are optional and apply to more than one command, and arguments where one-and-only-one command is required. You can see an example in llvm-objdump.rst:

  SYNOPSIS
       llvm-objdump [commands] [options] [filenames...]
  
  DESCRIPTION
       The llvm-objdump utility prints stuff[...]
  
  COMMANDS
       At least one of the following commands are required, and some commands can be combined with other commands:
  [ ... ]
  
  OPTIONS
       llvm-objdump supports the following options:
  [ ... ] 

Recent versions of the lipo man page also try to make this clear, although sometimes it's hard.

I rendered the CommandGuide as man and also as html, seemed ok.



================
Comment at: docs/CommandGuide/llvm-lipo.rst:12
+:program:`llvm-lipo` can create universal binaries, extract regular 
+object files and report some information about the input.
+
----------------
Serial comma  is helpful in lists. "object files,"

"Report some information" is somewhat vague. Consider "report architectural information".

Consider "program can create universal binaries from [MDT? Mach-O?] files, extract regular object files from universal binaries, and display architecture information about both universal and regular files." 




================
Comment at: docs/CommandGuide/llvm-lipo.rst:26
+
+.. option:: -verify_arch <architecture_1> <architecture_2> ...
+
----------------
Only one arch is necessary. Consider -verify_arch <architecture 1> [<architecture 2> ...]


================
Comment at: docs/CommandGuide/llvm-lipo.rst:28
+
+  Verify that the specified architectures are present in the input file.
+
----------------
Verify that the specified architecture is present by doing what? Best to call out the command's exit status here.  

Is it obvious what "architecture" means here? How would you describe that to someone?

The -verify_arch command only works on a single file (the Apple man page suggests otherwise, but AFAICT the code has never supported that). Do you need to mention that here? 


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62706/new/

https://reviews.llvm.org/D62706





More information about the llvm-commits mailing list