[Support][CommandLine] Make it possible to get error messages from ParseCommandLineOp...

Summary:
Previously, ParseCommandLineOptions returns false and ignores error messages
when IgnoreErrors. It would be useful to also return error messages if users
decide to check parsing result instead of having the program exit on error.

Reviewers: chandlerc, mehdi_amini, rnk

Reviewed By: rnk

Subscribers: llvm-commits

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

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

diff --git a/include/llvm/Support/CommandLine.h b/include/llvm/Support/CommandLine.h
index d1d698400df8fda69965073d4fa7f857c93683a4..ae32e20d6daba932545aaa3d2742ff24bef7696f 100644 (file)
--- a/include/llvm/Support/CommandLine.h
+++ b/include/llvm/Support/CommandLine.h
@@ -50,9 +50,12 @@ namespace cl {
 //===----------------------------------------------------------------------===//
 // ParseCommandLineOptions - Command line option processing entry point.
 //
+// Returns true on success. Otherwise, this will print the error message to
+// stderr and exit if \p Errs is not set (nullptr by default), or print the
+// error message to \p Errs and return false if \p Errs is provided.
 bool ParseCommandLineOptions(int argc, const char *const *argv,
                              StringRef Overview = "",
-                             bool IgnoreErrors = false);
+                             raw_ostream *Errs = nullptr);
 
 //===----------------------------------------------------------------------===//
 // ParseEnvironmentOptions - Environment variable option processing alternate

