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

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
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]>;
+
----------------
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.



================
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"]>;
----------------
jhenderson wrote:
> Ditto.
The canonical forms are --format=just-symbols and -j now. It seems wasteful to have a third form --just-symbol-name.


================
Comment at: llvm/tools/llvm-nm/llvm-nm.cpp:87
+
+static bool ArchiveMap;
+static bool DebugSyms;
----------------
aganea wrote:
> Can you take the opportunity and put all this onto a stack-based state class? So that the tool can be used as-a-lib & be thread-safe? A practical example could be usage of  `llvm/tools/llvm-shlib/gen-msvc-exports.py`, but in-process.
Answered in D104889. Not sure localization will help for the oneshot utility.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105330