[llvm] r353050 - Revert r353048.

James Henderson via llvm-commits llvm-commits at lists.llvm.org
Mon Feb 4 07:09:58 PST 2019


Author: jhenderson
Date: Mon Feb  4 07:09:58 2019
New Revision: 353050

URL: http://llvm.org/viewvc/llvm-project?rev=353050&view=rev
Log:
Revert r353048.

It was causing unexpected unit test failures on build bots.

Modified:
    llvm/trunk/lib/Support/CommandLine.cpp
    llvm/trunk/tools/llvm-symbolizer/llvm-symbolizer.cpp
    llvm/trunk/unittests/Support/CommandLineTest.cpp

Modified: llvm/trunk/lib/Support/CommandLine.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/CommandLine.cpp?rev=353050&r1=353049&r2=353050&view=diff
==============================================================================
--- llvm/trunk/lib/Support/CommandLine.cpp (original)
+++ llvm/trunk/lib/Support/CommandLine.cpp Mon Feb  4 07:09:58 2019
@@ -1469,23 +1469,17 @@ static StringRef getValueStr(const Optio
   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() + ArgPrefixesSize; }
+size_t alias::getOptionWidth() const { return ArgStr.size() + 6; }
 
 void Option::printHelpStr(StringRef HelpStr, size_t Indent,
-                          size_t FirstLineIndentedBy) {
-  assert(Indent >= FirstLineIndentedBy);
+                                 size_t FirstLineIndentedBy) {
   std::pair<StringRef, StringRef> Split = HelpStr.split('\n');
-  outs().indent(Indent - FirstLineIndentedBy)
-      << ArgHelpPrefix << Split.first << "\n";
+  outs().indent(Indent - FirstLineIndentedBy) << " - " << Split.first << "\n";
   while (!Split.second.empty()) {
     Split = Split.second.split('\n');
     outs().indent(Indent) << Split.first << "\n";
@@ -1494,8 +1488,8 @@ void Option::printHelpStr(StringRef Help
 
 // Print out the option for the alias.
 void alias::printOptionInfo(size_t GlobalWidth) const {
-  outs() << ArgPrefix << ArgStr;
-  printHelpStr(HelpStr, GlobalWidth, ArgStr.size() + ArgPrefixesSize);
+  outs() << "  -" << ArgStr;
+  printHelpStr(HelpStr, GlobalWidth, ArgStr.size() + 6);
 }
 
 //===----------------------------------------------------------------------===//
@@ -1516,7 +1510,7 @@ size_t basic_parser_impl::getOptionWidth
     Len += getValueStr(O, ValName).size() + FormattingLen;
   }
 
-  return Len + ArgPrefixesSize;
+  return Len + 6;
 }
 
 // printOptionInfo - Print out information about this option.  The
@@ -1524,7 +1518,7 @@ size_t basic_parser_impl::getOptionWidth
 //
 void basic_parser_impl::printOptionInfo(const Option &O,
                                         size_t GlobalWidth) const {
-  outs() << ArgPrefix << O.ArgStr;
+  outs() << "  -" << O.ArgStr;
 
   auto ValName = getValueName();
   if (!ValName.empty()) {
@@ -1540,7 +1534,7 @@ void basic_parser_impl::printOptionInfo(
 
 void basic_parser_impl::printOptionName(const Option &O,
                                         size_t GlobalWidth) const {
-  outs() << ArgPrefix << O.ArgStr;
+  outs() << "  -" << O.ArgStr;
   outs().indent(GlobalWidth - O.ArgStr.size());
 }
 
@@ -1648,28 +1642,12 @@ unsigned generic_parser_base::findOption
   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() + 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);
-    }
+    size_t Size = O.ArgStr.size() + 6;
+    for (unsigned i = 0, e = getNumOptions(); i != e; ++i)
+      Size = std::max(Size, getOption(i).size() + 8);
     return Size;
   } else {
     size_t BaseSize = 0;
@@ -1685,38 +1663,13 @@ size_t generic_parser_base::getOptionWid
 void generic_parser_base::printOptionInfo(const Option &O,
                                           size_t GlobalWidth) const {
   if (O.hasArgStr()) {
-    // 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() << "  -" << O.ArgStr;
+    Option::printHelpStr(O.HelpStr, GlobalWidth, O.ArgStr.size() + 6);
 
-    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) {
-      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';
+      size_t NumSpaces = GlobalWidth - getOption(i).size() - 8;
+      outs() << "    =" << getOption(i);
+      outs().indent(NumSpaces) << " -   " << getDescription(i) << '\n';
     }
   } else {
     if (!O.HelpStr.empty())

Modified: llvm/trunk/tools/llvm-symbolizer/llvm-symbolizer.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-symbolizer/llvm-symbolizer.cpp?rev=353050&r1=353049&r2=353050&view=diff
==============================================================================
--- llvm/trunk/tools/llvm-symbolizer/llvm-symbolizer.cpp (original)
+++ llvm/trunk/tools/llvm-symbolizer/llvm-symbolizer.cpp Mon Feb  4 07:09:58 2019
@@ -38,7 +38,7 @@ ClUseSymbolTable("use-symbol-table", cl:
 
 static cl::opt<FunctionNameKind> ClPrintFunctions(
     "functions", cl::init(FunctionNameKind::LinkageName),
-    cl::desc("Print function name for a given address"), cl::ValueOptional,
+    cl::desc("Print function name for a given address:"), cl::ValueOptional,
     cl::values(clEnumValN(FunctionNameKind::None, "none", "omit function name"),
                clEnumValN(FunctionNameKind::ShortName, "short",
                           "print short function name"),

Modified: llvm/trunk/unittests/Support/CommandLineTest.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Support/CommandLineTest.cpp?rev=353050&r1=353049&r2=353050&view=diff
==============================================================================
--- llvm/trunk/unittests/Support/CommandLineTest.cpp (original)
+++ llvm/trunk/unittests/Support/CommandLineTest.cpp Mon Feb  4 07:09:58 2019
@@ -13,7 +13,6 @@
 #include "llvm/Config/config.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/InitLLVM.h"
-#include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Program.h"
 #include "llvm/Support/StringSaver.h"
@@ -840,218 +839,4 @@ TEST(CommandLineTest, GetCommandLineArgu
 }
 #endif
 
-class OutputRedirector {
-public:
-  OutputRedirector(int RedirectFD)
-      : RedirectFD(RedirectFD), OldFD(dup(RedirectFD)) {
-    if (OldFD == -1 ||
-        sys::fs::createTemporaryFile("unittest-redirect", "", NewFD,
-                                     FilePath) ||
-        dup2(NewFD, RedirectFD) == -1)
-      Valid = false;
-  }
-
-  ~OutputRedirector() {
-    dup2(OldFD, RedirectFD);
-    close(OldFD);
-    close(NewFD);
-  }
-
-  SmallVector<char, 128> FilePath;
-  bool Valid = true;
-
-private:
-  int RedirectFD;
-  int OldFD;
-  int NewFD;
-};
-
-struct AutoDeleteFile {
-  SmallVector<char, 128> FilePath;
-  ~AutoDeleteFile() {
-    if (!FilePath.empty())
-      sys::fs::remove(std::string(FilePath.data(), FilePath.size()));
-  }
-};
-
-class PrintOptionInfoTest : public ::testing::Test {
-public:
-  // Return std::string because the output of a failing EXPECT check is
-  // unreadable for StringRef. It also avoids any lifetime issues.
-  template <typename... Ts> std::string runTest(Ts... OptionAttributes) {
-    AutoDeleteFile File;
-    {
-      OutputRedirector Stdout(fileno(stdout));
-      if (!Stdout.Valid)
-        return "";
-      File.FilePath = Stdout.FilePath;
-
-      StackOption<OptionValue> TestOption(Opt, cl::desc(HelpText),
-                                          OptionAttributes...);
-      printOptionInfo(TestOption, 25);
-      outs().flush();
-    }
-    auto Buffer = MemoryBuffer::getFile(File.FilePath);
-    if (!Buffer)
-      return "";
-    return Buffer->get()->getBuffer().str();
-  }
-
-  enum class OptionValue { Val };
-  const StringRef Opt = "some-option";
-  const StringRef HelpText = "some help";
-
-private:
-  // This is a workaround for cl::Option sub-classes having their
-  // printOptionInfo functions private.
-  void printOptionInfo(const cl::Option &O, size_t Width) {
-    O.printOptionInfo(Width);
-  }
-};
-
-TEST_F(PrintOptionInfoTest, PrintOptionInfoValueOptionalWithoutSentinel) {
-  std::string Output =
-      runTest(cl::ValueOptional,
-              cl::values(clEnumValN(OptionValue::Val, "v1", "desc1")));
-
-  // clang-format off
-  EXPECT_EQ(Output, ("  -" + Opt + "=<value> - " + HelpText + "\n"
-                     "    =v1                -   desc1\n")
-                        .str());
-  // clang-format on
-}
-
-TEST_F(PrintOptionInfoTest, PrintOptionInfoValueOptionalWithSentinel) {
-  std::string Output = runTest(
-      cl::ValueOptional, cl::values(clEnumValN(OptionValue::Val, "v1", "desc1"),
-                                    clEnumValN(OptionValue::Val, "", "")));
-
-  // clang-format off
-  EXPECT_EQ(Output,
-            ("  -" + Opt + "         - " + HelpText + "\n"
-             "  -" + Opt + "=<value> - " + HelpText + "\n"
-             "    =v1                -   desc1\n")
-                .str());
-  // clang-format on
-}
-
-TEST_F(PrintOptionInfoTest, PrintOptionInfoValueOptionalWithSentinelWithHelp) {
-  std::string Output = runTest(
-      cl::ValueOptional, cl::values(clEnumValN(OptionValue::Val, "v1", "desc1"),
-                                    clEnumValN(OptionValue::Val, "", "desc2")));
-
-  // clang-format off
-  EXPECT_EQ(Output, ("  -" + Opt + "         - " + HelpText + "\n"
-                     "  -" + Opt + "=<value> - " + HelpText + "\n"
-                     "    =v1                -   desc1\n"
-                     "    =<empty>           -   desc2\n")
-                        .str());
-  // clang-format on
-}
-
-TEST_F(PrintOptionInfoTest, PrintOptionInfoValueRequiredWithEmptyValueName) {
-  std::string Output = runTest(
-      cl::ValueRequired, cl::values(clEnumValN(OptionValue::Val, "v1", "desc1"),
-                                    clEnumValN(OptionValue::Val, "", "")));
-
-  // clang-format off
-  EXPECT_EQ(Output, ("  -" + Opt + "=<value> - " + HelpText + "\n"
-                     "    =v1                -   desc1\n"
-                     "    =<empty>\n")
-                        .str());
-  // clang-format on
-}
-
-TEST_F(PrintOptionInfoTest, PrintOptionInfoEmptyValueDescription) {
-  std::string Output = runTest(
-      cl::ValueRequired, cl::values(clEnumValN(OptionValue::Val, "v1", "")));
-
-  // clang-format off
-  EXPECT_EQ(Output,
-            ("  -" + Opt + "=<value> - " + HelpText + "\n"
-             "    =v1\n").str());
-  // clang-format on
-}
-
-class GetOptionWidthTest : public ::testing::Test {
-public:
-  enum class OptionValue { Val };
-
-  template <typename... Ts>
-  size_t runTest(StringRef ArgName, Ts... OptionAttributes) {
-    StackOption<OptionValue> TestOption(ArgName, cl::desc("some help"),
-                                        OptionAttributes...);
-    return getOptionWidth(TestOption);
-  }
-
-private:
-  // This is a workaround for cl::Option sub-classes having their
-  // printOptionInfo
-  // functions private.
-  size_t getOptionWidth(const cl::Option &O) { return O.getOptionWidth(); }
-};
-
-TEST_F(GetOptionWidthTest, GetOptionWidthArgNameLonger) {
-  StringRef ArgName("a-long-argument-name");
-  Twine ExpectedStr = "  -" + ArgName + "=<value> - ";
-  EXPECT_EQ(
-      runTest(ArgName, cl::values(clEnumValN(OptionValue::Val, "v", "help"))),
-      ExpectedStr.str().size());
-}
-
-TEST_F(GetOptionWidthTest, GetOptionWidthFirstOptionNameLonger) {
-  StringRef OptName("a-long-option-name");
-  Twine ExpectedStr = "    =" + OptName + " - ";
-  EXPECT_EQ(
-      runTest("a", cl::values(clEnumValN(OptionValue::Val, OptName, "help"),
-                              clEnumValN(OptionValue::Val, "b", "help"))),
-      ExpectedStr.str().size());
-}
-
-TEST_F(GetOptionWidthTest, GetOptionWidthSecondOptionNameLonger) {
-  StringRef OptName("a-long-option-name");
-  Twine ExpectedStr = "    =" + OptName + " - ";
-  EXPECT_EQ(
-      runTest("a", cl::values(clEnumValN(OptionValue::Val, "b", "help"),
-                              clEnumValN(OptionValue::Val, OptName, "help"))),
-      ExpectedStr.str().size());
-}
-
-TEST_F(GetOptionWidthTest, GetOptionWidthEmptyOptionNameLonger) {
-  Twine ExpectedStr = "    =<empty> - ";
-  // The length of a=<value> (including indentation) is actually the same as the
-  // =<empty> string, so it is impossible to distinguish via testing the case
-  // where the empty string is picked from where the option name is picked.
-  EXPECT_EQ(runTest("a", cl::values(clEnumValN(OptionValue::Val, "b", "help"),
-                                    clEnumValN(OptionValue::Val, "", "help"))),
-            ExpectedStr.str().size());
-}
-
-TEST_F(GetOptionWidthTest,
-       GetOptionWidthValueOptionalEmptyOptionWithNoDescription) {
-  StringRef ArgName("a");
-  // The length of a=<value> (including indentation) is actually the same as the
-  // =<empty> string, so it is impossible to distinguish via testing the case
-  // where the empty string is ignored from where it is not ignored.
-  // The dash will not actually be printed, but the space it would take up is
-  // included to ensure a consistent column width.
-  Twine ExpectedStr = "  -" + ArgName + "=<value> - ";
-  EXPECT_EQ(runTest(ArgName, cl::ValueOptional,
-                    cl::values(clEnumValN(OptionValue::Val, "value", "help"),
-                               clEnumValN(OptionValue::Val, "", ""))),
-            ExpectedStr.str().size());
-}
-
-TEST_F(GetOptionWidthTest,
-       GetOptionWidthValueRequiredEmptyOptionWithNoDescription) {
-  // The length of a=<value> (including indentation) is actually the same as the
-  // =<empty> string, so it is impossible to distinguish via testing the case
-  // where the empty string is picked from where the option name is picked
-  Twine ExpectedStr = "    =<empty> - ";
-  EXPECT_EQ(runTest("a", cl::ValueRequired,
-                    cl::values(clEnumValN(OptionValue::Val, "value", "help"),
-                               clEnumValN(OptionValue::Val, "", ""))),
-            ExpectedStr.str().size());
-}
-
 }  // anonymous namespace




More information about the llvm-commits mailing list