From: James Henderson Date: Mon, 4 Feb 2019 15:09:58 +0000 (+0000) Subject: Revert r353048. X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=5e786f3ff1f03e2f69d5ec0b6c26de4a2217d0d0;p=llvm Revert r353048. It was causing unexpected unit test failures on build bots. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@353050 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/lib/Support/CommandLine.cpp b/lib/Support/CommandLine.cpp index 52f1c0e007b..f8bc6a8f615 100644 --- a/lib/Support/CommandLine.cpp +++ b/lib/Support/CommandLine.cpp @@ -1469,23 +1469,17 @@ 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() + 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 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 HelpStr, size_t Indent, // 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(const Option &O) const { 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(const Option &O) const { // 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(const Option &O, 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(StringRef Name) { return e; } -static StringRef EqValue = "="; -static StringRef EmptyOption = ""; -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::getOptionWidth(const Option &O) const { 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()) diff --git a/tools/llvm-symbolizer/llvm-symbolizer.cpp b/tools/llvm-symbolizer/llvm-symbolizer.cpp index 0b0b8a68a4a..15b2b9dddd4 100644 --- a/tools/llvm-symbolizer/llvm-symbolizer.cpp +++ b/tools/llvm-symbolizer/llvm-symbolizer.cpp @@ -38,7 +38,7 @@ ClUseSymbolTable("use-symbol-table", cl::init(true), static cl::opt 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"), diff --git a/unittests/Support/CommandLineTest.cpp b/unittests/Support/CommandLineTest.cpp index 5c95c9d044b..416e0eef5bf 100644 --- a/unittests/Support/CommandLineTest.cpp +++ b/unittests/Support/CommandLineTest.cpp @@ -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, GetCommandLineArguments) { } #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 FilePath; - bool Valid = true; - -private: - int RedirectFD; - int OldFD; - int NewFD; -}; - -struct AutoDeleteFile { - SmallVector 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 std::string runTest(Ts... OptionAttributes) { - AutoDeleteFile File; - { - OutputRedirector Stdout(fileno(stdout)); - if (!Stdout.Valid) - return ""; - File.FilePath = Stdout.FilePath; - - StackOption 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 + "= - " + 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 + "= - " + 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 + "= - " + HelpText + "\n" - " =v1 - desc1\n" - " = - 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 + "= - " + HelpText + "\n" - " =v1 - desc1\n" - " =\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 + "= - " + HelpText + "\n" - " =v1\n").str()); - // clang-format on -} - -class GetOptionWidthTest : public ::testing::Test { -public: - enum class OptionValue { Val }; - - template - size_t runTest(StringRef ArgName, Ts... OptionAttributes) { - StackOption 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 + "= - "; - 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 = " = - "; - // The length of a= (including indentation) is actually the same as the - // = 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= (including indentation) is actually the same as the - // = 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 + "= - "; - 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= (including indentation) is actually the same as the - // = 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 = " = - "; - EXPECT_EQ(runTest("a", cl::ValueRequired, - cl::values(clEnumValN(OptionValue::Val, "value", "help"), - clEnumValN(OptionValue::Val, "", ""))), - ExpectedStr.str().size()); -} - } // anonymous namespace