[llvm-strings] Switch command line parsing from llvm::cl to OptTable

Some behavior changes:

* `-t=d` is removed. Use `-t d` instead.
* one-dash long options like `-all` are supported. Use `--all` instead.
* `--all=0` or `--all=false` cannot be used. (Note: `--all` is silently ignored anyway)
* `--help-list` is removed. This is a `cl::` specific option.

Nobody is likely leveraging any of the above.

Advantages:

* `-t` diagnostic gets improved.
* in the absence of `HideUnrelatedOptions`, `--help` will not list unrelated options if linking against libLLVM-13git.so or linker GC is not used.
* Decrease the probability of cl::opt collision if we do decide to support multiplexing

Note: because the tool is so simple, used more for forensics instead of a building
tool, and its long options are unlikely used in one-dash form, I just drop the
one-dash form in this patch.

Reviewed By: jhenderson

Differential Revision: https://reviews.llvm.org/D104889

docs/CommandGuide/llvm-strings.rst

@@ -52,10 +52,6 @@ OPTIONS
 
  Display a summary of command line options.
 
-.. option:: --help-list
-
- Display an uncategorized summary of command line options.
-
 .. option:: --print-file-name, -f
 
  Display the name of the containing file before each string.

test/tools/llvm-strings/grouped.test

@@ -0,0 +1,4 @@
+## Show that short options can be grouped.
+
+RUN: echo abcd | llvm-strings -af | FileCheck %s
+CHECK: {standard input}: abcd

test/tools/llvm-strings/help.test

@@ -1,15 +1,11 @@
 ## Show that help text is printed correctly when requested.
 
-RUN: llvm-strings -h | FileCheck %s --check-prefixes=CHECK,CATEG
+RUN: llvm-strings -h | FileCheck %s
-RUN: llvm-strings --help | FileCheck %s --check-prefixes=CHECK,CATEG
+RUN: llvm-strings --help | FileCheck %s
-RUN: llvm-strings --help-list \
-RUN:   | FileCheck %s --check-prefixes=CHECK,LIST
 
 CHECK: OVERVIEW: llvm string dumper
-CHECK: USAGE: llvm-strings{{(.exe)?}} [options] <input object files>{{$}}
+CHECK: USAGE: llvm-strings [options] <input object files>{{$}}
 CHECK: OPTIONS:
-CATEG:    General options:
+CHECK:   --all
-LIST-NOT: General options:
+CHECK:   -a
-CATEG:    Generic Options:
+CHECK: Pass @FILE as argument to read options from FILE.
-LIST-NOT: Generic Options:
-CHECK: @FILE

test/tools/llvm-strings/length.test

@@ -21,9 +21,9 @@ RUN: llvm-strings --bytes 2 %t | FileCheck --check-prefix CHECK-2 %s --implicit-
 
 ## Show different syntaxes work.
 RUN: llvm-strings --bytes=2 %t | FileCheck --check-prefix CHECK-2 %s --implicit-check-not={{.}}
-RUN: llvm-strings -n=2 %t | FileCheck --check-prefix CHECK-2 %s --implicit-check-not={{.}}
+RUN: llvm-strings -n 2 %t | FileCheck --check-prefix CHECK-2 %s --implicit-check-not={{.}}
 
-CHECK-0: invalid minimum string length 0
+CHECK-0: llvm-strings: error: expected a positive integer, but got '0'
 
 CHECK-1:      a
 CHECK-1-NEXT: ab
@@ -43,4 +43,4 @@ CHECK-5:      abcde
 
 ## Show that a non-numeric argument is rejected.
 RUN: not llvm-strings -n foo %t 2>&1 | FileCheck %s --check-prefix=ERR
-ERR: llvm-strings{{.*}}: for the --bytes option: 'foo' value invalid for integer argument!
+ERR: llvm-strings: error: expected a positive integer, but got 'foo'

test/tools/llvm-strings/radix.test

@@ -26,7 +26,7 @@ RUN: llvm-strings --radix x %t/a.txt | FileCheck %s -check-prefix CHECK-HEX --st
 
 ## Show different syntaxes work.
 RUN: llvm-strings --radix=d %t/a.txt | FileCheck %s -check-prefix CHECK-DEC --strict-whitespace
-RUN: llvm-strings -t=d %t/a.txt | FileCheck %s -check-prefix CHECK-DEC --strict-whitespace
+RUN: llvm-strings -t d %t/a.txt | FileCheck %s -check-prefix CHECK-DEC --strict-whitespace
 
 CHECK-NONE: {{^}}three
 CHECK-NONE: {{^}}four
