[PATCH] D46787: [CommandLine] Error message for incorrect PositionalEatArgs usage

[Keno Fischer via Phabricator via llvm-commits]

loladiro created this revision.
loladiro added a reviewer: aprantl.

bugpoint has several options specified as `PositionalEatArgs` to pass
options through to the underlying tool, e.g. `-tool-args`. The `-help`
message suggests the usage is: `-tool-args=<string>`. However, this is
misleading, because that's not how these arguments work. Rather than taking
a value, the option consumes all positional arguments until the next
recognized option (or all arguments if `--` is specified at some point).
To make this slightly clearer, instead print the help as:

  -tool-args <string>...                            - <tool arguments>...

Additionally, add an error if the user attempts to use a `PositionalEatArgs`
argument with a value, instead of silently ignoring it. Example:

  ./bin/bugpoint -tool-args=-mpcu=skylake-avx512
  bugpoint: for the -tool-args option: This argument does not take a value.
      Instead, it consumes any positional arguments until the next recognized option.


Repository:
  rL LLVM

https://reviews.llvm.org/D46787

Files:
  lib/Support/CommandLine.cpp


Index: lib/Support/CommandLine.cpp
===================================================================
--- lib/Support/CommandLine.cpp
+++ lib/Support/CommandLine.cpp
@@ -1270,8 +1270,15 @@
 
     // If this is a named positional argument, just remember that it is the
     // active one...
-    if (Handler->getFormattingFlag() == cl::Positional)
+    if (Handler->getFormattingFlag() == cl::Positional) {
+      if ((Handler->getMiscFlags() & PositionalEatsArgs) && !Value.empty()) {
+        Handler->error("This argument does not take a value.\n"
+                       "\tInstead, it consumes any positional arguments until "
+                       "the next recognized option.");
+        ErrorParsing = true;
+      }
       ActivePositionalArg = Handler;
+    }
     else
       ErrorParsing |= ProvideOption(Handler, ArgName, Value, argc, argv, i);
   }
@@ -1474,8 +1481,12 @@
 size_t basic_parser_impl::getOptionWidth(const Option &O) const {
   size_t Len = O.ArgStr.size();
   auto ValName = getValueName();
-  if (!ValName.empty())
-    Len += getValueStr(O, ValName).size() + 3;
+  if (!ValName.empty()) {
+    size_t FormattingLen = 3;
+    if (O.getMiscFlags() & PositionalEatsArgs)
+      FormattingLen = 6;
+    Len += getValueStr(O, ValName).size() + FormattingLen;
+  }
 
   return Len + 6;
 }
@@ -1488,8 +1499,13 @@
   outs() << "  -" << O.ArgStr;
 
   auto ValName = getValueName();
-  if (!ValName.empty())
-    outs() << "=<" << getValueStr(O, ValName) << '>';
+  if (!ValName.empty()) {
+    if (O.getMiscFlags() & PositionalEatsArgs) {
+      outs() << " <" << getValueStr(O, ValName) << ">...";
+    } else {
+      outs() << "=<" << getValueStr(O, ValName) << '>';
+    }
+  }
 
   Option::printHelpStr(O.HelpStr, GlobalWidth, getOptionWidth(O));
 }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D46787.146445.patch
Type: text/x-patch
Size: 1780 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180512/8aa112d0/attachment.bin>