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

[MaskRay]

Summary

Part of https://lists.llvm.org/pipermail/llvm-dev/2021-July/151622.html
"Binary utilities: switch command line parsing from llvm::cl to OptTable"

• --totals=false and --totals=0 cannot be used. Omit the option.
• --help-list is removed. This is a cl:: specific option.

OptTable avoids global option collision if we decide to support multiplexing for binary utilities.

Note: because the tool is simple, and its long options are uncommon, I just drop
the one-dash forms except -arch <value> (Darwin style).

[MaskRay]

forgot to include Opts.td

[MaskRay]

update doc about removal of --help-list

[jhenderson]

Note: because the tool is simple, and its long options are uncommon, I just drop

the one-dash forms in this patch.

No objection from me here, but just a thought about Mach-O users: if I'm not mistaken, single-dash long options are the norm on that platform. Might be worth pinging some Mach-O developers about that.

llvm/tools/llvm-size/llvm-size.cpp

919

(assuming we're dropping long form)

[jhenderson]

Forgot the LGTM, and obligatory @gbreynoo ping.

[MaskRay]

Keep -arch <value> (Darwin style one-dash long option)

llvm/docs/CommandGuide/llvm-size.rst

Segment : 48
              Section (__DATA, __data): 32
              total 48
      total 48
 
 .. option:: --help, -h
 
  Display a summary of command line options.
 
-.. option:: --help-list
-
- Display an uncategorized summary of command line options.
-
 .. option:: -m
 
  Equivalent to :option:`--format` with a value of ``darwin``.
 
 .. option:: -o
 
  Equivalent to :option:`--radix` with a value of ``8``.
 

llvm/test/tools/llvm-size/help.test

 ## Show that help text is printed correctly when requested.
 
-RUN: llvm-size -h | FileCheck %s --check-prefixes=CHECK,CATEG
-RUN: llvm-size --help | FileCheck %s --check-prefixes=CHECK,CATEG
-RUN: llvm-size --help-list \
-RUN:   | FileCheck %s --check-prefixes=CHECK,LIST
+RUN: llvm-size -h | FileCheck %s
+RUN: llvm-size --help | FileCheck %s
 
-CHECK: OVERVIEW: llvm object size dumper
-CHECK: USAGE: llvm-size{{(.exe)?}} [options] <input files>{{$}}
+CHECK: OVERVIEW: LLVM object size dumper
+CHECK: USAGE: {{.*}}llvm-size{{(.exe)?}} [options] <input object files>{{$}}
 CHECK: OPTIONS:
-CATEG:    Generic Options:
-LIST-NOT: Generic Options:
-CATEG:    llvm-size Options:
-LIST-NOT: llvm-size Options:
+CHECK: OPTIONS (Mach-O specific):
 CHECK: @FILE

llvm/test/tools/llvm-size/radix.test

 # RUN:   | FileCheck %s --check-prefix=BAD-VAL -DNUM=2
 # RUN: not llvm-size %t1.o --radix=32 2>&1 \
 # RUN:   | FileCheck %s --check-prefix=BAD-VAL -DNUM=32
 # RUN: not llvm-size %t1.o --radix=-1 2>&1 \
 # RUN:   | FileCheck %s --check-prefix=BAD-VAL -DNUM=-1
 # RUN: not llvm-size %t1.o --radix=bad 2>&1 \
 # RUN:   | FileCheck %s --check-prefix=BAD-VAL -DNUM=bad
 
-# BAD-VAL: {{.*}}llvm-size{{.*}}: for the --radix option: Cannot find option named '[[NUM]]'!
+# BAD-VAL: {{.*}}llvm-size{{.*}}: error: --radix value should be one of: 8, 10, 16
 
 --- !ELF
 FileHeader:
   Class:   ELFCLASS64
   Data:    ELFDATA2LSB
   Type:    ET_REL
   Machine: EM_X86_64
 Sections:

llvm/test/tools/llvm-size/unknown-format.test

 ## Show that the an error is emitted if an unknown value is passed to --format.
 
 # RUN: not llvm-size --format=unknown 2>&1 | FileCheck %s
-# CHECK: {{.*}}llvm-size{{.*}}: for the --format option: Cannot find option named 'unknown'!
+# CHECK: {{.*}}llvm-size{{.*}}: error: --format value should be one of: 'berkeley', 'darwin', 'sysv'

llvm/tools/llvm-size/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS
   Object
+  Option
   Support
   )
 
