Skip to content

Commit 875f058

Browse files
committed
[CommandLine] Do not crash if an option has both ValueRequired and Grouping.
If an option, which requires a value, has a `cl::Grouping` formatting modifier, it works well as far as it is used at the end of a group, or as a separate argument. However, if the option appears accidentally in the middle of a group, the program just crashes. This patch prints an error message instead. Differential Revision: https://reviews.llvm.org/D58499 llvm-svn: 355184
1 parent 88c643a commit 875f058

File tree

3 files changed

+31
-5
lines changed

3 files changed

+31
-5
lines changed

llvm/docs/CommandLine.rst

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1172,7 +1172,8 @@ As usual, you can only specify one of these arguments at most.
11721172
``ls``) that have lots of single letter arguments, but only require a single
11731173
dash. For example, the '``ls -labF``' command actually enables four different
11741174
options, all of which are single letters. Note that **cl::Grouping** options
1175-
cannot have values.
1175+
can have values only if they are used separately or at the end of the groups.
1176+
It is a runtime error if such an option is used elsewhere in the group.
11761177

11771178
The CommandLine library does not restrict how you use the **cl::Prefix** or
11781179
**cl::Grouping** modifiers, but it is possible to specify ambiguous argument

llvm/lib/Support/CommandLine.cpp

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -671,10 +671,13 @@ HandlePrefixedOrGroupedOption(StringRef &Arg, StringRef &Value,
671671
StringRef OneArgName = Arg.substr(0, Length);
672672
Arg = Arg.substr(Length);
673673

674-
// Because ValueRequired is an invalid flag for grouped arguments,
675-
// we don't need to pass argc/argv in.
676-
assert(PGOpt->getValueExpectedFlag() != cl::ValueRequired &&
677-
"Option can not be cl::Grouping AND cl::ValueRequired!");
674+
if (PGOpt->getValueExpectedFlag() == cl::ValueRequired) {
675+
ErrorParsing |= PGOpt->error("may not occur within a group!");
676+
return nullptr;
677+
}
678+
679+
// Because the value for the option is not required, we don't need to pass
680+
// argc/argv in.
678681
int Dummy = 0;
679682
ErrorParsing |=
680683
ProvideOption(PGOpt, OneArgName, StringRef(), 0, nullptr, Dummy);

llvm/unittests/Support/CommandLineTest.cpp

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1128,4 +1128,26 @@ TEST(CommandLineTest, PrefixOptions) {
11281128
EXPECT_TRUE(MacroDefs.front().compare("HAVE_FOO") == 0);
11291129
}
11301130

1131+
TEST(CommandLineTest, GroupingWithValue) {
1132+
cl::ResetCommandLineParser();
1133+
1134+
StackOption<bool> OptF("f", cl::Grouping, cl::desc("Some flag"));
1135+
StackOption<std::string> OptV("v", cl::Grouping,
1136+
cl::desc("Grouping option with a value"));
1137+
1138+
// Should be possible to use an option which requires a value
1139+
// at the end of a group.
1140+
const char *args1[] = {"prog", "-fv", "val1"};
1141+
EXPECT_TRUE(
1142+
cl::ParseCommandLineOptions(3, args1, StringRef(), &llvm::nulls()));
1143+
EXPECT_TRUE(OptF);
1144+
EXPECT_STREQ("val1", OptV.c_str());
1145+
cl::ResetAllOptionOccurrences();
1146+
1147+
// Should not crash if it is accidentally used elsewhere in the group.
1148+
const char *args2[] = {"prog", "-vf", "val2"};
1149+
EXPECT_FALSE(
1150+
cl::ParseCommandLineOptions(3, args2, StringRef(), &llvm::nulls()));
1151+
}
1152+
11311153
} // anonymous namespace

0 commit comments

Comments
 (0)