[llvm] r355184 - [CommandLine] Do not crash if an option has both ValueRequired and Grouping.

[Igor Kudrin via llvm-commits]

Author: ikudrin
Date: Fri Mar  1 01:20:56 2019
New Revision: 355184

URL: http://llvm.org/viewvc/llvm-project?rev=355184&view=rev
Log:
[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

Modified:
    llvm/trunk/docs/CommandLine.rst
    llvm/trunk/lib/Support/CommandLine.cpp
    llvm/trunk/unittests/Support/CommandLineTest.cpp

Modified: llvm/trunk/docs/CommandLine.rst
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/docs/CommandLine.rst?rev=355184&r1=355183&r2=355184&view=diff
==============================================================================
--- llvm/trunk/docs/CommandLine.rst (original)
+++ llvm/trunk/docs/CommandLine.rst Fri Mar  1 01:20:56 2019
@@ -1172,7 +1172,8 @@ As usual, you can only specify one of th
   ``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
-  cannot have values.
+  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.
 
 The CommandLine library does not restrict how you use the **cl::Prefix** or
 **cl::Grouping** modifiers, but it is possible to specify ambiguous argument

Modified: llvm/trunk/lib/Support/CommandLine.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/CommandLine.cpp?rev=355184&r1=355183&r2=355184&view=diff
==============================================================================
--- llvm/trunk/lib/Support/CommandLine.cpp (original)
+++ llvm/trunk/lib/Support/CommandLine.cpp Fri Mar  1 01:20:56 2019
@@ -671,10 +671,13 @@ HandlePrefixedOrGroupedOption(StringRef
     StringRef OneArgName = Arg.substr(0, Length);
     Arg = Arg.substr(Length);
 
-    // Because ValueRequired is an invalid flag for grouped arguments,
-    // we don't need to pass argc/argv in.
-    assert(PGOpt->getValueExpectedFlag() != cl::ValueRequired &&
-           "Option can not be cl::Grouping AND cl::ValueRequired!");
+    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, OneArgName, StringRef(), 0, nullptr, Dummy);

Modified: llvm/trunk/unittests/Support/CommandLineTest.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Support/CommandLineTest.cpp?rev=355184&r1=355183&r2=355184&view=diff
==============================================================================
--- llvm/trunk/unittests/Support/CommandLineTest.cpp (original)
+++ llvm/trunk/unittests/Support/CommandLineTest.cpp Fri Mar  1 01:20:56 2019
@@ -1128,4 +1128,26 @@ TEST(CommandLineTest, PrefixOptions) {
   EXPECT_TRUE(MacroDefs.front().compare("HAVE_FOO") == 0);
 }
 
+TEST(CommandLineTest, GroupingWithValue) {
+  cl::ResetCommandLineParser();
+
+  StackOption<bool> OptF("f", cl::Grouping, cl::desc("Some flag"));
+  StackOption<std::string> OptV("v", cl::Grouping,
+                                cl::desc("Grouping option with a value"));
+
+  // 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());
+  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()));
+}
+
 }  // anonymous namespace