+set(LLVM_TARGET_DEFINITIONS Opts.td)
+tablegen(LLVM Opts.inc -gen-opt-parser-defs)
+add_public_tablegen_target(SizeOptsTableGen)
+
 add_llvm_tool(llvm-size
   llvm-size.cpp
+  DEPENDS
+  SizeOptsTableGen
   )
 
 if(LLVM_INSTALL_BINUTILS_SYMLINKS)
   add_llvm_tool_symlink(size llvm-size)
 endif()

llvm/tools/llvm-size/Opts.td

+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 common : FF<"common", "Print common symbols in the ELF file. When using Berkeley format, this is added to bss">;
+defm format : Eq<"format", "Specify output format">;
+def help : FF<"help", "Display this help">;
+defm radix : Eq<"radix", "Print size in radix">;
+def totals : FF<"totals", "Print totals of all objects - Berkeley format only">;
+def version : FF<"version", "Display the version">;
+
+// Mach-O specific options.
+def grp_mach_o : OptionGroup<"kind">, HelpText<"OPTIONS (Mach-O specific)">;
+def arch_EQ : Joined<["--"], "arch=">, HelpText<"architecture(s) from a Mach-O file to dump">, Group<grp_mach_o>;
+def : Separate<["--", "-"], "arch">, Alias<arch_EQ>;
+def l : F<"l", "When format is darwin, use long format to include addresses and offsets">, Group<grp_mach_o>;
+
+def : F<"A", "Alias for --format">, Alias<format_EQ>, AliasArgs<["sysv"]>;
+def : F<"B", "Alias for --format">, Alias<format_EQ>, AliasArgs<["berkeley"]>;
+def : F<"d", "Alias for --radix=10">, Alias<radix_EQ>, AliasArgs<["10"]>;
+def : F<"h", "Alias for --help">, Alias<help>;
+def : F<"m", "Alias for --format">, Alias<format_EQ>, AliasArgs<["darwin"]>;
+def : F<"o", "Alias for --radix=8">, Alias<radix_EQ>, AliasArgs<["8"]>;
+def : F<"t", "Alias for --totals">, Alias<totals>;
+def : F<"x", "Alias for --radix=16">, Alias<radix_EQ>, AliasArgs<["16"]>;

llvm/tools/llvm-size/llvm-size.cpp

 //===----------------------------------------------------------------------===//
 
 #include "llvm/ADT/APInt.h"
 #include "llvm/Object/Archive.h"
 #include "llvm/Object/ELFObjectFile.h"
 #include "llvm/Object/MachO.h"
 #include "llvm/Object/MachOUniversal.h"
 #include "llvm/Object/ObjectFile.h"
+#include "llvm/Option/Arg.h"
+#include "llvm/Option/ArgList.h"
+#include "llvm/Option/Option.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Format.h"
 #include "llvm/Support/InitLLVM.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/WithColor.h"
 #include "llvm/Support/raw_ostream.h"
 #include <algorithm>
 #include <string>
 #include <system_error>
 
 using namespace llvm;
 using namespace object;
 
-cl::OptionCategory SizeCat("llvm-size Options");
+namespace {
+using namespace llvm::opt; // for HelpHidden in Opts.inc
+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
+
+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 SizeOptTable : public opt::OptTable {
+public:
+  SizeOptTable() : OptTable(InfoTable) { setGroupedShortOptions(true); }
+};
 
 enum OutputFormatTy { berkeley, sysv, darwin };
-static cl::opt<OutputFormatTy>
-    OutputFormat("format", cl::desc("Specify output format"),
-                 cl::values(clEnumVal(sysv, "System V format"),
-                            clEnumVal(berkeley, "Berkeley format"),
-                            clEnumVal(darwin, "Darwin -m format")),
-                 cl::init(berkeley), cl::cat(SizeCat));
-
-static cl::opt<OutputFormatTy>
-    OutputFormatShort(cl::desc("Specify output format"),
-                      cl::values(clEnumValN(sysv, "A", "System V format"),
-                                 clEnumValN(berkeley, "B", "Berkeley format"),
-                                 clEnumValN(darwin, "m", "Darwin -m format")),
-                      cl::init(berkeley), cl::cat(SizeCat));
+enum RadixTy { octal = 8, decimal = 10, hexadecimal = 16 };
+} // namespace
+
+static bool ArchAll = false;
+static std::vector<StringRef> ArchFlags;
+static bool ELFCommons;
+static OutputFormatTy OutputFormat;
+static bool DarwinLongFormat;
+static RadixTy Radix;
+static bool TotalSizes;
+
+static std::vector<std::string> InputFilenames;
+
+static std::string ToolName;
 