@@ -58,4 +58,4 @@ CHECK-HEX: {{^}}     28 nine
 
 ## Show that an invalid value is rejected.
 RUN: not llvm-strings --radix z %t/a.txt 2>&1 | FileCheck %s --check-prefix=INVALID
-INVALID: llvm-strings{{.*}}: for the --radix option: Cannot find option named 'z'!
+INVALID: llvm-strings: error: --radix value should be one of: '' (no offset), 'o' (octal), 'd' (decimal), 'x' (hexadecimal)

tools/llvm-strings/CMakeLists.txt

@@ -1,11 +1,18 @@
 set(LLVM_LINK_COMPONENTS
   Core
   Object
+  Option
   Support
   )
 
+set(LLVM_TARGET_DEFINITIONS Opts.td)
+tablegen(LLVM Opts.inc -gen-opt-parser-defs)
+add_public_tablegen_target(StringsOptsTableGen)
+
 add_llvm_tool(llvm-strings
   llvm-strings.cpp
+  DEPENDS
+  StringsOptsTableGen
   )
 
 if(LLVM_INSTALL_BINUTILS_SYMLINKS)

tools/llvm-strings/Opts.td

@@ -0,0 +1,23 @@
+include "llvm/Option/OptParser.td"
+
+class F<string letter, string help> : Flag<["-"], letter>, HelpText<help>;
+class FF<string name, string help> : Flag<["--"], name>, HelpText<help>;
+
+multiclass Eq<string name, string help> {
+  def NAME #_EQ : Joined<["--"], name #"=">,
+                  HelpText<help>;
+  def : Separate<["--"], name>, Alias<!cast<Joined>(NAME #_EQ)>;
+}
+
+def all : FF<"all", "Silently ignored. Present for GNU strings compatibility">;
+defm bytes : Eq<"bytes", "Print sequences of the specified length">;
+def help : FF<"help", "Display this help">;
+def print_file_name : Flag<["--"], "print-file-name">, HelpText<"Print the name of the file before each string">;
+defm radix : Eq<"radix", "Print the offset within the file with the specified radix: o (octal), d (decimal), x (hexadecimal)">, MetaVarName<"<radix>">;
+def version : FF<"version", "Display the version">;
+
+def : F<"a", "Alias for --all">, Alias<all>;
+def : F<"f", "Alias for --print-file-name">, Alias<print_file_name>;
+def : F<"h", "Alias for --help">, Alias<help>;
+def : JoinedOrSeparate<["-"], "n">, Alias<bytes_EQ>, HelpText<"Alias for --bytes">;
+def : JoinedOrSeparate<["-"], "t">, Alias<radix_EQ>, HelpText<"Alias for --radix">, MetaVarName<"<radix>">;

tools/llvm-strings/llvm-strings.cpp

@@ -11,51 +11,81 @@
 //
 //===----------------------------------------------------------------------===//
 
+#include "Opts.inc"
 #include "llvm/Object/Binary.h"
+#include "llvm/Option/Arg.h"
+#include "llvm/Option/ArgList.h"
+#include "llvm/Option/Option.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/Format.h"
 #include "llvm/Support/InitLLVM.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Program.h"
+#include "llvm/Support/WithColor.h"
 #include <cctype>
 #include <string>
 
 using namespace llvm;
 using namespace llvm::object;
 
+namespace {
+enum ID {
+  OPT_INVALID = 0, // This is not an option ID.
+#define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM,  \
+               HELPTEXT, METAVAR, VALUES)                                      \
+  OPT_##ID,
+#include "Opts.inc"
+#undef OPTION
+};
+
+#define PREFIX(NAME, VALUE) const char *const NAME[] = VALUE;
+#include "Opts.inc"
+#undef PREFIX
+
+static const opt::OptTable::Info InfoTable[] = {
+#define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM,  \
+               HELPTEXT, METAVAR, VALUES)                                      \
+  {                                                                            \
+      PREFIX,      NAME,      HELPTEXT,                                        \
+      METAVAR,     OPT_##ID,  opt::Option::KIND##Class,                        \
+      PARAM,       FLAGS,     OPT_##GROUP,                                     \
+      OPT_##ALIAS, ALIASARGS, VALUES},
+#include "Opts.inc"
+#undef OPTION
+};
+
+class StringsOptTable : public opt::OptTable {
+public:
+  StringsOptTable() : OptTable(InfoTable) { setGroupedShortOptions(true); }
+};
+} // namespace
+
+const char ToolName[] = "llvm-strings";
+
 static cl::list<std::string> InputFileNames(cl::Positional,
                                             cl::desc("<input object files>"),
                                             cl::ZeroOrMore);
 
