Index: lib/Support/CommandLine.cpp =================================================================== --- lib/Support/CommandLine.cpp +++ lib/Support/CommandLine.cpp @@ -1663,13 +1663,35 @@ 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() << " -" << O.ArgStr; + Option::printHelpStr(O.HelpStr, GlobalWidth, O.ArgStr.size() + 6); + break; + } + } + } + outs() << " -" << O.ArgStr << "="; + Option::printHelpStr(O.HelpStr, GlobalWidth, O.ArgStr.size() + 14); 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 ValueName = getOption(i); + StringRef Description = getDescription(i); + if (O.getValueExpectedFlag() == ValueOptional && ValueName.empty() && + Description.empty()) + continue; + size_t NumSpaces = GlobalWidth - ValueName.size() - 8; + outs() << " =" << ValueName; + if (ValueName.empty()) { + outs() << ""; + NumSpaces -= 7; + } + if (!Description.empty()) + outs().indent(NumSpaces) << " - " << Description; + outs() << '\n'; } } else { if (!O.HelpStr.empty()) Index: tools/llvm-symbolizer/llvm-symbolizer.cpp =================================================================== --- tools/llvm-symbolizer/llvm-symbolizer.cpp +++ tools/llvm-symbolizer/llvm-symbolizer.cpp @@ -38,7 +38,7 @@ 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"), Index: unittests/Support/CommandLineTest.cpp =================================================================== --- unittests/Support/CommandLineTest.cpp +++ unittests/Support/CommandLineTest.cpp @@ -13,6 +13,7 @@ #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" @@ -839,4 +840,137 @@ } #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 +} + } // anonymous namespace