From 8248d7c6616fd5b612d991adcff59ed288eaef5e Mon Sep 17 00:00:00 2001
From: Keno Fischer <keno@alumni.harvard.edu>
Date: Mon, 14 May 2018 23:26:06 +0000
Subject: [CommandLine] Error message for incorrect PositionalEatArgs usage

Summary:
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.
```

Reviewed By: aprantl
Differential Revision: https://reviews.llvm.org/D46787

llvm-svn: 332311
---
 llvm/lib/Support/CommandLine.cpp | 34 +++++++++++++++++++++++++---------
 1 file changed, 25 insertions(+), 9 deletions(-)

(limited to 'llvm/lib/Support/CommandLine.cpp')

diff --git a/llvm/lib/Support/CommandLine.cpp b/llvm/lib/Support/CommandLine.cpp
index f1be43c027b..a1e659a01c8 100644
--- a/llvm/lib/Support/CommandLine.cpp
+++ b/llvm/lib/Support/CommandLine.cpp
@@ -1270,8 +1270,15 @@ bool CommandLineParser::ParseCommandLineOptions(int argc,
 
     // 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.", *Errs);
+        ErrorParsing = true;
+      }
       ActivePositionalArg = Handler;
+    }
     else
       ErrorParsing |= ProvideOption(Handler, ArgName, Value, argc, argv, i);
   }
@@ -1396,15 +1403,15 @@ bool CommandLineParser::ParseCommandLineOptions(int argc,
 // Option Base class implementation
 //
 
-bool Option::error(const Twine &Message, StringRef ArgName) {
+bool Option::error(const Twine &Message, StringRef ArgName, raw_ostream &Errs) {
   if (!ArgName.data())
     ArgName = ArgStr;
   if (ArgName.empty())
-    errs() << HelpStr; // Be nice for positional arguments
+    Errs << HelpStr; // Be nice for positional arguments
   else
-    errs() << GlobalParser->ProgramName << ": for the -" << ArgName;
+    Errs << GlobalParser->ProgramName << ": for the -" << ArgName;
 
-  errs() << " option: " << Message << "\n";
+  Errs << " option: " << Message << "\n";
   return true;
 }
 
@@ -1474,8 +1481,12 @@ void alias::printOptionInfo(size_t GlobalWidth) const {
 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 @@ void basic_parser_impl::printOptionInfo(const Option &O,
   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));
 }
-- 
cgit v1.2.3