diff --git a/lib/Support/CommandLine.cpp b/lib/Support/CommandLine.cpp
index b1142d79cdb21bd780af5b2a11fc57d96def0359..65747d6929155a4e7be26133510e2e0c70c5a82f 100644 (file)
--- a/lib/Support/CommandLine.cpp
+++ b/lib/Support/CommandLine.cpp
@@ -123,7 +123,7 @@ public:
   void ResetAllOptionOccurrences();
 
   bool ParseCommandLineOptions(int argc, const char *const *argv,
-                               StringRef Overview, bool IgnoreErrors);
+                               StringRef Overview, raw_ostream *Errs = nullptr);
 
   void addLiteralOption(Option &Opt, SubCommand *SC, StringRef Name) {
     if (Opt.hasArgStr())
@@ -1013,9 +1013,9 @@ void cl::ParseEnvironmentOptions(const char *progName, const char *envVar,
 }
 
 bool cl::ParseCommandLineOptions(int argc, const char *const *argv,
-                                 StringRef Overview, bool IgnoreErrors) {
+                                 StringRef Overview, raw_ostream *Errs) {
   return GlobalParser->ParseCommandLineOptions(argc, argv, Overview,
-                                               IgnoreErrors);
+                                               Errs);
 }
 
 void CommandLineParser::ResetAllOptionOccurrences() {
@@ -1030,7 +1030,7 @@ void CommandLineParser::ResetAllOptionOccurrences() {
 bool CommandLineParser::ParseCommandLineOptions(int argc,
                                                 const char *const *argv,
                                                 StringRef Overview,
-                                                bool IgnoreErrors) {
+                                                raw_ostream *Errs) {
   assert(hasOptions() && "No options specified!");
 
   // Expand response files.
@@ -1045,6 +1045,9 @@ bool CommandLineParser::ParseCommandLineOptions(int argc,
   ProgramName = sys::path::filename(StringRef(argv[0]));
 
   ProgramOverview = Overview;
+  bool IgnoreErrors = Errs;
+  if (!Errs)
+    Errs = &errs();
   bool ErrorParsing = false;
 
   // Check out the positional arguments to collect information about them.
@@ -1097,15 +1100,14 @@ bool CommandLineParser::ParseCommandLineOptions(int argc,
         // not specified after an option that eats all extra arguments, or this
         // one will never get any!
         //
-        if (!IgnoreErrors) {
+        if (!IgnoreErrors)
           Opt->error("error - option can never match, because "
                      "another positional argument will match an "
                      "unbounded number of values, and this option"
                      " does not require a value!");
-          errs() << ProgramName << ": CommandLine Error: Option '"
-                 << Opt->ArgStr << "' is all messed up!\n";
-          errs() << PositionalOpts.size();
-        }
+        *Errs << ProgramName << ": CommandLine Error: Option '" << Opt->ArgStr
+              << "' is all messed up!\n";
+        *Errs << PositionalOpts.size();
         ErrorParsing = true;
       }
       UnboundedFound |= EatsUnboundedNumberOfValues(Opt);
@@ -1200,15 +1202,13 @@ bool CommandLineParser::ParseCommandLineOptions(int argc,
 
     if (!Handler) {
       if (SinkOpts.empty()) {
-        if (!IgnoreErrors) {
-          errs() << ProgramName << ": Unknown command line argument '"
-                 << argv[i] << "'.  Try: '" << argv[0] << " -help'\n";
-
-          if (NearestHandler) {
-            // If we know a near match, report it as well.
-            errs() << ProgramName << ": Did you mean '-" << NearestHandlerString
-                   << "'?\n";
-          }
+        *Errs << ProgramName << ": Unknown command line argument '" << argv[i]
+              << "'.  Try: '" << argv[0] << " -help'\n";
+
+        if (NearestHandler) {
+          // If we know a near match, report it as well.
+          *Errs << ProgramName << ": Did you mean '-" << NearestHandlerString
+                 << "'?\n";
         }
 
         ErrorParsing = true;
@@ -1231,22 +1231,18 @@ bool CommandLineParser::ParseCommandLineOptions(int argc,
 
   // Check and handle positional arguments now...
   if (NumPositionalRequired > PositionalVals.size()) {
-    if (!IgnoreErrors) {
-      errs() << ProgramName
+      *Errs << ProgramName
              << ": Not enough positional command line arguments specified!\n"
              << "Must specify at least " << NumPositionalRequired
              << " positional argument" << (NumPositionalRequired > 1 ? "s" : "")
              << ": See: " << argv[0] << " - help\n";
-    }
 
     ErrorParsing = true;
   } else if (!HasUnlimitedPositionals &&
              PositionalVals.size() > PositionalOpts.size()) {
-    if (!IgnoreErrors) {
-      errs() << ProgramName << ": Too many positional arguments specified!\n"
-             << "Can specify at most " << PositionalOpts.size()
-             << " positional arguments: See: " << argv[0] << " -help\n";
-    }
+    *Errs << ProgramName << ": Too many positional arguments specified!\n"
+          << "Can specify at most " << PositionalOpts.size()
+          << " positional arguments: See: " << argv[0] << " -help\n";
     ErrorParsing = true;
 
   } else if (!ConsumeAfterOpt) {
@@ -2182,5 +2178,6 @@ void cl::ResetAllOptionOccurrences() {
 
 void LLVMParseCommandLineOptions(int argc, const char *const *argv,
                                  const char *Overview) {
-  llvm::cl::ParseCommandLineOptions(argc, argv, StringRef(Overview), true);
+  llvm::cl::ParseCommandLineOptions(argc, argv, StringRef(Overview),
+                                    &llvm::nulls());
 }

diff --git a/unittests/Support/CommandLineTest.cpp b/unittests/Support/CommandLineTest.cpp
index 945eb1d4e1cfeb3416b40d88bf4c77ab0b0cbdf0..33573c4e696077cf354bf74991319d7d14ed19c4 100644 (file)
--- a/unittests/Support/CommandLineTest.cpp
+++ b/unittests/Support/CommandLineTest.cpp
@@ -303,7 +303,8 @@ TEST(CommandLineTest, SetValueInSubcategories) {
   EXPECT_FALSE(SC1Opt);
   EXPECT_FALSE(SC2Opt);
   const char *args[] = {"prog", "-top-level"};
-  EXPECT_TRUE(cl::ParseCommandLineOptions(2, args, StringRef(), true));
+  EXPECT_TRUE(
+      cl::ParseCommandLineOptions(2, args, StringRef(), &llvm::nulls()));
   EXPECT_TRUE(TopLevelOpt);
   EXPECT_FALSE(SC1Opt);
   EXPECT_FALSE(SC2Opt);
@@ -315,7 +316,8 @@ TEST(CommandLineTest, SetValueInSubcategories) {
   EXPECT_FALSE(SC1Opt);
   EXPECT_FALSE(SC2Opt);
   const char *args2[] = {"prog", "sc1", "-sc1"};
-  EXPECT_TRUE(cl::ParseCommandLineOptions(3, args2, StringRef(), true));
+  EXPECT_TRUE(
+      cl::ParseCommandLineOptions(3, args2, StringRef(), &llvm::nulls()));
   EXPECT_FALSE(TopLevelOpt);
   EXPECT_TRUE(SC1Opt);
   EXPECT_FALSE(SC2Opt);
@@ -327,7 +329,8 @@ TEST(CommandLineTest, SetValueInSubcategories) {
   EXPECT_FALSE(SC1Opt);
   EXPECT_FALSE(SC2Opt);
   const char *args3[] = {"prog", "sc2", "-sc2"};
-  EXPECT_TRUE(cl::ParseCommandLineOptions(3, args3, StringRef(), true));
+  EXPECT_TRUE(
+      cl::ParseCommandLineOptions(3, args3, StringRef(), &llvm::nulls()));
   EXPECT_FALSE(TopLevelOpt);
   EXPECT_FALSE(SC1Opt);
   EXPECT_TRUE(SC2Opt);
@@ -342,8 +345,13 @@ TEST(CommandLineTest, LookupFailsInWrongSubCommand) {
   StackOption<bool> SC1Opt("sc1", cl::sub(SC1), cl::init(false));
   StackOption<bool> SC2Opt("sc2", cl::sub(SC2), cl::init(false));
 
+  std::string Errs;
+  raw_string_ostream OS(Errs);
+
   const char *args[] = {"prog", "sc1", "-sc2"};
-  EXPECT_FALSE(cl::ParseCommandLineOptions(3, args, StringRef(), true));
+  EXPECT_FALSE(cl::ParseCommandLineOptions(3, args, StringRef(), &OS));
+  OS.flush();
+  EXPECT_FALSE(Errs.empty());
 }
 
 TEST(CommandLineTest, AddToAllSubCommands) {
@@ -358,23 +366,30 @@ TEST(CommandLineTest, AddToAllSubCommands) {
   const char *args2[] = {"prog", "sc1", "-everywhere"};
   const char *args3[] = {"prog", "sc2", "-everywhere"};
 
+  std::string Errs;
+  raw_string_ostream OS(Errs);
+
   EXPECT_FALSE(AllOpt);
-  EXPECT_TRUE(cl::ParseCommandLineOptions(2, args, StringRef(), true));
+  EXPECT_TRUE(cl::ParseCommandLineOptions(2, args, StringRef(), &OS));
   EXPECT_TRUE(AllOpt);
 
   AllOpt = false;
 
   cl::ResetAllOptionOccurrences();
   EXPECT_FALSE(AllOpt);
-  EXPECT_TRUE(cl::ParseCommandLineOptions(3, args2, StringRef(), true));
+  EXPECT_TRUE(cl::ParseCommandLineOptions(3, args2, StringRef(), &OS));
   EXPECT_TRUE(AllOpt);
 
   AllOpt = false;
 
   cl::ResetAllOptionOccurrences();
   EXPECT_FALSE(AllOpt);
-  EXPECT_TRUE(cl::ParseCommandLineOptions(3, args3, StringRef(), true));
+  EXPECT_TRUE(cl::ParseCommandLineOptions(3, args3, StringRef(), &OS));
   EXPECT_TRUE(AllOpt);
+
+  // Since all parsing succeeded, the error message should be empty.
+  OS.flush();
+  EXPECT_TRUE(Errs.empty());
 }
 
 TEST(CommandLineTest, ReparseCommandLineOptions) {
@@ -386,14 +401,16 @@ TEST(CommandLineTest, ReparseCommandLineOptions) {
   const char *args[] = {"prog", "-top-level"};
 
   EXPECT_FALSE(TopLevelOpt);
-  EXPECT_TRUE(cl::ParseCommandLineOptions(2, args, StringRef(), true));
+  EXPECT_TRUE(
+      cl::ParseCommandLineOptions(2, args, StringRef(), &llvm::nulls()));
   EXPECT_TRUE(TopLevelOpt);
 
   TopLevelOpt = false;
 
   cl::ResetAllOptionOccurrences();
   EXPECT_FALSE(TopLevelOpt);
-  EXPECT_TRUE(cl::ParseCommandLineOptions(2, args, StringRef(), true));
+  EXPECT_TRUE(
+      cl::ParseCommandLineOptions(2, args, StringRef(), &llvm::nulls()));
   EXPECT_TRUE(TopLevelOpt);
 }
 
@@ -406,14 +423,21 @@ TEST(CommandLineTest, RemoveFromRegularSubCommand) {
 
   const char *args[] = {"prog", "sc", "-remove-option"};
 
+  std::string Errs;
+  raw_string_ostream OS(Errs);
+
   EXPECT_FALSE(RemoveOption);
-  EXPECT_TRUE(cl::ParseCommandLineOptions(3, args, StringRef(), true));
+  EXPECT_TRUE(cl::ParseCommandLineOptions(3, args, StringRef(), &OS));
   EXPECT_TRUE(RemoveOption);
+  OS.flush();
+  EXPECT_TRUE(Errs.empty());
 
   RemoveOption.removeArgument();
 
   cl::ResetAllOptionOccurrences();
-  EXPECT_FALSE(cl::ParseCommandLineOptions(3, args, StringRef(), true));
+  EXPECT_FALSE(cl::ParseCommandLineOptions(3, args, StringRef(), &OS));
+  OS.flush();
+  EXPECT_FALSE(Errs.empty());
 }
 
 TEST(CommandLineTest, RemoveFromTopLevelSubCommand) {
@@ -427,13 +451,15 @@ TEST(CommandLineTest, RemoveFromTopLevelSubCommand) {
   const char *args[] = {"prog", "-top-level-remove"};
 
   EXPECT_FALSE(TopLevelRemove);
-  EXPECT_TRUE(cl::ParseCommandLineOptions(2, args, StringRef(), true));
+  EXPECT_TRUE(
+      cl::ParseCommandLineOptions(2, args, StringRef(), &llvm::nulls()));
   EXPECT_TRUE(TopLevelRemove);
 
   TopLevelRemove.removeArgument();
 
   cl::ResetAllOptionOccurrences();
-  EXPECT_FALSE(cl::ParseCommandLineOptions(2, args, StringRef(), true));
+  EXPECT_FALSE(
+      cl::ParseCommandLineOptions(2, args, StringRef(), &llvm::nulls()));
 }
 
 TEST(CommandLineTest, RemoveFromAllSubCommands) {
@@ -452,32 +478,38 @@ TEST(CommandLineTest, RemoveFromAllSubCommands) {
 
   // It should work for all subcommands including the top-level.
   EXPECT_FALSE(RemoveOption);
-  EXPECT_TRUE(cl::ParseCommandLineOptions(2, args0, StringRef(), true));
+  EXPECT_TRUE(
+      cl::ParseCommandLineOptions(2, args0, StringRef(), &llvm::nulls()));
   EXPECT_TRUE(RemoveOption);
 
   RemoveOption = false;
 
   cl::ResetAllOptionOccurrences();
   EXPECT_FALSE(RemoveOption);
-  EXPECT_TRUE(cl::ParseCommandLineOptions(3, args1, StringRef(), true));
+  EXPECT_TRUE(
+      cl::ParseCommandLineOptions(3, args1, StringRef(), &llvm::nulls()));
   EXPECT_TRUE(RemoveOption);
 
   RemoveOption = false;
 
   cl::ResetAllOptionOccurrences();
   EXPECT_FALSE(RemoveOption);
-  EXPECT_TRUE(cl::ParseCommandLineOptions(3, args2, StringRef(), true));
+  EXPECT_TRUE(
+      cl::ParseCommandLineOptions(3, args2, StringRef(), &llvm::nulls()));
   EXPECT_TRUE(RemoveOption);
 
   RemoveOption.removeArgument();
 
   // It should not work for any subcommands including the top-level.
   cl::ResetAllOptionOccurrences();
-  EXPECT_FALSE(cl::ParseCommandLineOptions(2, args0, StringRef(), true));
+  EXPECT_FALSE(
+      cl::ParseCommandLineOptions(2, args0, StringRef(), &llvm::nulls()));
   cl::ResetAllOptionOccurrences();
-  EXPECT_FALSE(cl::ParseCommandLineOptions(3, args1, StringRef(), true));
+  EXPECT_FALSE(
+      cl::ParseCommandLineOptions(3, args1, StringRef(), &llvm::nulls()));
   cl::ResetAllOptionOccurrences();
-  EXPECT_FALSE(cl::ParseCommandLineOptions(3, args2, StringRef(), true));
+  EXPECT_FALSE(
+      cl::ParseCommandLineOptions(3, args2, StringRef(), &llvm::nulls()));
 }
 
 TEST(CommandLineTest, GetRegisteredSubcommands) {
@@ -491,7 +523,8 @@ TEST(CommandLineTest, GetRegisteredSubcommands) {
   const char *args0[] = {"prog", "sc1"};
   const char *args1[] = {"prog", "sc2"};
 
-  EXPECT_TRUE(cl::ParseCommandLineOptions(2, args0, StringRef(), true));
+  EXPECT_TRUE(
+      cl::ParseCommandLineOptions(2, args0, StringRef(), &llvm::nulls()));
   EXPECT_FALSE(Opt1);
   EXPECT_FALSE(Opt2);
   for (auto *S : cl::getRegisteredSubcommands()) {
@@ -500,7 +533,8 @@ TEST(CommandLineTest, GetRegisteredSubcommands) {
   }
 
   cl::ResetAllOptionOccurrences();
-  EXPECT_TRUE(cl::ParseCommandLineOptions(2, args1, StringRef(), true));
+  EXPECT_TRUE(
+      cl::ParseCommandLineOptions(2, args1, StringRef(), &llvm::nulls()));
   EXPECT_FALSE(Opt1);
   EXPECT_FALSE(Opt2);
   for (auto *S : cl::getRegisteredSubcommands()) {