Skip to content

Commit

Permalink
[Option] Support special argument "--"
Browse files Browse the repository at this point in the history
Many command line option implementations, including getopt_long and our
Support/CommandLine.cpp, support `--` as an end-of-option indicator. All
the subsequent arguments are then treated as positional arguments.

D1387 added KIND_REMAINING_ARGS and 76ff1d9 dropped special handling of `--`.
Users need to add `def DASH_DASH : Option<["--"], "", KIND_REMAINING_ARGS>;` and
append `OPT_DASH_DASH` to the `OPT_INPUT` list., which is not ergonomic.

Restore this feature under an option and modify llvm-strings to utilize the
feature as an example. In the future, we probably should enable this feature by
default and exclude some tools that handle `DASH_DASH` differently (clang,
clang-scan-deps, etc. I suspect that many are workarounds for LLVMOption not
supporting `--` as a built-in feature).

Reviewed By: serge-sans-paille

Differential Revision: https://reviews.llvm.org/D152286
  • Loading branch information
MaskRay committed Jun 7, 2023
1 parent 66a562d commit 926e51c
Show file tree
Hide file tree
Showing 6 changed files with 55 additions and 2 deletions.
5 changes: 5 additions & 0 deletions llvm/include/llvm/Option/OptTable.h
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ class OptTable {
ArrayRef<Info> OptionInfos;
bool IgnoreCase;
bool GroupedShortOptions = false;
bool DashDashParsing = false;
const char *EnvVar = nullptr;

unsigned InputOptionID = 0;
Expand Down Expand Up @@ -139,6 +140,10 @@ class OptTable {
/// Support grouped short options. e.g. -ab represents -a -b.
void setGroupedShortOptions(bool Value) { GroupedShortOptions = Value; }

/// Set whether "--" stops option parsing and treats all subsequent arguments
/// as positional. E.g. -- -a -b gives two positional inputs.
void setDashDashParsing(bool Value) { DashDashParsing = Value; }

/// Find possible value for given flags. This is used for shell
/// autocompletion.
///
Expand Down
10 changes: 10 additions & 0 deletions llvm/lib/Option/OptTable.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -468,6 +468,16 @@ InputArgList OptTable::ParseArgs(ArrayRef<const char *> ArgArr,
continue;
}

// In DashDashParsing mode, the first "--" stops option scanning and treats
// all subsequent arguments as positional.
if (DashDashParsing && Str == "--") {
while (++Index < End) {
Args.append(new Arg(getOption(InputOptionID), Str, Index,
Args.getArgString(Index)));
}
break;
}

unsigned Prev = Index;
std::unique_ptr<Arg> A = GroupedShortOptions
? parseOneArgGrouped(Args, Index)
Expand Down
6 changes: 6 additions & 0 deletions llvm/test/tools/llvm-strings/dash-filename.test
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
## Show that -- stops option scanning.

RUN: rm -rf %t && mkdir %t && cd %t
RUN: echo abcd > -a
RUN: llvm-strings -f -- -a | FileCheck %s
CHECK: -a: abcd
1 change: 1 addition & 0 deletions llvm/tools/llvm-strings/llvm-strings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ class StringsOptTable : public opt::GenericOptTable {
public:
StringsOptTable() : GenericOptTable(InfoTable) {
setGroupedShortOptions(true);
setDashDashParsing(true);
}
};
} // namespace
Expand Down
33 changes: 33 additions & 0 deletions llvm/unittests/Option/OptionParsingTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -391,6 +391,39 @@ TYPED_TEST(OptTableTest, ParseGroupedShortOptions) {
EXPECT_TRUE(AL3.hasArg(OPT_Blorp));
}

TYPED_TEST(OptTableTest, ParseDashDash) {
TypeParam T;
T.setDashDashParsing(true);
unsigned MAI, MAC;

const char *Args1[] = {"-A", "--"};
InputArgList AL = T.ParseArgs(Args1, MAI, MAC);
EXPECT_TRUE(AL.hasArg(OPT_A));
EXPECT_EQ(size_t(0), AL.getAllArgValues(OPT_INPUT).size());
EXPECT_EQ(size_t(0), AL.getAllArgValues(OPT_UNKNOWN).size());

const char *Args2[] = {"-A", "--", "-A", "--", "-B"};
AL = T.ParseArgs(Args2, MAI, MAC);
EXPECT_TRUE(AL.hasArg(OPT_A));
EXPECT_FALSE(AL.hasArg(OPT_B));
const std::vector<std::string> Input = AL.getAllArgValues(OPT_INPUT);
ASSERT_EQ(size_t(3), Input.size());
EXPECT_EQ("-A", Input[0]);
EXPECT_EQ("--", Input[1]);
EXPECT_EQ("-B", Input[2]);
EXPECT_EQ(size_t(0), AL.getAllArgValues(OPT_UNKNOWN).size());

T.setDashDashParsing(false);
AL = T.ParseArgs(Args2, MAI, MAC);
EXPECT_TRUE(AL.hasArg(OPT_A));
EXPECT_TRUE(AL.hasArg(OPT_B));
EXPECT_EQ(size_t(0), AL.getAllArgValues(OPT_INPUT).size());
const std::vector<std::string> Unknown = AL.getAllArgValues(OPT_UNKNOWN);
ASSERT_EQ(size_t(2), Unknown.size());
EXPECT_EQ("--", Unknown[0]);
EXPECT_EQ("--", Unknown[1]);
}

TYPED_TEST(OptTableTest, UnknownOptions) {
TypeParam T;
unsigned MAI, MAC;
Expand Down
2 changes: 0 additions & 2 deletions llvm/unittests/Option/Opts.td
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,6 @@ def Glorrmp_eq : Flag<["--"], "glorrmp=">;
def Blurmpq : Flag<["--"], "blurmp">;
def Blurmpq_eq : Flag<["--"], "blurmp=">;

def DashDash : Option<["--"], "", KIND_REMAINING_ARGS>;

class XOpts<string base> : KeyPathAndMacro<"X->", base> {}

def marshalled_flag_d : Flag<["-"], "marshalled-flag-d">,
Expand Down

0 comments on commit 926e51c

Please sign in to comment.