Skip to content

Commit

Permalink
[CommandLine] Allow grouping options which can have values.
Browse files Browse the repository at this point in the history 
This patch allows all forms of values for options to be used at the end
of a group. With the fix, it is possible to follow the way GNU binutils
tools handle grouping options better. For example, the -j option can be
used with objdump in any of the following ways:

$ objdump -d -j .text a.o
$ objdump -d -j.text a.o
$ objdump -dj .text a.o
$ objdump -dj.text a.o

Differential Revision: https://reviews.llvm.org/D58711


git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@355185 91177308-0d34-0410-b5e6-96231b3b80d8
  • Loading branch information
@igorkudrin
igorkudrin committed Mar 1, 2019
1 parent 54df8c8 commit b58513a
Show file tree
Hide file tree
Showing 5 changed files with 236 additions and 52 deletions.
49 changes: 29 additions & 20 deletions docs/CommandLine.rst
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1168,12 +1168,18 @@ As usual, you can only specify one of these arguments at most.
.. _grouping:
.. _cl::Grouping:

* The **cl::Grouping** modifier 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.
It is a runtime error if such an option is used elsewhere in the group.
Controlling options grouping
^^^^^^^^^^^^^^^^^^^^^^^^^^^^

The **cl::Grouping** modifier can be combined with any formatting types except
for `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 `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
Expand All @@ -1188,19 +1194,24 @@ basically looks like this:

parse(string OrigInput) {

1. string input = OrigInput;
2. if (isOption(input)) return getOption(input).parse(); // Normal option
3. while (!isOption(input) && !input.empty()) input.pop_back(); // Remove the last letter
4. if (input.empty()) return error(); // No matching option
5. if (getOption(input).isPrefix())
return getOption(input).parse(input);
6. while (!input.empty()) { // Must be grouping options
getOption(input).parse();
OrigInput.erase(OrigInput.begin(), OrigInput.begin()+input.length());
input = OrigInput;
while (!isOption(input) && !input.empty()) input.pop_back();
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()
}
7. if (!OrigInput.empty()) error();
5. if (!OrigInput.empty()) error();

}

Expand Down Expand Up @@ -1240,8 +1251,6 @@ specify boolean properties that modify the option.
with ``cl::CommaSeparated``, this modifier only makes sense with a `cl::list`_
option.

So far, these are the only three miscellaneous option modifiers.

.. _response files:

Response files
Expand Down
19 changes: 10 additions & 9 deletions include/llvm/Support/CommandLine.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -158,23 +158,24 @@ enum OptionHidden { // Control whether -help shows this option
// AlwaysPrefix - Only allow the behavior enabled by the Prefix flag and reject
// the Option=Value form.
//
// Grouping - With this option 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
//

enum FormattingFlags {
NormalFormatting = 0x00, // Nothing special
Positional = 0x01, // Is a positional argument, no '-' required
Prefix = 0x02, // Can this option directly prefix its value?
AlwaysPrefix = 0x03, // Can this option only directly prefix its value?
Grouping = 0x04 // Can this option group with other options?
AlwaysPrefix = 0x03 // Can this option only directly prefix its value?
};

enum MiscFlags { // Miscellaneous flags to adjust argument
CommaSeparated = 0x01, // Should this cl::list split between commas?
PositionalEatsArgs = 0x02, // Should this positional cl::list eat -args?
Sink = 0x04 // Should this cl::list eat all unknown options?
Sink = 0x04, // Should this cl::list eat all unknown options?

// Grouping - 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
};

//===----------------------------------------------------------------------===//
Expand Down Expand Up @@ -268,8 +269,8 @@ class Option {
// detail representing the non-value
unsigned Value : 2;
unsigned HiddenFlag : 2; // enum OptionHidden
unsigned Formatting : 3; // enum FormattingFlags
unsigned Misc : 3;
unsigned Formatting : 2; // enum FormattingFlags
unsigned Misc : 4;
unsigned Position = 0; // Position of last occurrence of the option
unsigned AdditionalVals = 0; // Greater than 0 for multi-valued option.

Expand Down
45 changes: 24 additions & 21 deletions lib/Support/CommandLine.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -600,7 +600,7 @@ static bool ProvidePositionalOption(Option *Handler, StringRef Arg, int i) {

// Option predicates...
static inline bool isGrouping(const Option *O) {
return O->getFormattingFlag() == cl::Grouping;
return O->getMiscFlags() & cl::Grouping;
}
static inline bool isPrefixedOrGrouping(const Option *O) {
return isGrouping(O) || O->getFormattingFlag() == cl::Prefix ||
Expand Down Expand Up @@ -651,26 +651,29 @@ HandlePrefixedOrGroupedOption(StringRef &Arg, StringRef &Value,
if (!PGOpt)
return nullptr;

// If the option is a prefixed option, then the value is simply the
// rest of the name... so fall through to later processing, by
// setting up the argument name flags and value fields.
if (PGOpt->getFormattingFlag() == cl::Prefix ||
PGOpt->getFormattingFlag() == cl::AlwaysPrefix) {
Value = Arg.substr(Length);
do {
StringRef MaybeValue =
(Length < Arg.size()) ? Arg.substr(Length) : StringRef();
Arg = Arg.substr(0, Length);
assert(OptionsMap.count(Arg) && OptionsMap.find(Arg)->second == PGOpt);
return PGOpt;
}

// This must be a grouped option... handle them now. Grouping options can't
// have values.
assert(isGrouping(PGOpt) && "Broken getOptionPred!");
// 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;
}

do {
// Move current arg name out of Arg into OneArgName.
StringRef OneArgName = Arg.substr(0, Length);
Arg = Arg.substr(Length);
// 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;
Expand All @@ -679,15 +682,15 @@ HandlePrefixedOrGroupedOption(StringRef &Arg, StringRef &Value,
// Because the value for the option is not required, we don't need to pass
// argc/argv in.
int Dummy = 0;
ErrorParsing |=
ProvideOption(PGOpt, OneArgName, StringRef(), 0, nullptr, Dummy);
ErrorParsing |= ProvideOption(PGOpt, Arg, StringRef(), 0, nullptr, Dummy);

// Get the next grouping option.
Arg = MaybeValue;
PGOpt = getOptionPred(Arg, Length, isGrouping, OptionsMap);
} while (PGOpt && Length != Arg.size());
} while (PGOpt);

// Return the last option with Arg cut down to just the last one.
return PGOpt;
// We could not find a grouping option in the remainder of Arg.
return nullptr;
}

static bool RequiresValue(const Option *O) {
Expand Down
2 changes: 1 addition & 1 deletion tools/llvm-readobj/llvm-readobj.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -688,7 +688,7 @@ static void registerReadelfAliases() {
StringRef ArgName = OptEntry.getKey();
cl::Option *Option = OptEntry.getValue();
if (ArgName.size() == 1)
Option->setFormattingFlag(cl::Grouping);
apply(Option, cl::Grouping);
}
}

Expand Down
173 changes: 172 additions & 1 deletion unittests/Support/CommandLineTest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1132,8 +1132,13 @@ TEST(CommandLineTest, GroupingWithValue) {
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("Grouping option with a value"));
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.
Expand All @@ -1142,12 +1147,178 @@ TEST(CommandLineTest, GroupingWithValue) {
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();

// 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"};
EXPECT_TRUE(
cl::ParseCommandLineOptions(2, args11, StringRef(), &llvm::nulls()));
EXPECT_TRUE(OptF);
EXPECT_STREQ("val11", OptA.c_str());
OptA.clear();
cl::ResetAllOptionOccurrences();

const char *args12[] = {"prog", "-fa", "val12"};
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();
}

} // anonymous namespace

0 comments on commit b58513a

Please sign in to comment.