[CommandLine] Don't print empty sentinel values from EnumValN lists in help text

In order to make an option value truly optional, both the ValueOptional
attribute and an empty-named value are required. Prior to this change,
this empty-named value appears in the command-line help text:

-some-option - some help text
  =v1        - description 1
  =v2        - description 2
  =          -

This change improves the help text for these sort of options in a number
of ways:

1) ValueOptional options with an empty-named value now print their help
   text twice: both without and then with '=<value>' after the name. The
   latter version then lists the allowed values after it.
2) Empty-named values with no help text in ValueOptional options are not
   listed in the permitted values.

-some-option         - some help text
-some-option=<value> - some help text
  =v1                - description 1
  =v2                - description 2

3) Otherwise empty-named options are printed as =<empty> rather than
   simply '='.
4) Option values without help text do not have the '-' separator
   printed.

-some-option=<value> - some help text
  =v1                - description 1
  =v2
  =<empty>           - description

It also tweaks the llvm-symbolizer -functions help text to not print a
trailing ':' as that looks bad combined with 1) above.

This is mostly a reland of r352750.

Reviewed by: ruiu, thopre, mstorsjo

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


git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@353048 91177308-0d34-0410-b5e6-96231b3b80d8

1 files changed, 63 insertions, 16 deletions

diff --git a/lib/Support/CommandLine.cpp b/lib/Support/CommandLine.cpp
index f8bc6a8f615..52f1c0e007b 100644
--- a/lib/Support/CommandLine.cpp
+++ b/lib/Support/CommandLine.cpp
@@ -1469,17 +1469,23 @@ static StringRef getValueStr(const Option &O, StringRef DefaultMsg) {
   return O.ValueStr;
 }
 
+static StringRef ArgPrefix = "  -";
+static StringRef ArgHelpPrefix = " - ";
+static size_t ArgPrefixesSize = ArgPrefix.size() + ArgHelpPrefix.size();
+
 //===----------------------------------------------------------------------===//
 // cl::alias class implementation
 //
 
 // Return the width of the option tag for printing...
