[llvm] [Support] Remove cl::Grouping (PR #224958)
Fangrui Song via llvm-commits
llvm-commits at lists.llvm.org
Thu Sep 24 10:47:00 PDT 2026
https://github.com/MaskRay updated https://github.com/llvm/llvm-project/pull/224958
>From 5ad0307b7dbece4836ca2b44b558e782bb41dff1 Mon Sep 17 00:00:00 2001
From: Fangrui Song <i at maskray.me>
Date: Sun, 20 Sep 2026 11:59:47 -0700
Subject: [PATCH 1/2] [llvm-cov] Migrate gcov to OptTable
llvm-cov gcov is the last user of cl::Grouping. Parse its options with
an OptTable that enables grouped short options, as the other
binutils-style tools do, so that cl::Grouping can be removed.
Spellings follow the other migrated tools: long options take `--` only,
so `-gcno` and `-gcda=` become `--gcno` and `--gcda=`. `--help` now
lists only gcov's options rather than every cl:: option linked into
llvm-cov, and an unknown option or a missing source file is reported
as `llvm-cov gcov: error: ...`.
LLM-aided
---
llvm/docs/CommandGuide/llvm-cov.md | 4 +-
llvm/test/tools/llvm-cov/gcov/basic.test | 14 +-
llvm/tools/llvm-cov/CMakeLists.txt | 7 +
llvm/tools/llvm-cov/GcovOpts.td | 51 ++++++
llvm/tools/llvm-cov/gcov.cpp | 168 ++++++++----------
.../gn/secondary/llvm/tools/llvm-cov/BUILD.gn | 9 +
.../llvm-project-overlay/llvm/BUILD.bazel | 11 ++
7 files changed, 157 insertions(+), 107 deletions(-)
create mode 100644 llvm/tools/llvm-cov/GcovOpts.td
diff --git a/llvm/docs/CommandGuide/llvm-cov.md b/llvm/docs/CommandGuide/llvm-cov.md
index 11508c00d2598..54ac3e05c8b36 100644
--- a/llvm/docs/CommandGuide/llvm-cov.md
+++ b/llvm/docs/CommandGuide/llvm-cov.md
@@ -107,7 +107,7 @@ an entire source file.
:::
:::{option} --help
-Display available options (--help-hidden for more).
+Display available options.
:::
:::{option} -l, --long-file-names
@@ -157,7 +157,7 @@ Include unconditional branches in the output for the --branch-probabilities
option.
:::
-:::{option} -version
+:::{option} --version
Display the version of llvm-cov.
:::
diff --git a/llvm/test/tools/llvm-cov/gcov/basic.test b/llvm/test/tools/llvm-cov/gcov/basic.test
index 7557739add8ba..9c849d4824a8b 100644
--- a/llvm/test/tools/llvm-cov/gcov/basic.test
+++ b/llvm/test/tools/llvm-cov/gcov/basic.test
@@ -57,11 +57,11 @@ RUN: llvm-cov gcov -l test_paths.cpp
RUN: ls test_paths.cpp##a.c.gcov
# Long file names and preserve paths.
-RUN: mkdir -p src && llvm-cov gcov -lp -gcno test_paths.gcno -gcda test_paths.gcda src/../test_paths.cpp
+RUN: mkdir -p src && llvm-cov gcov -lp --gcno test_paths.gcno --gcda test_paths.gcda src/../test_paths.cpp
RUN: ls src#^#test_paths.cpp##src#a.c.gcov
# Hash pathnames.
-RUN: llvm-cov gcov -x -gcno test_paths.gcno -gcda test_paths.gcda src/../test_paths.cpp
+RUN: llvm-cov gcov -x --gcno test_paths.gcno --gcda test_paths.gcda src/../test_paths.cpp
RUN: ls a.c##0c546a4dd99c1774b7b06e4fad16158c.gcov
# Function summaries. This changes stdout, but not the gcov files.
@@ -153,7 +153,7 @@ RUN: FileCheck --input-file=test.h.gcov %s --check-prefix=H-C
H-C: unconditional 0 taken 1
# Missing gcda file just gives 0 counts.
-RUN: llvm-cov gcov test.c -gcda=no_such_gcda_file | FileCheck %s --check-prefix=NO-GCDA
+RUN: llvm-cov gcov test.c --gcda=no_such_gcda_file | FileCheck %s --check-prefix=NO-GCDA
RUN: diff -ub test_no_gcda.cpp.gcov test.cpp.gcov
RUN: diff -ub test_no_gcda.h.gcov test.h.gcov
NO-GCDA: File 'test.cpp'
@@ -165,19 +165,19 @@ NO-GCDA-NEXT: Lines executed:0.00% of 1
NO-GCDA-NEXT: Creating 'test.h.gcov'
# Invalid gcno file.
-RUN: llvm-cov gcov test.c -gcno=test_read_fail.gcno
+RUN: llvm-cov gcov test.c --gcno=test_read_fail.gcno
# Not a .gcda file. Error but keep the .gcov output.
RUN: echo invalid > not.gcda
-RUN: llvm-cov gcov test.c -gcda=not.gcda 2> %t.err | FileCheck %s --check-prefix=NO-GCDA
+RUN: llvm-cov gcov test.c --gcda=not.gcda 2> %t.err | FileCheck %s --check-prefix=NO-GCDA
RUN: FileCheck %s --check-prefix=NOT-GCDA < %t.err
NOT-GCDA: not.gcda:not a gcov data file
# Bad file checksum on gcda.
-RUN: llvm-cov gcov test.c -gcda=test_file_checksum_fail.gcda 2> %t.err | FileCheck %s --check-prefix=NO-GCDA
+RUN: llvm-cov gcov test.c --gcda=test_file_checksum_fail.gcda 2> %t.err | FileCheck %s --check-prefix=NO-GCDA
# Bad function checksum on gcda
-RUN: llvm-cov gcov test.c -gcda=test_func_checksum_fail.gcda 2> %t.err | FileCheck %s --check-prefix=NO-GCDA
+RUN: llvm-cov gcov test.c --gcda=test_func_checksum_fail.gcda 2> %t.err | FileCheck %s --check-prefix=NO-GCDA
# Has arcs from exit blocks
RUN-DISABLED: llvm-cov gcov test_exit_block_arcs.c 2>&1 | FileCheck %s -check-prefix=EXIT_BLOCK_ARCS
diff --git a/llvm/tools/llvm-cov/CMakeLists.txt b/llvm/tools/llvm-cov/CMakeLists.txt
index 6602a2319cb57..41e826ac88a46 100644
--- a/llvm/tools/llvm-cov/CMakeLists.txt
+++ b/llvm/tools/llvm-cov/CMakeLists.txt
@@ -2,11 +2,16 @@ set(LLVM_LINK_COMPONENTS
Core
Support
Object
+ Option
Coverage
ProfileData
TargetParser
)
+set(LLVM_TARGET_DEFINITIONS GcovOpts.td)
+tablegen(LLVM GcovOpts.inc -gen-opt-parser-defs)
+add_public_tablegen_target(CovGcovOptsTableGen)
+
add_llvm_tool(llvm-cov
llvm-cov.cpp
gcov.cpp
@@ -20,6 +25,8 @@ add_llvm_tool(llvm-cov
SourceCoverageViewHTML.cpp
SourceCoverageViewText.cpp
TestingSupport.cpp
+ DEPENDS
+ CovGcovOptsTableGen
)
target_link_libraries(llvm-cov PRIVATE LLVMHTTP LLVMDebuginfod)
diff --git a/llvm/tools/llvm-cov/GcovOpts.td b/llvm/tools/llvm-cov/GcovOpts.td
new file mode 100644
index 0000000000000..fbf1123bb424d
--- /dev/null
+++ b/llvm/tools/llvm-cov/GcovOpts.td
@@ -0,0 +1,51 @@
+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_blocks : FF<"all-blocks", "Display all basic blocks">;
+def branch_counts : FF<"branch-counts", "Display branch counts instead of percentages (requires -b)">;
+def branch_probabilities : FF<"branch-probabilities", "Display branch probabilities">;
+def demangled_names : FF<"demangled-names", "Demangle function names">;
+def dump_gcov : FF<"dump", "Dump the gcov file to stderr">;
+def function_summaries : FF<"function-summaries", "Show coverage for each function">;
+defm gcda : Eq<"gcda", "Override inferred gcda file">, MetaVarName<"<file>">;
+defm gcno : Eq<"gcno", "Override inferred gcno file">, MetaVarName<"<file>">;
+def hash_filenames : FF<"hash-filenames", "Hash long pathnames">;
+def help : FF<"help", "Display this help">;
+// Supported by gcov 4.9~8. gcov 9 (GCC r265587) removed --intermediate-format
+// and -i was changed to mean --json-format. We consider this format still
+// useful and support -i.
+def intermediate_format : FF<"intermediate-format", "Output .gcov in intermediate text format">;
+def long_file_names : FF<"long-file-names", "Prefix filenames with the main file">;
+def no_output : FF<"no-output", "Do not output any .gcov files">;
+defm object_directory : Eq<"object-directory", "Find objects in DIR or based on FILE's path">, MetaVarName<"<DIR>">;
+def : Joined<["--"], "object-file=">, Alias<object_directory_EQ>, HelpText<"Alias for --object-directory">, MetaVarName<"<FILE>">;
+def : Separate<["--"], "object-file">, Alias<object_directory_EQ>;
+def preserve_paths : FF<"preserve-paths", "Preserve path components">;
+def relative_only : FF<"relative-only", "Only dump files with relative paths or absolute paths with the prefix specified by -s">;
+defm source_prefix : Eq<"source-prefix", "Source prefix to elide">, MetaVarName<"<prefix>">;
+def stdout : FF<"stdout", "Print to stdout">;
+def unconditional_branches : FF<"unconditional-branches", "Display unconditional branch info (requires -b)">;
+def version : FF<"version", "Display the version">;
+
+def : F<"a", "Alias for --all-blocks">, Alias<all_blocks>;
+def : F<"b", "Alias for --branch-probabilities">, Alias<branch_probabilities>;
+def : F<"c", "Alias for --branch-counts">, Alias<branch_counts>;
+def : F<"f", "Alias for --function-summaries">, Alias<function_summaries>;
+def : F<"i", "Alias for --intermediate-format">, Alias<intermediate_format>;
+def : F<"l", "Alias for --long-file-names">, Alias<long_file_names>;
+def : F<"m", "Alias for --demangled-names">, Alias<demangled_names>;
+def : F<"n", "Alias for --no-output">, Alias<no_output>;
+def : JoinedOrSeparate<["-"], "o">, Alias<object_directory_EQ>, HelpText<"Alias for --object-directory">, MetaVarName<"<DIR|FILE>">;
+def : F<"p", "Alias for --preserve-paths">, Alias<preserve_paths>;
+def : F<"r", "Alias for --relative-only">, Alias<relative_only>;
+def : JoinedOrSeparate<["-"], "s">, Alias<source_prefix_EQ>, HelpText<"Alias for --source-prefix">, MetaVarName<"<prefix>">;
+def : F<"t", "Alias for --stdout">, Alias<stdout>;
+def : F<"u", "Alias for --unconditional-branches">, Alias<unconditional_branches>;
+def : F<"x", "Alias for --hash-filenames">, Alias<hash_filenames>;
diff --git a/llvm/tools/llvm-cov/gcov.cpp b/llvm/tools/llvm-cov/gcov.cpp
index 00ea12415b220..f500d2a3e8d5e 100644
--- a/llvm/tools/llvm-cov/gcov.cpp
+++ b/llvm/tools/llvm-cov/gcov.cpp
@@ -12,17 +12,42 @@
#include "llvm/ProfileData/GCOV.h"
#include "llvm/ADT/SmallString.h"
+#include "llvm/Option/ArgList.h"
+#include "llvm/Option/OptTable.h"
+#include "llvm/Option/Option.h"
#include "llvm/Support/CommandLine.h"
#include "llvm/Support/Errc.h"
#include "llvm/Support/FileSystem.h"
#include "llvm/Support/Path.h"
+#include "llvm/Support/StringSaver.h"
+#include "llvm/Support/WithColor.h"
#include <system_error>
using namespace llvm;
+namespace {
+enum ID {
+ OPT_INVALID = 0, // This is not an option ID.
+#define OPTION(...) LLVM_MAKE_OPT_ID(__VA_ARGS__),
+#include "GcovOpts.inc"
+#undef OPTION
+};
+
+using namespace llvm::opt;
+#define OPTTABLE_CODE
+#include "GcovOpts.inc"
+
+class GcovOptTable : public opt::OptTable {
+public:
+ GcovOptTable() : OptTable(optionTables()) {
+ setGroupedShortOptions(true);
+ setDashDashParsing(true);
+ }
+};
+} // namespace
+
static void reportCoverage(StringRef SourceFile, StringRef ObjectDir,
- const std::string &InputGCNO,
- const std::string &InputGCDA, bool DumpGCOV,
- const GCOV::Options &Options) {
+ StringRef InputGCNO, StringRef InputGCDA,
+ bool DumpGCOV, const GCOV::Options &Options) {
SmallString<128> CoverageFileStem(ObjectDir);
if (CoverageFileStem.empty()) {
// If no directory was specified with -o, look next to the source file.
@@ -35,10 +60,10 @@ static void reportCoverage(StringRef SourceFile, StringRef ObjectDir,
// A file was given. Ignore the source file and look next to this file.
sys::path::replace_extension(CoverageFileStem, "");
- std::string GCNO =
- InputGCNO.empty() ? std::string(CoverageFileStem) + ".gcno" : InputGCNO;
- std::string GCDA =
- InputGCDA.empty() ? std::string(CoverageFileStem) + ".gcda" : InputGCDA;
+ std::string GCNO = InputGCNO.empty() ? std::string(CoverageFileStem) + ".gcno"
+ : InputGCNO.str();
+ std::string GCDA = InputGCDA.empty() ? std::string(CoverageFileStem) + ".gcda"
+ : InputGCDA.str();
GCOVFile GF;
// Open .gcda and .gcda without requiring a NUL terminator. The concurrent
@@ -81,97 +106,44 @@ static void reportCoverage(StringRef SourceFile, StringRef ObjectDir,
}
int gcovMain(int argc, const char *argv[]) {
- cl::list<std::string> SourceFiles(cl::Positional, cl::OneOrMore,
- cl::desc("SOURCEFILE"));
-
- cl::opt<bool> AllBlocks("a", cl::Grouping, cl::init(false),
- cl::desc("Display all basic blocks"));
- cl::alias AllBlocksA("all-blocks", cl::aliasopt(AllBlocks));
-
- cl::opt<bool> BranchProb("b", cl::Grouping, cl::init(false),
- cl::desc("Display branch probabilities"));
- cl::alias BranchProbA("branch-probabilities", cl::aliasopt(BranchProb));
-
- cl::opt<bool> BranchCount("c", cl::Grouping, cl::init(false),
- cl::desc("Display branch counts instead "
- "of percentages (requires -b)"));
- cl::alias BranchCountA("branch-counts", cl::aliasopt(BranchCount));
-
- cl::opt<bool> LongNames("l", cl::Grouping, cl::init(false),
- cl::desc("Prefix filenames with the main file"));
- cl::alias LongNamesA("long-file-names", cl::aliasopt(LongNames));
-
- cl::opt<bool> FuncSummary("f", cl::Grouping, cl::init(false),
- cl::desc("Show coverage for each function"));
- cl::alias FuncSummaryA("function-summaries", cl::aliasopt(FuncSummary));
-
- // Supported by gcov 4.9~8. gcov 9 (GCC r265587) removed --intermediate-format
- // and -i was changed to mean --json-format. We consider this format still
- // useful and support -i.
- cl::opt<bool> Intermediate(
- "intermediate-format", cl::init(false),
- cl::desc("Output .gcov in intermediate text format"));
- cl::alias IntermediateA("i", cl::desc("Alias for --intermediate-format"),
- cl::Grouping, cl::NotHidden,
- cl::aliasopt(Intermediate));
-
- cl::opt<bool> Demangle("demangled-names", cl::init(false),
- cl::desc("Demangle function names"));
- cl::alias DemangleA("m", cl::desc("Alias for --demangled-names"),
- cl::Grouping, cl::NotHidden, cl::aliasopt(Demangle));
-
- cl::opt<bool> NoOutput("n", cl::Grouping, cl::init(false),
- cl::desc("Do not output any .gcov files"));
- cl::alias NoOutputA("no-output", cl::aliasopt(NoOutput));
-
- cl::opt<std::string> ObjectDir(
- "o", cl::value_desc("DIR|FILE"), cl::init(""),
- cl::desc("Find objects in DIR or based on FILE's path"));
- cl::alias ObjectDirA("object-directory", cl::aliasopt(ObjectDir));
- cl::alias ObjectDirB("object-file", cl::aliasopt(ObjectDir));
-
- cl::opt<bool> PreservePaths("p", cl::Grouping, cl::init(false),
- cl::desc("Preserve path components"));
- cl::alias PreservePathsA("preserve-paths", cl::aliasopt(PreservePaths));
-
- cl::opt<bool> RelativeOnly(
- "r", cl::Grouping,
- cl::desc("Only dump files with relative paths or absolute paths with the "
- "prefix specified by -s"));
- cl::alias RelativeOnlyA("relative-only", cl::aliasopt(RelativeOnly));
- cl::opt<std::string> SourcePrefix("s", cl::desc("Source prefix to elide"));
- cl::alias SourcePrefixA("source-prefix", cl::aliasopt(SourcePrefix));
-
- cl::opt<bool> UseStdout("t", cl::Grouping, cl::init(false),
- cl::desc("Print to stdout"));
- cl::alias UseStdoutA("stdout", cl::aliasopt(UseStdout));
-
- cl::opt<bool> UncondBranch("u", cl::Grouping, cl::init(false),
- cl::desc("Display unconditional branch info "
- "(requires -b)"));
- cl::alias UncondBranchA("unconditional-branches", cl::aliasopt(UncondBranch));
-
- cl::opt<bool> HashFilenames("x", cl::Grouping, cl::init(false),
- cl::desc("Hash long pathnames"));
- cl::alias HashFilenamesA("hash-filenames", cl::aliasopt(HashFilenames));
-
-
- cl::OptionCategory DebugCat("Internal and debugging options");
- cl::opt<bool> DumpGCOV("dump", cl::init(false), cl::cat(DebugCat),
- cl::desc("Dump the gcov file to stderr"));
- cl::opt<std::string> InputGCNO("gcno", cl::cat(DebugCat), cl::init(""),
- cl::desc("Override inferred gcno file"));
- cl::opt<std::string> InputGCDA("gcda", cl::cat(DebugCat), cl::init(""),
- cl::desc("Override inferred gcda file"));
-
- cl::ParseCommandLineOptions(argc, argv, "LLVM code coverage tool\n");
-
- GCOV::Options Options(AllBlocks, BranchProb, BranchCount, FuncSummary,
- PreservePaths, UncondBranch, Intermediate, LongNames,
- Demangle, NoOutput, RelativeOnly, UseStdout,
- HashFilenames, SourcePrefix);
-
- for (const auto &SourceFile : SourceFiles)
+ StringRef ToolName = sys::path::filename(argv[0]);
+ auto Error = [&](const Twine &Msg) {
+ WithColor::error(errs(), ToolName) << Msg << '\n';
+ exit(1);
+ };
+ BumpPtrAllocator A;
+ StringSaver Saver(A);
+ GcovOptTable Tbl;
+ opt::InputArgList Args =
+ Tbl.parseArgs(argc, const_cast<char **>(argv), OPT_UNKNOWN, Saver, Error);
+ if (Args.hasArg(OPT_help)) {
+ Tbl.printHelp(outs(), (ToolName + " [options] SOURCEFILE").str().c_str(),
+ "LLVM code coverage tool");
+ return 0;
+ }
+ if (Args.hasArg(OPT_version)) {
+ cl::PrintVersionMessage();
+ return 0;
+ }
+ std::vector<std::string> SourceFiles = Args.getAllArgValues(OPT_INPUT);
+ if (SourceFiles.empty())
+ Error("no source file specified");
+
+ GCOV::Options Options(
+ Args.hasArg(OPT_all_blocks), Args.hasArg(OPT_branch_probabilities),
+ Args.hasArg(OPT_branch_counts), Args.hasArg(OPT_function_summaries),
+ Args.hasArg(OPT_preserve_paths), Args.hasArg(OPT_unconditional_branches),
+ Args.hasArg(OPT_intermediate_format), Args.hasArg(OPT_long_file_names),
+ Args.hasArg(OPT_demangled_names), Args.hasArg(OPT_no_output),
+ Args.hasArg(OPT_relative_only), Args.hasArg(OPT_stdout),
+ Args.hasArg(OPT_hash_filenames),
+ Args.getLastArgValue(OPT_source_prefix_EQ).str());
+
+ StringRef ObjectDir = Args.getLastArgValue(OPT_object_directory_EQ);
+ StringRef InputGCNO = Args.getLastArgValue(OPT_gcno_EQ);
+ StringRef InputGCDA = Args.getLastArgValue(OPT_gcda_EQ);
+ bool DumpGCOV = Args.hasArg(OPT_dump_gcov);
+ for (const std::string &SourceFile : SourceFiles)
reportCoverage(SourceFile, ObjectDir, InputGCNO, InputGCDA, DumpGCOV,
Options);
return 0;
diff --git a/llvm/utils/gn/secondary/llvm/tools/llvm-cov/BUILD.gn b/llvm/utils/gn/secondary/llvm/tools/llvm-cov/BUILD.gn
index bf2493c1820a9..3fa2f04df93d4 100644
--- a/llvm/utils/gn/secondary/llvm/tools/llvm-cov/BUILD.gn
+++ b/llvm/utils/gn/secondary/llvm/tools/llvm-cov/BUILD.gn
@@ -1,10 +1,19 @@
+import("//llvm/utils/TableGen/tablegen.gni")
+
+tablegen("GcovOpts") {
+ visibility = [ ":llvm-cov" ]
+ args = [ "-gen-opt-parser-defs" ]
+}
+
executable("llvm-cov") {
deps = [
+ ":GcovOpts",
"//llvm/include/llvm/Config:llvm-config",
"//llvm/lib/Debuginfod",
"//llvm/lib/HTTP",
"//llvm/lib/IR",
"//llvm/lib/Object",
+ "//llvm/lib/Option",
"//llvm/lib/ProfileData",
"//llvm/lib/ProfileData/Coverage",
"//llvm/lib/Support",
diff --git a/utils/bazel/llvm-project-overlay/llvm/BUILD.bazel b/utils/bazel/llvm-project-overlay/llvm/BUILD.bazel
index b7765075c72fb..fe23e55ade755 100644
--- a/utils/bazel/llvm-project-overlay/llvm/BUILD.bazel
+++ b/utils/bazel/llvm-project-overlay/llvm/BUILD.bazel
@@ -5244,6 +5244,15 @@ cc_binary(
],
)
+gentbl_cc_library(
+ name = "CovGcovOptsTableGen",
+ strip_include_prefix = "tools/llvm-cov",
+ tbl_outs = {"tools/llvm-cov/GcovOpts.inc": ["-gen-opt-parser-defs"]},
+ tblgen = ":llvm-tblgen",
+ td_file = "tools/llvm-cov/GcovOpts.td",
+ deps = [":OptParserTdFiles"],
+)
+
cc_binary(
name = "llvm-cov",
srcs = glob([
@@ -5253,11 +5262,13 @@ cc_binary(
copts = llvm_copts,
stamp = 0,
deps = [
+ ":CovGcovOptsTableGen",
":Coverage",
":Debuginfod",
":HTTP",
":Instrumentation",
":Object",
+ ":Option",
":ProfileData",
":Support",
":TargetParser",
>From 893d81381e3e5fe3e3cd4d9090dfef67f3e6d1ce Mon Sep 17 00:00:00 2001
From: Fangrui Song <i at maskray.me>
Date: Sun, 20 Sep 2026 12:09:44 -0700
Subject: [PATCH 2/2] [Support] Remove cl::Grouping
This feature emulates POSIX's grouped short options in a non-perfect
way. https://reviews.llvm.org/D61270 made every single-charater
cl::option implicitly group, leading to weird error message for `opt
-foo=bar`: `opt: for the -o option: may not occur within a group!`
Every tool (primarily binutils-style tools) has since migrated to OptTable,
with llvm-cov gcov the last (#224955).
Delete this feature, which would block TableGen based representation.
LLM-aided
---
llvm/docs/CommandLine.md | 52 +----
llvm/include/llvm/Support/CommandLine.h | 12 +-
llvm/lib/Support/CommandLine.cpp | 129 ++++---------
llvm/unittests/Support/CommandLineTest.cpp | 209 ++-------------------
4 files changed, 52 insertions(+), 350 deletions(-)
diff --git a/llvm/docs/CommandLine.md b/llvm/docs/CommandLine.md
index 2cf37ebc52d42..4df2a7282f890 100644
--- a/llvm/docs/CommandLine.md
+++ b/llvm/docs/CommandLine.md
@@ -58,8 +58,7 @@ CommandLine library to have the following features:
1. Capable: The CommandLine library can handle lots of different forms of
options often found in real programs. For example, {ref}`positional <positional>` arguments,
- `ls` style {ref}`grouping <grouping>` options (to allow processing '`ls -lad`'
- naturally), `ld` style {ref}`prefix <prefix>` options (to parse '`-lmalloc
+ `ld` style {ref}`prefix <prefix>` options (to parse '`-lmalloc
-L/usr/lib`'), and interpreter style options.
This document will hopefully let you jump in and start using CommandLine in your
@@ -1165,55 +1164,6 @@ As usual, you can only specify one of these arguments at most.
**cl::Prefix** options must not have the **cl::ValueDisallowed** modifier
specified.
-(grouping)=
-(cl::Grouping)=
-
-#### Controlling options grouping
-
-The **cl::Grouping** modifier can be combined with any formatting types except
-for {ref}`cl::Positional <cl::Positional>`. It is used to implement Unix-style tools (like `ls`)
-that have lots of single letter arguments, but only require a single dash.
-For example, the '`ls -labF`' command actually enables four different options,
-all of which are single letters.
-
-Note that **cl::Grouping** options can have values only if they are used
-separately or at the end of the groups. For {ref}`cl::ValueRequired <cl::ValueRequired>`, it is
-a runtime error if such an option is used elsewhere in the group.
-
-The CommandLine library does not restrict how you use the **cl::Prefix** or
-**cl::Grouping** modifiers, but it is possible to specify ambiguous argument
-settings. Thus, it is possible to have multiple letter options that are prefix
-or grouping options, and they will still work as designed.
-
-To do this, the CommandLine library uses a greedy algorithm to parse the input
-option into (potentially multiple) prefix and grouping options. The strategy
-basically looks like this:
-
-```
-parse(string OrigInput) {
-
-1. string Input = OrigInput;
-2. if (isOption(Input)) return getOption(Input).parse(); // Normal option
-3. while (!Input.empty() && !isOption(Input)) Input.pop_back(); // Remove the last letter
-4. while (!Input.empty()) {
- string MaybeValue = OrigInput.substr(Input.length())
- if (getOption(Input).isPrefix())
- return getOption(Input).parse(MaybeValue)
- if (!MaybeValue.empty() && MaybeValue[0] == '=')
- return getOption(Input).parse(MaybeValue.substr(1))
- if (!getOption(Input).isGrouping())
- return error()
- getOption(Input).parse()
- Input = OrigInput = MaybeValue
- while (!Input.empty() && !isOption(Input)) Input.pop_back();
- if (!Input.empty() && !getOption(Input).isGrouping())
- return error()
- }
-5. if (!OrigInput.empty()) error();
-
-}
-```
-
#### Miscellaneous option modifiers
The miscellaneous option modifiers are the only flags where you can specify more
diff --git a/llvm/include/llvm/Support/CommandLine.h b/llvm/include/llvm/Support/CommandLine.h
index dd42c4bfb7f49..50d662177a970 100644
--- a/llvm/include/llvm/Support/CommandLine.h
+++ b/llvm/include/llvm/Support/CommandLine.h
@@ -163,12 +163,6 @@ enum MiscFlags { // Miscellaneous flags to adjust argument
PositionalEatsArgs = 0x02, // Should this positional cl::list eat -args?
Sink = 0x04, // Should this cl::list eat all unknown options?
- // Can this option group with other options?
- // If this is enabled, multiple letter options are allowed to bunch together
- // with only a single hyphen for the whole group. This allows emulation
- // of the behavior that ls uses for example: ls -la === ls -l -a
- Grouping = 0x08,
-
// Default option
DefaultOption = 0x10
};
@@ -1343,11 +1337,7 @@ template <> struct applicator<FormattingFlags> {
};
template <> struct applicator<MiscFlags> {
- static void opt(MiscFlags MF, Option &O) {
- assert((MF != Grouping || O.ArgStr.size() == 1) &&
- "cl::Grouping can only apply to single character Options.");
- O.setMiscFlag(MF);
- }
+ static void opt(MiscFlags MF, Option &O) { O.setMiscFlag(MF); }
};
// Apply modifiers to an option in a type safe way.
diff --git a/llvm/lib/Support/CommandLine.cpp b/llvm/lib/Support/CommandLine.cpp
index 03ef86bb225d3..59204db590b4c 100644
--- a/llvm/lib/Support/CommandLine.cpp
+++ b/llvm/lib/Support/CommandLine.cpp
@@ -140,15 +140,6 @@ static SmallString<8> argPrefix(StringRef ArgName, size_t Pad = DefaultPad) {
return Prefix;
}
-// Option predicates...
-static inline bool isGrouping(const Option *O) {
- return O->getMiscFlags() & cl::Grouping;
-}
-static inline bool isPrefixedOrGrouping(const Option *O) {
- return isGrouping(O) || O->getFormattingFlag() == cl::Prefix ||
- O->getFormattingFlag() == cl::AlwaysPrefix;
-}
-
using OptionsMapTy = DenseMap<StringRef, Option *>;
namespace {
@@ -418,7 +409,8 @@ class CommandLineParser {
Option *LookupLongOption(SubCommand &Sub, StringRef &Arg, StringRef &Value,
bool LongOptionsUseDoubleDash, bool HaveDoubleDash) {
Option *Opt = LookupOption(Sub, Arg, Value);
- if (Opt && LongOptionsUseDoubleDash && !HaveDoubleDash && !isGrouping(Opt))
+ if (Opt && LongOptionsUseDoubleDash && !HaveDoubleDash &&
+ Opt->ArgStr.size() != 1)
return nullptr;
return Opt;
}
@@ -480,8 +472,6 @@ void Option::setArgStr(StringRef S) {
globalParser().updateArgStr(this, S);
assert(!S.starts_with("-") && "Option can't start with '-");
ArgStr = S;
- if (ArgStr.size() == 1)
- setMiscFlag(Grouping);
}
void Option::addCategory(OptionCategory &C) {
@@ -748,92 +738,44 @@ bool llvm::cl::ProvidePositionalOption(Option *Handler, StringRef Arg, int i) {
return ProvideOption(Handler, Handler->ArgStr, Arg, 0, nullptr, Dummy);
}
-// getOptionPred - Check to see if there are any options that satisfy the
-// specified predicate with names that are the prefixes in Name. This is
-// checked by progressively stripping characters off of the name, checking to
-// see if there options that satisfy the predicate. If we find one, return it,
-// otherwise return null.
-//
-static Option *getOptionPred(StringRef Name, size_t &Length,
- bool (*Pred)(const Option *),
- const OptionsMapTy &OptionsMap) {
- auto OMI = OptionsMap.find(Name);
- if (OMI != OptionsMap.end() && !Pred(OMI->second))
- OMI = OptionsMap.end();
-
- // Loop while we haven't found an option and Name still has at least two
- // characters in it (so that the next iteration will not be the empty
- // string.
- while (OMI == OptionsMap.end() && Name.size() > 1) {
- Name = Name.drop_back();
- OMI = OptionsMap.find(Name);
- if (OMI != OptionsMap.end() && !Pred(OMI->second))
- OMI = OptionsMap.end();
- }
-
- if (OMI != OptionsMap.end() && Pred(OMI->second)) {
- Length = Name.size();
- return OMI->second; // Found one!
- }
- return nullptr; // No option found!
-}
-
-/// HandlePrefixedOrGroupedOption - The specified argument string (which started
-/// with at least one '-') does not fully match an available option. Check to
-/// see if this is a prefix or grouped option. If so, split arg into output an
-/// Arg/Value pair and return the Option to parse it with.
-static Option *HandlePrefixedOrGroupedOption(StringRef &Arg, StringRef &Value,
- bool &ErrorParsing,
- const OptionsMapTy &OptionsMap) {
+// Find the cl::Prefix or cl::AlwaysPrefix option whose name is the longest
+// prefix of Name.
+static Option *findPrefixOption(StringRef Name, size_t &Length,
+ const OptionsMapTy &OptionsMap) {
+ for (; !Name.empty(); Name = Name.drop_back()) {
+ Option *O = OptionsMap.lookup(Name);
+ if (O && (O->getFormattingFlag() == cl::Prefix ||
+ O->getFormattingFlag() == cl::AlwaysPrefix)) {
+ Length = Name.size();
+ return O;
+ }
+ }
+ return nullptr;
+}
+
+/// HandlePrefixedOption - The specified argument string (which started with at
+/// least one '-') does not fully match an available option. Check to see if
+/// this is a prefix option. If so, split arg into output an Arg/Value pair and
+/// return the Option to parse it with.
+static Option *HandlePrefixedOption(StringRef &Arg, StringRef &Value,
+ const OptionsMapTy &OptionsMap) {
if (Arg.size() == 1)
return nullptr;
- // Do the lookup!
size_t Length = 0;
- Option *PGOpt = getOptionPred(Arg, Length, isPrefixedOrGrouping, OptionsMap);
- if (!PGOpt)
+ Option *POpt = findPrefixOption(Arg, Length, OptionsMap);
+ if (!POpt)
return nullptr;
- do {
- StringRef MaybeValue =
- (Length < Arg.size()) ? Arg.substr(Length) : StringRef();
- Arg = Arg.substr(0, Length);
- assert(OptionsMap.count(Arg) && OptionsMap.find(Arg)->second == PGOpt);
-
- // cl::Prefix options do not preserve '=' when used separately.
- // The behavior for them with grouped options should be the same.
- if (MaybeValue.empty() || PGOpt->getFormattingFlag() == cl::AlwaysPrefix ||
- (PGOpt->getFormattingFlag() == cl::Prefix && MaybeValue[0] != '=')) {
- Value = MaybeValue;
- return PGOpt;
- }
-
- if (MaybeValue[0] == '=') {
- Value = MaybeValue.substr(1);
- return PGOpt;
- }
-
- // This must be a grouped option.
- assert(isGrouping(PGOpt) && "Broken getOptionPred!");
-
- // Grouping options inside a group can't have values.
- if (PGOpt->getValueExpectedFlag() == cl::ValueRequired) {
- ErrorParsing |= PGOpt->error("may not occur within a group!");
- return nullptr;
- }
-
- // Because the value for the option is not required, we don't need to pass
- // argc/argv in.
- int Dummy = 0;
- ErrorParsing |= ProvideOption(PGOpt, Arg, StringRef(), 0, nullptr, Dummy);
+ StringRef MaybeValue =
+ (Length < Arg.size()) ? Arg.substr(Length) : StringRef();
+ Arg = Arg.substr(0, Length);
- // Get the next grouping option.
- Arg = MaybeValue;
- PGOpt = getOptionPred(Arg, Length, isGrouping, OptionsMap);
- } while (PGOpt);
-
- // We could not find a grouping option in the remainder of Arg.
- return nullptr;
+ // cl::Prefix options do not preserve '=' when used separately.
+ if (POpt->getFormattingFlag() == cl::Prefix && MaybeValue.starts_with("="))
+ MaybeValue = MaybeValue.drop_front();
+ Value = MaybeValue;
+ return POpt;
}
static bool RequiresValue(const Option *O) {
@@ -1718,10 +1660,9 @@ bool CommandLineParser::ParseCommandLineOptions(
Handler = LookupLongOption(SubCommand::getTopLevel(), ArgName, Value,
LongOptionsUseDoubleDash, HaveDoubleDash);
- // Check to see if this "option" is really a prefixed or grouped argument.
+ // Check to see if this "option" is really a prefixed argument.
if (!Handler && !(LongOptionsUseDoubleDash && HaveDoubleDash))
- Handler = HandlePrefixedOrGroupedOption(ArgName, Value, ErrorParsing,
- OptionsMap);
+ Handler = HandlePrefixedOption(ArgName, Value, OptionsMap);
// Otherwise, look for the closest available option to report to the user
// in the upcoming error.
diff --git a/llvm/unittests/Support/CommandLineTest.cpp b/llvm/unittests/Support/CommandLineTest.cpp
index 956d5b97c2703..355b0e7e41a2e 100644
--- a/llvm/unittests/Support/CommandLineTest.cpp
+++ b/llvm/unittests/Support/CommandLineTest.cpp
@@ -1740,197 +1740,22 @@ TEST(CommandLineTest, PrefixOptions) {
EXPECT_EQ(MacroDefs.front().compare("HAVE_FOO"), 0);
}
-TEST(CommandLineTest, GroupingWithValue) {
+TEST(CommandLineTest, NoGrouping) {
cl::ResetCommandLineParser();
- StackOption<bool> OptF("f", cl::Grouping, cl::desc("Some flag"));
- StackOption<bool> OptB("b", cl::Grouping, cl::desc("Another flag"));
- StackOption<bool> OptD("d", cl::Grouping, cl::ValueDisallowed,
- cl::desc("ValueDisallowed option"));
- StackOption<std::string> OptV("v", cl::Grouping,
- cl::desc("ValueRequired option"));
- StackOption<std::string> OptO("o", cl::Grouping, cl::ValueOptional,
- cl::desc("ValueOptional option"));
-
- // Should be possible to use an option which requires a value
- // at the end of a group.
- const char *args1[] = {"prog", "-fv", "val1"};
- EXPECT_TRUE(
- cl::ParseCommandLineOptions(3, args1, StringRef(), &llvm::nulls()));
- EXPECT_TRUE(OptF);
- EXPECT_STREQ("val1", OptV.c_str());
- OptV.clear();
- cl::ResetAllOptionOccurrences();
-
- // Should not crash if it is accidentally used elsewhere in the group.
- const char *args2[] = {"prog", "-vf", "val2"};
- EXPECT_FALSE(
- cl::ParseCommandLineOptions(3, args2, StringRef(), &llvm::nulls()));
- OptV.clear();
- cl::ResetAllOptionOccurrences();
-
- // Should allow the "opt=value" form at the end of the group
- const char *args3[] = {"prog", "-fv=val3"};
- EXPECT_TRUE(
- cl::ParseCommandLineOptions(2, args3, StringRef(), &llvm::nulls()));
- EXPECT_TRUE(OptF);
- EXPECT_STREQ("val3", OptV.c_str());
- OptV.clear();
- cl::ResetAllOptionOccurrences();
-
- // Should allow assigning a value for a ValueOptional option
- // at the end of the group
- const char *args4[] = {"prog", "-fo=val4"};
- EXPECT_TRUE(
- cl::ParseCommandLineOptions(2, args4, StringRef(), &llvm::nulls()));
- EXPECT_TRUE(OptF);
- EXPECT_STREQ("val4", OptO.c_str());
- OptO.clear();
- cl::ResetAllOptionOccurrences();
-
- // Should assign an empty value if a ValueOptional option is used elsewhere
- // in the group.
- const char *args5[] = {"prog", "-fob"};
- EXPECT_TRUE(
- cl::ParseCommandLineOptions(2, args5, StringRef(), &llvm::nulls()));
- EXPECT_TRUE(OptF);
- EXPECT_EQ(1, OptO.getNumOccurrences());
- EXPECT_EQ(1, OptB.getNumOccurrences());
- EXPECT_TRUE(OptO.empty());
- cl::ResetAllOptionOccurrences();
-
- // Should not allow an assignment for a ValueDisallowed option.
- const char *args6[] = {"prog", "-fd=false"};
- EXPECT_FALSE(
- cl::ParseCommandLineOptions(2, args6, StringRef(), &llvm::nulls()));
-}
-
-TEST(CommandLineTest, GroupingAndPrefix) {
- cl::ResetCommandLineParser();
-
- StackOption<bool> OptF("f", cl::Grouping, cl::desc("Some flag"));
- StackOption<bool> OptB("b", cl::Grouping, cl::desc("Another flag"));
- StackOption<std::string> OptP("p", cl::Prefix, cl::Grouping,
- cl::desc("Prefix and Grouping"));
- StackOption<std::string> OptA("a", cl::AlwaysPrefix, cl::Grouping,
- cl::desc("AlwaysPrefix and Grouping"));
-
- // Should be possible to use a cl::Prefix option without grouping.
- const char *args1[] = {"prog", "-pval1"};
- EXPECT_TRUE(
- cl::ParseCommandLineOptions(2, args1, StringRef(), &llvm::nulls()));
- EXPECT_STREQ("val1", OptP.c_str());
- OptP.clear();
- cl::ResetAllOptionOccurrences();
-
- // Should be possible to pass a value in a separate argument.
- const char *args2[] = {"prog", "-p", "val2"};
- EXPECT_TRUE(
- cl::ParseCommandLineOptions(3, args2, StringRef(), &llvm::nulls()));
- EXPECT_STREQ("val2", OptP.c_str());
- OptP.clear();
- cl::ResetAllOptionOccurrences();
-
- // The "-opt=value" form should work, too.
- const char *args3[] = {"prog", "-p=val3"};
- EXPECT_TRUE(
- cl::ParseCommandLineOptions(2, args3, StringRef(), &llvm::nulls()));
- EXPECT_STREQ("val3", OptP.c_str());
- OptP.clear();
- cl::ResetAllOptionOccurrences();
-
- // All three previous cases should work the same way if an option with both
- // cl::Prefix and cl::Grouping modifiers is used at the end of a group.
- const char *args4[] = {"prog", "-fpval4"};
- EXPECT_TRUE(
- cl::ParseCommandLineOptions(2, args4, StringRef(), &llvm::nulls()));
- EXPECT_TRUE(OptF);
- EXPECT_STREQ("val4", OptP.c_str());
- OptP.clear();
- cl::ResetAllOptionOccurrences();
-
- const char *args5[] = {"prog", "-fp", "val5"};
- EXPECT_TRUE(
- cl::ParseCommandLineOptions(3, args5, StringRef(), &llvm::nulls()));
- EXPECT_TRUE(OptF);
- EXPECT_STREQ("val5", OptP.c_str());
- OptP.clear();
- cl::ResetAllOptionOccurrences();
-
- const char *args6[] = {"prog", "-fp=val6"};
- EXPECT_TRUE(
- cl::ParseCommandLineOptions(2, args6, StringRef(), &llvm::nulls()));
- EXPECT_TRUE(OptF);
- EXPECT_STREQ("val6", OptP.c_str());
- OptP.clear();
- cl::ResetAllOptionOccurrences();
-
- // Should assign a value even if the part after a cl::Prefix option is equal
- // to the name of another option.
- const char *args7[] = {"prog", "-fpb"};
- EXPECT_TRUE(
- cl::ParseCommandLineOptions(2, args7, StringRef(), &llvm::nulls()));
- EXPECT_TRUE(OptF);
- EXPECT_STREQ("b", OptP.c_str());
- EXPECT_FALSE(OptB);
- OptP.clear();
- cl::ResetAllOptionOccurrences();
-
- // Should be possible to use a cl::AlwaysPrefix option without grouping.
- const char *args8[] = {"prog", "-aval8"};
- EXPECT_TRUE(
- cl::ParseCommandLineOptions(2, args8, StringRef(), &llvm::nulls()));
- EXPECT_STREQ("val8", OptA.c_str());
- OptA.clear();
- cl::ResetAllOptionOccurrences();
-
- // Should not be possible to pass a value in a separate argument.
- const char *args9[] = {"prog", "-a", "val9"};
- EXPECT_FALSE(
- cl::ParseCommandLineOptions(3, args9, StringRef(), &llvm::nulls()));
- cl::ResetAllOptionOccurrences();
-
- // With the "-opt=value" form, the "=" symbol should be preserved.
- const char *args10[] = {"prog", "-a=val10"};
- EXPECT_TRUE(
- cl::ParseCommandLineOptions(2, args10, StringRef(), &llvm::nulls()));
- EXPECT_STREQ("=val10", OptA.c_str());
- OptA.clear();
- cl::ResetAllOptionOccurrences();
+ StackOption<bool> OptF("f", cl::desc("Some flag"));
+ StackOption<bool> OptB("b", cl::desc("Another flag"));
- // All three previous cases should work the same way if an option with both
- // cl::AlwaysPrefix and cl::Grouping modifiers is used at the end of a group.
- const char *args11[] = {"prog", "-faval11"};
+ const char *args1[] = {"prog", "-f", "-b"};
EXPECT_TRUE(
- cl::ParseCommandLineOptions(2, args11, StringRef(), &llvm::nulls()));
+ cl::ParseCommandLineOptions(3, args1, StringRef(), &llvm::nulls()));
EXPECT_TRUE(OptF);
- EXPECT_STREQ("val11", OptA.c_str());
- OptA.clear();
+ EXPECT_TRUE(OptB);
cl::ResetAllOptionOccurrences();
- const char *args12[] = {"prog", "-fa", "val12"};
+ const char *args2[] = {"prog", "-fb"};
EXPECT_FALSE(
- cl::ParseCommandLineOptions(3, args12, StringRef(), &llvm::nulls()));
- cl::ResetAllOptionOccurrences();
-
- const char *args13[] = {"prog", "-fa=val13"};
- EXPECT_TRUE(
- cl::ParseCommandLineOptions(2, args13, StringRef(), &llvm::nulls()));
- EXPECT_TRUE(OptF);
- EXPECT_STREQ("=val13", OptA.c_str());
- OptA.clear();
- cl::ResetAllOptionOccurrences();
-
- // Should assign a value even if the part after a cl::AlwaysPrefix option
- // is equal to the name of another option.
- const char *args14[] = {"prog", "-fab"};
- EXPECT_TRUE(
- cl::ParseCommandLineOptions(2, args14, StringRef(), &llvm::nulls()));
- EXPECT_TRUE(OptF);
- EXPECT_STREQ("b", OptA.c_str());
- EXPECT_FALSE(OptB);
- OptA.clear();
- cl::ResetAllOptionOccurrences();
+ cl::ParseCommandLineOptions(2, args2, StringRef(), &llvm::nulls()));
}
TEST(CommandLineTest, LongOptions) {
@@ -1970,8 +1795,7 @@ TEST(CommandLineTest, LongOptions) {
EXPECT_TRUE(Errs.empty()); Errs.clear();
cl::ResetAllOptionOccurrences();
- // Fails because `-ab` and `--ab` are treated the same and appear more than
- // once. Also, `val1` is unexpected.
+ // Fails because `val1` is unexpected.
EXPECT_FALSE(
cl::ParseCommandLineOptions(4, args3, StringRef(), &OS));
outs()<< Errs << "\n";
@@ -1983,8 +1807,8 @@ TEST(CommandLineTest, LongOptions) {
// `--` for long options.
//
- // Fails because `-ab` is treated as `-a -b`, so `-a` is seen twice, and
- // `val1` is unexpected.
+ // Fails because `-ab` is neither a short option nor `--ab`, and `val1` is
+ // unexpected.
EXPECT_FALSE(cl::ParseCommandLineOptions(4, args1, StringRef(), &OS, nullptr,
nullptr, true));
EXPECT_FALSE(Errs.empty()); Errs.clear();
@@ -1996,13 +1820,10 @@ TEST(CommandLineTest, LongOptions) {
EXPECT_TRUE(Errs.empty()); Errs.clear();
cl::ResetAllOptionOccurrences();
- // Works because `-ab` is treated as `-a -b`, and `--ab` is a long option.
- EXPECT_TRUE(cl::ParseCommandLineOptions(4, args3, StringRef(), &OS, nullptr,
- nullptr, true));
- EXPECT_TRUE(OptA);
- EXPECT_TRUE(OptBLong);
- EXPECT_STREQ("val1", OptAB.c_str());
- EXPECT_TRUE(Errs.empty()); Errs.clear();
+ // Fails because `-ab` is not `--ab`.
+ EXPECT_FALSE(cl::ParseCommandLineOptions(4, args3, StringRef(), &OS, nullptr,
+ nullptr, true));
+ EXPECT_FALSE(Errs.empty()); Errs.clear();
cl::ResetAllOptionOccurrences();
}
More information about the llvm-commits
mailing list