-static cl::opt<bool>
+static int MinLength = 4;
-    PrintFileName("print-file-name",
+static bool PrintFileName;
-                  cl::desc("Print the name of the file before each string"));
-static cl::alias PrintFileNameShort("f", cl::desc(""),
-                                    cl::aliasopt(PrintFileName));
-
-static cl::opt<int>
-    MinLength("bytes", cl::desc("Print sequences of the specified length"),
-              cl::init(4));
-static cl::alias MinLengthShort("n", cl::desc(""), cl::aliasopt(MinLength));
-
-static cl::opt<bool>
-    AllSections("all",
-                  cl::desc("Check all sections, not just the data section"));
-static cl::alias AllSectionsShort("a", cl::desc(""),
-                                    cl::aliasopt(AllSections));
 
 enum radix { none, octal, hexadecimal, decimal };
-static cl::opt<radix>
+static radix Radix;
-    Radix("radix", cl::desc("print the offset within the file"),
-          cl::values(clEnumValN(octal, "o", "octal"),
-                     clEnumValN(hexadecimal, "x", "hexadecimal"),
-                     clEnumValN(decimal, "d", "decimal")),
-          cl::init(none));
-static cl::alias RadixShort("t", cl::desc(""), cl::aliasopt(Radix));
 
-static cl::extrahelp
+LLVM_ATTRIBUTE_NORETURN static void reportCmdLineError(const Twine &Message) {
-    HelpResponse("\nPass @FILE as argument to read options from FILE.\n");
+  WithColor::error(errs(), ToolName) << Message << "\n";
+  exit(1);
+}
+
+template <typename T>
+static void parseIntArg(const opt::InputArgList &Args, int ID, T &Value) {
+  if (const opt::Arg *A = Args.getLastArg(ID)) {
+    StringRef V(A->getValue());
+    if (!llvm::to_integer(V, Value, 0) || Value <= 0)
+      reportCmdLineError("expected a positive integer, but got '" + V + "'");
+  }
+}
 
 static void strings(raw_ostream &OS, StringRef FileName, StringRef Contents) {
   auto print = [&OS, FileName](unsigned Offset, StringRef L) {
@@ -96,13 +126,48 @@ static void strings(raw_ostream &OS, StringRef FileName, StringRef Contents) {
 
 int main(int argc, char **argv) {
   InitLLVM X(argc, argv);
+  BumpPtrAllocator A;
+  StringSaver Saver(A);
+  StringsOptTable Tbl;
+  opt::InputArgList Args =
+      Tbl.parseArgs(argc, argv, OPT_UNKNOWN, Saver,
+                    [&](StringRef Msg) { reportCmdLineError(Msg); });
+  if (Args.hasArg(OPT_help)) {
+    Tbl.printHelp(
+        outs(),
+        (Twine(ToolName) + " [options] <input object files>").str().c_str(),
+        "llvm string dumper");
+    // TODO Replace this with OptTable API once it adds extrahelp support.
+    outs() << "\nPass @FILE as argument to read options from FILE.\n";
+    return 0;
+  }
+  if (Args.hasArg(OPT_version)) {
+    outs() << ToolName << '\n';
+    cl::PrintVersionMessage();
+    return 0;
+  }
+
+  parseIntArg(Args, OPT_bytes_EQ, MinLength);
+  PrintFileName = Args.hasArg(OPT_print_file_name);
+  StringRef R = Args.getLastArgValue(OPT_radix_EQ);
+  if (R.empty())
+    Radix = none;
+  else if (R == "o")
+    Radix = octal;
+  else if (R == "d")
+    Radix = decimal;
+  else if (R == "x")
+    Radix = hexadecimal;
+  else
+    reportCmdLineError("--radix value should be one of: '' (no offset), 'o' "
+                       "(octal), 'd' (decimal), 'x' (hexadecimal)");
 
-  cl::ParseCommandLineOptions(argc, argv, "llvm string dumper\n");
   if (MinLength == 0) {
     errs() << "invalid minimum string length 0\n";
     return EXIT_FAILURE;
   }
 
+  std::vector<std::string> InputFileNames = Args.getAllArgValues(OPT_INPUT);
   if (InputFileNames.empty())
     InputFileNames.push_back("-");