[PATCH] D105330: [llvm-nm] Switch command line parsing from llvm::cl to OptTable

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/docs/CommandGuide/llvm-nm.rst:135
 
- Print only symbols defined in this file.
+ Print only symbols defined in this file. ``-U`` is a deprecated alias.
 
----------------
Up to you, but I personally think it's fine to remove reference to -U (also -W, -M) entirely.


================
Comment at: llvm/docs/ReleaseNotes.rst:173
+* The llvm-nm short aliases ``-M`` (``--print-armap``), ``-U``
+  (``--defined-only``), and ``-W`` (``--no-weak``) are deprecated.
+  (`D105055 <https://reviews.llvm.org/D105330>`_)
----------------
Either "have been deprecated" or "are now deprecated". Perhaps also say "Use the long form versions instead." or some equivalent phrasing.


================
Comment at: llvm/docs/ReleaseNotes.rst:174
+  (``--defined-only``), and ``-W`` (``--no-weak``) are deprecated.
+  (`D105055 <https://reviews.llvm.org/D105330>`_)
+
----------------
This is the wrong link.


================
Comment at: llvm/tools/llvm-nm/Opts.td:38
+def version : FF<"version", "Display the version">;
+def without_aliases : FF<"without-aliases", "Exclude aliases from output">, Flags<[HelpHidden]>;
+
----------------
MaskRay wrote:
> jhenderson wrote:
> > I don't follow why you've hidden this option?
> --without-aliases is currently hidden. It seems to be for debugging.
> 
> The name is confusing: it applies to just LLVM bitcode files but the documentation isn't clear about this fact.
> 
It's in the command guide text. Either we should remove it from there, or it should be visible in the help text.


================
Comment at: llvm/tools/llvm-nm/Opts.td:51
+
+def : FF<"just-symbol-name", "Alias for --format=just-symbols">, Alias<format_EQ>, AliasArgs<["just-symbols"]>, Flags<[HelpHidden]>;
+def : FF<"portability", "Alias for --format=posix">, Alias<format_EQ>, AliasArgs<["posix"]>;
----------------
MaskRay wrote:
> jhenderson wrote:
> > Ditto.
> The canonical forms are --format=just-symbols and -j now. It seems wasteful to have a third form --just-symbol-name.
Fair enough. In that case, can we go through a deprecation process like the short option aliases? (Namely remove it from the command guide and add a release note).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105330