From b58513a2a09596e5729a1b48183c375b27fe8386 Mon Sep 17 00:00:00 2001 From: Igor Kudrin Date: Fri, 1 Mar 2019 09:22:42 +0000 Subject: [PATCH] [CommandLine] Allow grouping options which can have values. 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 --- docs/CommandLine.rst | 49 +++++--- include/llvm/Support/CommandLine.h | 19 +-- lib/Support/CommandLine.cpp | 45 +++---- tools/llvm-readobj/llvm-readobj.cpp | 2 +- unittests/Support/CommandLineTest.cpp | 173 +++++++++++++++++++++++++- 5 files changed, 236 insertions(+), 52 deletions(-) diff --git a/docs/CommandLine.rst b/docs/CommandLine.rst index 8f3207ef08321..46ba6d32bb325 100644 --- a/docs/CommandLine.rst +++ b/docs/CommandLine.rst @@ -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 @@ -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(); } @@ -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 diff --git a/include/llvm/Support/CommandLine.h b/include/llvm/Support/CommandLine.h index a78020f010fa0..cb85534ad8eb2 100644 --- a/include/llvm/Support/CommandLine.h +++ b/include/llvm/Support/CommandLine.h @@ -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 }; //===----------------------------------------------------------------------===// @@ -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. diff --git a/lib/Support/CommandLine.cpp b/lib/Support/CommandLine.cpp index 8b279baf8d606..39991a8905da6 100644 --- a/lib/Support/CommandLine.cpp +++ b/lib/Support/CommandLine.cpp @@ -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 || @@ -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; @@ -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) { diff --git a/tools/llvm-readobj/llvm-readobj.cpp b/tools/llvm-readobj/llvm-readobj.cpp index 14f8daea66a87..161a290a51934 100644 --- a/tools/llvm-readobj/llvm-readobj.cpp +++ b/tools/llvm-readobj/llvm-readobj.cpp @@ -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); } } diff --git a/unittests/Support/CommandLineTest.cpp b/unittests/Support/CommandLineTest.cpp index 4839a31f0c139..4bfa8a35a16cd 100644 --- a/unittests/Support/CommandLineTest.cpp +++ b/unittests/Support/CommandLineTest.cpp @@ -1132,8 +1132,13 @@ TEST(CommandLineTest, GroupingWithValue) { cl::ResetCommandLineParser(); StackOption OptF("f", cl::Grouping, cl::desc("Some flag")); + StackOption OptB("b", cl::Grouping, cl::desc("Another flag")); + StackOption OptD("d", cl::Grouping, cl::ValueDisallowed, + cl::desc("ValueDisallowed option")); StackOption OptV("v", cl::Grouping, - cl::desc("Grouping option with a value")); + cl::desc("ValueRequired option")); + StackOption 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. @@ -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 OptF("f", cl::Grouping, cl::desc("Some flag")); + StackOption OptB("b", cl::Grouping, cl::desc("Another flag")); + StackOption OptP("p", cl::Prefix, cl::Grouping, + cl::desc("Prefix and Grouping")); + StackOption 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