[llvm] 98f0783 - [llvm-strings] Switch command line parsing from llvm::cl to OptTable
Fangrui Song via llvm-commits
llvm-commits at lists.llvm.org
Mon Jul 5 10:46:24 PDT 2021
Author: Fangrui Song
Date: 2021-07-05T10:46:17-07:00
New Revision: 98f078324fc51da460660920f4a1aa308bfc3547
URL: https://github.com/llvm/llvm-project/commit/98f078324fc51da460660920f4a1aa308bfc3547
DIFF: https://github.com/llvm/llvm-project/commit/98f078324fc51da460660920f4a1aa308bfc3547.diff
LOG: [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
Added:
llvm/test/tools/llvm-strings/grouped.test
llvm/tools/llvm-strings/Opts.td
Modified:
llvm/docs/CommandGuide/llvm-strings.rst
llvm/test/tools/llvm-strings/help.test
llvm/test/tools/llvm-strings/length.test
llvm/test/tools/llvm-strings/radix.test
llvm/tools/llvm-strings/CMakeLists.txt
llvm/tools/llvm-strings/llvm-strings.cpp
Removed:
################################################################################
diff --git a/llvm/docs/CommandGuide/llvm-strings.rst b/llvm/docs/CommandGuide/llvm-strings.rst
index e2fe4cb88c9ae..f66b22ec8df09 100644
--- a/llvm/docs/CommandGuide/llvm-strings.rst
+++ b/llvm/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.
diff --git a/llvm/test/tools/llvm-strings/grouped.test b/llvm/test/tools/llvm-strings/grouped.test
new file mode 100644
index 0000000000000..b5c38b6cc86b1
--- /dev/null
+++ b/llvm/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
diff --git a/llvm/test/tools/llvm-strings/help.test b/llvm/test/tools/llvm-strings/help.test
index d5d4ca80a4f44..5872dc9753204 100644
--- a/llvm/test/tools/llvm-strings/help.test
+++ b/llvm/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 --help | FileCheck %s --check-prefixes=CHECK,CATEG
-RUN: llvm-strings --help-list \
-RUN: | FileCheck %s --check-prefixes=CHECK,LIST
+RUN: llvm-strings -h | FileCheck %s
+RUN: llvm-strings --help | FileCheck %s
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:
-LIST-NOT: General options:
-CATEG: Generic Options:
-LIST-NOT: Generic Options:
-CHECK: @FILE
+CHECK: --all
+CHECK: -a
+CHECK: Pass @FILE as argument to read options from FILE.
diff --git a/llvm/test/tools/llvm-strings/length.test b/llvm/test/tools/llvm-strings/length.test
index ac25c523c94f5..4897931f38fe3 100644
--- a/llvm/test/tools/llvm-strings/length.test
+++ b/llvm/test/tools/llvm-strings/length.test
@@ -21,9 +21,9 @@ RUN: llvm-strings --bytes 2 %t | FileCheck --check-prefix CHECK-2 %s --implicit-
## Show
diff erent 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'
diff --git a/llvm/test/tools/llvm-strings/radix.test b/llvm/test/tools/llvm-strings/radix.test
index 4dafbd1c84fc9..e3ef4ce3a0cd8 100644
--- a/llvm/test/tools/llvm-strings/radix.test
+++ b/llvm/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
diff erent 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)
diff --git a/llvm/tools/llvm-strings/CMakeLists.txt b/llvm/tools/llvm-strings/CMakeLists.txt
index 390f117513978..95dd69c778e47 100644
--- a/llvm/tools/llvm-strings/CMakeLists.txt
+++ b/llvm/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)
diff --git a/llvm/tools/llvm-strings/Opts.td b/llvm/tools/llvm-strings/Opts.td
new file mode 100644
index 0000000000000..2ad77fae6c15f
--- /dev/null
+++ b/llvm/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>">;
diff --git a/llvm/tools/llvm-strings/llvm-strings.cpp b/llvm/tools/llvm-strings/llvm-strings.cpp
index 51313d73401e2..0b068749917be 100644
--- a/llvm/tools/llvm-strings/llvm-strings.cpp
+++ b/llvm/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>
- PrintFileName("print-file-name",
- 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));
+static int MinLength = 4;
+static bool PrintFileName;
enum radix { none, octal, hexadecimal, decimal };
-static cl::opt<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 radix Radix;
-static cl::extrahelp
- HelpResponse("\nPass @FILE as argument to read options from FILE.\n");
+LLVM_ATTRIBUTE_NORETURN static void reportCmdLineError(const Twine &Message) {
+ 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("-");
More information about the llvm-commits
mailing list