From 759645ea89e1185868034637172a255ca9cf799c Mon Sep 17 00:00:00 2001 From: Justin Bogner Date: Mon, 14 Jul 2014 20:53:57 +0000 Subject: [PATCH] Support: Fix option handling when using cl::Required with aliasopt Until now, attempting to create an alias of a required option would complain if the user supplied the alias, because the required option didn't have a value. Similarly, if you said the alias was required, then using the base option would complain that the alias wasn't supplied. Lastly, if you put required on both, *neither* option would work. By changning alias to overload addOccurrence and setting cl::Required on the original option, we can get this to behave in a more useful way. I've also added a test and updated a user that was getting this wrong. llvm-svn: 212986 --- llvm/include/llvm/Support/CommandLine.h | 8 ++++++-- llvm/tools/llvm-profdata/llvm-profdata.cpp | 4 ++-- llvm/unittests/Support/CommandLineTest.cpp | 19 +++++++++++++++++++ 3 files changed, 27 insertions(+), 4 deletions(-) diff --git a/llvm/include/llvm/Support/CommandLine.h b/llvm/include/llvm/Support/CommandLine.h index 5cb5501..fdd9012 100644 --- a/llvm/include/llvm/Support/CommandLine.h +++ b/llvm/include/llvm/Support/CommandLine.h @@ -270,8 +270,8 @@ public: // addOccurrence - Wrapper around handleOccurrence that enforces Flags. // - bool addOccurrence(unsigned pos, StringRef ArgName, - StringRef Value, bool MultiArg = false); + virtual bool addOccurrence(unsigned pos, StringRef ArgName, + StringRef Value, bool MultiArg = false); // Prints option name followed by message. Always returns true. bool error(const Twine &Message, StringRef ArgName = StringRef()); @@ -1649,6 +1649,10 @@ class alias : public Option { StringRef Arg) override { return AliasFor->handleOccurrence(pos, AliasFor->ArgStr, Arg); } + bool addOccurrence(unsigned pos, StringRef /*ArgName*/, + StringRef Value, bool MultiArg = false) override { + return AliasFor->addOccurrence(pos, AliasFor->ArgStr, Value, MultiArg); + } // Handle printing stuff... size_t getOptionWidth() const override; void printOptionInfo(size_t GlobalWidth) const override; diff --git a/llvm/tools/llvm-profdata/llvm-profdata.cpp b/llvm/tools/llvm-profdata/llvm-profdata.cpp index ba88aad..49ad37e 100644 --- a/llvm/tools/llvm-profdata/llvm-profdata.cpp +++ b/llvm/tools/llvm-profdata/llvm-profdata.cpp @@ -38,9 +38,9 @@ int merge_main(int argc, const char *argv[]) { cl::desc("")); cl::opt OutputFilename("output", cl::value_desc("output"), - cl::init("-"), + cl::init("-"), cl::Required, cl::desc("Output file")); - cl::alias OutputFilenameA("o", cl::desc("Alias for --output"), cl::Required, + cl::alias OutputFilenameA("o", cl::desc("Alias for --output"), cl::aliasopt(OutputFilename)); cl::ParseCommandLineOptions(argc, argv, "LLVM profile data merger\n"); diff --git a/llvm/unittests/Support/CommandLineTest.cpp b/llvm/unittests/Support/CommandLineTest.cpp index b2d71ab..e4a1b67 100644 --- a/llvm/unittests/Support/CommandLineTest.cpp +++ b/llvm/unittests/Support/CommandLineTest.cpp @@ -212,4 +212,23 @@ TEST(CommandLineTest, AliasesWithArguments) { } } +void testAliasRequired(int argc, const char *const *argv) { + StackOption Option("option", cl::Required); + cl::alias Alias("o", llvm::cl::aliasopt(Option)); + + cl::ParseCommandLineOptions(argc, argv); + EXPECT_EQ("x", Option); + EXPECT_EQ(1, Option.getNumOccurrences()); + + Alias.removeArgument(); +} + +TEST(CommandLineTest, AliasRequired) { + const char *opts1[] = { "-tool", "-option=x" }; + const char *opts2[] = { "-tool", "-o", "x" }; + testAliasRequired(array_lengthof(opts1), opts1); + testAliasRequired(array_lengthof(opts2), opts2); +} + + } // anonymous namespace -- 2.7.4