-size_t alias::getOptionWidth() const { return ArgStr.size() + 6; }
+size_t alias::getOptionWidth() const { return ArgStr.size() + ArgPrefixesSize; }
 
 void Option::printHelpStr(StringRef HelpStr, size_t Indent,
-                                 size_t FirstLineIndentedBy) {
+                          size_t FirstLineIndentedBy) {
+  assert(Indent >= FirstLineIndentedBy);
   std::pair<StringRef, StringRef> Split = HelpStr.split('\n');
-  outs().indent(Indent - FirstLineIndentedBy) << " - " << Split.first << "\n";
+  outs().indent(Indent - FirstLineIndentedBy)
+      << ArgHelpPrefix << Split.first << "\n";
   while (!Split.second.empty()) {
     Split = Split.second.split('\n');
     outs().indent(Indent) << Split.first << "\n";
@@ -1488,8 +1494,8 @@ void Option::printHelpStr(StringRef HelpStr, size_t Indent,
 
 // Print out the option for the alias.
 void alias::printOptionInfo(size_t GlobalWidth) const {
-  outs() << "  -" << ArgStr;
-  printHelpStr(HelpStr, GlobalWidth, ArgStr.size() + 6);
+  outs() << ArgPrefix << ArgStr;
+  printHelpStr(HelpStr, GlobalWidth, ArgStr.size() + ArgPrefixesSize);
 }
 
 //===----------------------------------------------------------------------===//
@@ -1510,7 +1516,7 @@ size_t basic_parser_impl::getOptionWidth(const Option &O) const {
     Len += getValueStr(O, ValName).size() + FormattingLen;
   }
 
-  return Len + 6;
+  return Len + ArgPrefixesSize;
 }
 
 // printOptionInfo - Print out information about this option.  The
@@ -1518,7 +1524,7 @@ size_t basic_parser_impl::getOptionWidth(const Option &O) const {
 //
 void basic_parser_impl::printOptionInfo(const Option &O,
                                         size_t GlobalWidth) const {
-  outs() << "  -" << O.ArgStr;
+  outs() << ArgPrefix << O.ArgStr;
 
   auto ValName = getValueName();
   if (!ValName.empty()) {
@@ -1534,7 +1540,7 @@ void basic_parser_impl::printOptionInfo(const Option &O,
 
 void basic_parser_impl::printOptionName(const Option &O,
                                         size_t GlobalWidth) const {
-  outs() << "  -" << O.ArgStr;
+  outs() << ArgPrefix << O.ArgStr;
   outs().indent(GlobalWidth - O.ArgStr.size());
 }
 
@@ -1642,12 +1648,28 @@ unsigned generic_parser_base::findOption(StringRef Name) {
   return e;
 }
 
+static StringRef EqValue = "=<value>";
+static StringRef EmptyOption = "<empty>";
+static StringRef OptionPrefix = "    =";
+static size_t OptionPrefixesSize = OptionPrefix.size() + ArgHelpPrefix.size();
+
+static bool shouldPrintOption(StringRef Name, StringRef Description,
+                              const Option &O) {
+  return O.getValueExpectedFlag() != ValueOptional || !Name.empty() ||
+         !Description.empty();
+}
+
 // Return the width of the option tag for printing...
 size_t generic_parser_base::getOptionWidth(const Option &O) const {
   if (O.hasArgStr()) {
-    size_t Size = O.ArgStr.size() + 6;
-    for (unsigned i = 0, e = getNumOptions(); i != e; ++i)
-      Size = std::max(Size, getOption(i).size() + 8);
+    size_t Size = O.ArgStr.size() + ArgPrefixesSize + EqValue.size();
+    for (unsigned i = 0, e = getNumOptions(); i != e; ++i) {
+      StringRef Name = getOption(i);
+      if (!shouldPrintOption(Name, getDescription(i), O))
+        continue;
+      size_t NameSize = Name.empty() ? EmptyOption.size() : Name.size();
+      Size = std::max(Size, NameSize + OptionPrefixesSize);
+    }
     return Size;
   } else {
     size_t BaseSize = 0;
@@ -1663,13 +1685,38 @@ size_t generic_parser_base::getOptionWidth(const Option &O) const {
 void generic_parser_base::printOptionInfo(const Option &O,
                                           size_t GlobalWidth) const {
   if (O.hasArgStr()) {
-    outs() << "  -" << O.ArgStr;
-    Option::printHelpStr(O.HelpStr, GlobalWidth, O.ArgStr.size() + 6);
+    // When the value is optional, first print a line just describing the
+    // option without values.
+    if (O.getValueExpectedFlag() == ValueOptional) {
+      for (unsigned i = 0, e = getNumOptions(); i != e; ++i) {
+        if (getOption(i).empty()) {
+          outs() << ArgPrefix << O.ArgStr;
+          Option::printHelpStr(O.HelpStr, GlobalWidth,
+                               O.ArgStr.size() + ArgPrefixesSize);
+          break;
+        }
+      }
+    }
 
+    outs() << ArgPrefix << O.ArgStr << EqValue;
+    Option::printHelpStr(O.HelpStr, GlobalWidth,
+                         O.ArgStr.size() + EqValue.size() + ArgPrefixesSize);
     for (unsigned i = 0, e = getNumOptions(); i != e; ++i) {
-      size_t NumSpaces = GlobalWidth - getOption(i).size() - 8;
-      outs() << "    =" << getOption(i);
-      outs().indent(NumSpaces) << " -   " << getDescription(i) << '\n';
+      StringRef OptionName = getOption(i);
+      StringRef Description = getDescription(i);
+      if (!shouldPrintOption(OptionName, Description, O))
+        continue;
+      assert(GlobalWidth >= OptionName.size() + OptionPrefixesSize);
+      size_t NumSpaces = GlobalWidth - OptionName.size() - OptionPrefixesSize;
+      outs() << OptionPrefix << OptionName;
+      if (OptionName.empty()) {
+        outs() << EmptyOption;
+        assert(NumSpaces >= EmptyOption.size());
+        NumSpaces -= EmptyOption.size();
+      }
+      if (!Description.empty())
+        outs().indent(NumSpaces) << ArgHelpPrefix << "  " << Description;
+      outs() << '\n';
     }
   } else {
     if (!O.HelpStr.empty())