+// States
+static bool HadError = false;
 static bool BerkeleyHeaderPrinted = false;
 static bool MoreThanOneFile = false;
 static uint64_t TotalObjectText = 0;
 static uint64_t TotalObjectData = 0;
 static uint64_t TotalObjectBss = 0;
 static uint64_t TotalObjectTotal = 0;
 
-cl::opt<bool>
-    DarwinLongFormat("l",
-                     cl::desc("When format is darwin, use long format "
-                              "to include addresses and offsets."),
-                     cl::cat(SizeCat));
-
-cl::opt<bool>
-    ELFCommons("common",
-               cl::desc("Print common symbols in the ELF file.  When using "
-                        "Berkeley format, this is added to bss."),
-               cl::init(false), cl::cat(SizeCat));
-
-static cl::list<std::string>
-    ArchFlags("arch", cl::desc("architecture(s) from a Mach-O file to dump"),
-              cl::ZeroOrMore, cl::cat(SizeCat));
-static bool ArchAll = false;
-
-enum RadixTy { octal = 8, decimal = 10, hexadecimal = 16 };
-static cl::opt<RadixTy> Radix(
-    "radix", cl::desc("Print size in radix"), cl::init(decimal),
-    cl::values(clEnumValN(octal, "8", "Print size in octal"),
-               clEnumValN(decimal, "10", "Print size in decimal"),
-               clEnumValN(hexadecimal, "16", "Print size in hexadecimal")),
-    cl::cat(SizeCat));
-
-static cl::opt<RadixTy> RadixShort(
-    cl::desc("Print size in radix:"),
-    cl::values(clEnumValN(octal, "o", "Print size in octal"),
-               clEnumValN(decimal, "d", "Print size in decimal"),
-               clEnumValN(hexadecimal, "x", "Print size in hexadecimal")),
-    cl::init(decimal), cl::cat(SizeCat));
-
-static cl::opt<bool>
-    TotalSizes("totals",
-               cl::desc("Print totals of all objects - Berkeley format only"),
-               cl::init(false), cl::cat(SizeCat));
-
-static cl::alias TotalSizesShort("t", cl::desc("Short for --totals"),
-                                 cl::aliasopt(TotalSizes));
-
-static cl::list<std::string>
-    InputFilenames(cl::Positional, cl::desc("<input files>"), cl::ZeroOrMore);
-
-static cl::extrahelp
-    HelpResponse("\nPass @FILE as argument to read options from FILE.\n");
-
-static bool HadError = false;
-
-static std::string ToolName;
-
-static void error(const Twine &Message, StringRef File) {
+static void error(const Twine &Message, StringRef File = "") {
   HadError = true;
-  WithColor::error(errs(), ToolName) << "'" << File << "': " << Message << "\n";
+  if (File.empty())
+    WithColor::error(errs(), ToolName) << Message << '\n';
+  else
+    WithColor::error(errs(), ToolName)
+        << "'" << File << "': " << Message << '\n';
 }
 
 // This version of error() prints the archive name and member name, for example:
 // "libx.a(foo.o)" after the ToolName before the error message.  It sets
 // HadError but returns allowing the code to move on to other archive members.
 static void error(llvm::Error E, StringRef FileName, const Archive::Child &C,
                   StringRef ArchitectureName = StringRef()) {
   HadError = true;
static void printBerkeleyTotals() {
   fmt << "%7" << (Radix == octal ? PRIo64 : PRIu64) << "\t"
       << "%7" PRIx64 "\t";
   outs() << format(fmt.str().c_str(), TotalObjectTotal, TotalObjectTotal)
          << "(TOTALS)\n";
 }
 
 int main(int argc, char **argv) {
   InitLLVM X(argc, argv);
-  cl::HideUnrelatedOptions(SizeCat);
-  cl::ParseCommandLineOptions(argc, argv, "llvm object size dumper\n");
-
+  BumpPtrAllocator A;
+  StringSaver Saver(A);
+  SizeOptTable Tbl;
   ToolName = argv[0];
-  if (OutputFormatShort.getNumOccurrences())
-    OutputFormat = static_cast<OutputFormatTy>(OutputFormatShort);
-  if (RadixShort.getNumOccurrences())
-    Radix = RadixShort.getValue();
+  opt::InputArgList Args = Tbl.parseArgs(argc, argv, OPT_UNKNOWN, Saver,
+                                         [&](StringRef Msg) { error(Msg); });
+  if (Args.hasArg(OPT_help)) {
+    Tbl.printHelp(
+        outs(),
+        (Twine(ToolName) + " [options] <input object files>").str().c_str(),
+        "LLVM object size 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;
+  }
+
+  ELFCommons = Args.hasArg(OPT_common);
+  DarwinLongFormat = Args.hasArg(OPT_l);
+  TotalSizes = Args.hasArg(OPT_totals);
+  StringRef V = Args.getLastArgValue(OPT_format_EQ, "berkeley");
+  if (V == "berkeley")
+    OutputFormat = berkeley;
+  else if (V == "darwin")
+    OutputFormat = darwin;
+  else if (V == "sysv")
+    OutputFormat = sysv;
+  else
+    error("--format value should be one of: 'berkeley', 'darwin', 'sysv'");
+  V = Args.getLastArgValue(OPT_radix_EQ, "10");
+  if (V == "8")
+    Radix = RadixTy::octal;
+  else if (V == "10")
+    Radix = RadixTy::decimal;
+  else if (V == "16")
+    Radix = RadixTy::hexadecimal;
+  else
+    error("--radix value should be one of: 8, 10, 16 ");
 
-  for (StringRef Arch : ArchFlags) {
-    if (Arch == "all") {
+  for (const auto *A : Args.filtered(OPT_arch_EQ)) {
+    SmallVector<StringRef, 2> Values;
+    llvm::SplitString(A->getValue(), Values, ",");
+    for (StringRef V : Values) {
+      if (V == "all")
         ArchAll = true;
-    } else {
-      if (!MachOObjectFile::isValidArch(Arch)) {
+      else if (MachOObjectFile::isValidArch(V))
+        ArchFlags.push_back(V);
+      else {
         outs() << ToolName << ": for the -arch option: Unknown architecture "
-               << "named '" << Arch << "'";
+               << "named '" << V << "'";
         return 1;
       }
     }
   }
 
+  InputFilenames = Args.getAllArgValues(OPT_INPUT);
   if (InputFilenames.empty())
     InputFilenames.push_back("a.out");
 
   MoreThanOneFile = InputFilenames.size() > 1;
   llvm::for_each(InputFilenames, printFileSectionSizes);
   if (OutputFormat == berkeley && TotalSizes)
     printBerkeleyTotals();
 
   if (HadError)
     return 1;
 }

llvm/utils/gn/secondary/llvm/tools/llvm-size/BUILD.gn

 import("//llvm/tools/binutils_symlinks.gni")
+import("//llvm/utils/TableGen/tablegen.gni")
 import("//llvm/utils/gn/build/symlink_or_copy.gni")
 
+tablegen("Opts") {
+  visibility = [ ":llvm-size" ]
+  args = [ "-gen-opt-parser-defs" ]
+}
+
 if (llvm_install_binutils_symlinks) {
   symlink_or_copy("size") {
     deps = [ ":llvm-size" ]
     source = "llvm-size"
     output = "$root_out_dir/bin/size"
   }
 }
 
 # //:llvm-size depends on this symlink target, see comment in //BUILD.gn.
 group("symlinks") {
   deps = [ ":llvm-size" ]
   if (llvm_install_binutils_symlinks) {
     deps += [ ":size" ]
   }
 }
 
 executable("llvm-size") {
   deps = [
+    ":Opts",
     "//llvm/lib/Object",
+    "//llvm/lib/Option",
     "//llvm/lib/Support",
   ]
   sources = [ "llvm-size.cpp" ]
 }

utils/bazel/llvm-project-overlay/llvm/BUILD.bazel

deps = [
         ":ExecutionEngine",
         ":MC",
         ":MCDisassembler",
         ":Object",
         ":Support",
     ],
 )
 
+gentbl(
+    name = "SizeOptsTableGen",
+    strip_include_prefix = "tools/llvm-size",
+    tbl_outs = [(
+        "-gen-opt-parser-defs",
+        "tools/llvm-size/Opts.inc",
+    )],
+    tblgen = ":llvm-tblgen",
+    td_file = "tools/llvm-size/Opts.td",
+    td_srcs = ["include/llvm/Option/OptParser.td"],
+)
+
 cc_binary(
     name = "llvm-size",
     srcs = glob([
         "tools/llvm-size/*.cpp",
         "tools/llvm-size/*.h",
     ]),
     copts = llvm_copts,
     stamp = 0,
     deps = [
         ":Object",
+        ":Option",
+        ":SizeOptsTableGen",
         ":Support",
     ],
 )
 
 cc_binary(
     name = "llvm-split",
     srcs = glob([
         "tools/llvm-split/*.cpp",