[Support][CommandLine] Make it possible to get error messages from ParseCommandLineOptions when ignoring errors.

[ioeric]

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.

[ioeric]

• updated comment.

[rnk]

lib/Support/CommandLine.cpp

2182 ↗

(On Diff #91683)

Use raw_null_ostream instead.

unittests/Support/CommandLineTest.cpp

303 ↗

(On Diff #91683)

Use raw_null_ostream, here and below.

Actually, can you pick one or two unit tests and test that we get an error message in the error stream?

[ioeric]

• Addressed review comments.

[rnk]

lgtm

llvm/trunk/include/llvm/Support/CommandLine.h

 /// cl Namespace - This namespace contains all of the command line option
 /// processing machinery.  It is intentionally a short name to make qualified
 /// usage concise.
 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
 //                           entry point.
 //
 void ParseEnvironmentOptions(const char *progName, const char *envvar,
                              const char *Overview = "");
 

llvm/trunk/lib/Support/CommandLine.cpp

public:
   CommandLineParser() : ActiveSubCommand(nullptr) {
     registerSubCommand(&*TopLevelSubCommand);
     registerSubCommand(&*AllSubCommands);
   }
 
   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())
       return;
     if (!SC->OptionsMap.insert(std::make_pair(Name, &Opt)).second) {
       errs() << ProgramName << ": CommandLine Error: Option '" << Name
              << "' registered more than once!\n";
       report_fatal_error("inconsistency in registered CommandLine options");
void cl::ParseEnvironmentOptions(const char *progName, const char *envVar,
   // Parse the value of the environment variable into a "command line"
   // and hand it off to ParseCommandLineOptions().
   TokenizeGNUCommandLine(*envValue, Saver, newArgv);
   int newArgc = static_cast<int>(newArgv.size());
   ParseCommandLineOptions(newArgc, &newArgv[0], StringRef(Overview));
 }
 
 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() {
   // So that we can parse different command lines multiple times in succession
   // we reset all option values to look like they have never been seen before.
   for (auto SC : RegisteredSubCommands) {
     for (auto &O : SC->OptionsMap)
       O.second->reset();
   }
 }
 
 bool CommandLineParser::ParseCommandLineOptions(int argc,
                                                 const char *const *argv,
                                                 StringRef Overview,
-                                                bool IgnoreErrors) {
+                                                raw_ostream *Errs) {
   assert(hasOptions() && "No options specified!");
 
   // Expand response files.
   SmallVector<const char *, 20> newArgv(argv, argv + argc);
   BumpPtrAllocator A;
   StringSaver Saver(A);
   ExpandResponseFiles(Saver, TokenizeGNUCommandLine, newArgv);
   argv = &newArgv[0];
   argc = static_cast<int>(newArgv.size());
 
   // Copy the program name into ProgName, making sure not to overflow it.
   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.
   unsigned NumPositionalRequired = 0;
 
   // Determine whether or not there are an unlimited number of positionals
   bool HasUnlimitedPositionals = false;
 
for (size_t i = 0, e = PositionalOpts.size(); i != e; ++i) {
                        "cl::ConsumeAfter option is active!");
           ErrorParsing = true;
         }
       } else if (UnboundedFound && !Opt->hasArgStr()) {
         // This option does not "require" a value...  Make sure this option is
         // 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);
     }
     HasUnlimitedPositionals = UnboundedFound || ConsumeAfterOpt;
   }
 
   // PositionalVals - A vector of "positional" arguments we accumulate into
if (argv[i][0] != '-' || argv[i][1] == 0 || DashDashFound) {
       // in the upcoming error.
       if (!Handler && SinkOpts.empty())
         NearestHandler =
             LookupNearestOption(ArgName, OptionsMap, NearestHandlerString);
     }
 
     if (!Handler) {
       if (SinkOpts.empty()) {
-        if (!IgnoreErrors) {
-          errs() << ProgramName << ": Unknown command line argument '"
-                 << argv[i] << "'.  Try: '" << argv[0] << " -help'\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";
-          }
-        }
+          *Errs << ProgramName << ": Did you mean '-" << NearestHandlerString
 
         ErrorParsing = true;
       } else {
         for (SmallVectorImpl<Option *>::iterator I = SinkOpts.begin(),
                                                  E = SinkOpts.end();
              I != E; ++I)
           (*I)->addOccurrence(i, "", StringRef(argv[i]));
       }
       continue;
     }
 
     // If this is a named positional argument, just remember that it is the
     // active one...
     if (Handler->getFormattingFlag() == cl::Positional)
       ActivePositionalArg = Handler;
     else
       ErrorParsing |= ProvideOption(Handler, ArgName, Value, argc, argv, i);
   }
 
   // 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"
     ErrorParsing = true;
 
   } else if (!ConsumeAfterOpt) {
     // Positional args have already been handled if ConsumeAfter is specified.
     unsigned ValNo = 0, NumVals = static_cast<unsigned>(PositionalVals.size());
     for (size_t i = 0, e = PositionalOpts.size(); i != e; ++i) {
       if (RequiresValue(PositionalOpts[i])) {
         ProvidePositionalOption(PositionalOpts[i], PositionalVals[ValNo].first,

 
 void cl::ResetCommandLineParser() { GlobalParser->reset(); }
 void cl::ResetAllOptionOccurrences() {
   GlobalParser->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());
 }

llvm/trunk/unittests/Support/CommandLineTest.cpp

TEST(CommandLineTest, SetValueInSubcategories) {
   StackOption<bool> TopLevelOpt("top-level", cl::init(false));
   StackOption<bool> SC1Opt("sc1", cl::sub(SC1), cl::init(false));
   StackOption<bool> SC2Opt("sc2", cl::sub(SC2), cl::init(false));
 
   EXPECT_FALSE(TopLevelOpt);
   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);
 
   TopLevelOpt = false;
 
   cl::ResetAllOptionOccurrences();
   EXPECT_FALSE(TopLevelOpt);
   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);
 
   SC1Opt = false;
 
   cl::ResetAllOptionOccurrences();
   EXPECT_FALSE(TopLevelOpt);
   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);
 }
 
 TEST(CommandLineTest, LookupFailsInWrongSubCommand) {
   cl::ResetCommandLineParser();
 
   StackSubCommand SC1("sc1", "First subcommand");
   StackSubCommand SC2("sc2", "Second subcommand");
 
   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) {
   cl::ResetCommandLineParser();
 
   StackSubCommand SC1("sc1", "First subcommand");
   StackOption<bool> AllOpt("everywhere", cl::sub(*cl::AllSubCommands),
                            cl::init(false));
   StackSubCommand SC2("sc2", "Second subcommand");
 
   const char *args[] = {"prog", "-everywhere"};
   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) {
   cl::ResetCommandLineParser();
 
   StackOption<bool> TopLevelOpt("top-level", cl::sub(*cl::TopLevelSubCommand),
                                 cl::init(false));
 
   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);
 }
 
 TEST(CommandLineTest, RemoveFromRegularSubCommand) {
   cl::ResetCommandLineParser();
 
   StackSubCommand SC("sc", "Subcommand");
   StackOption<bool> RemoveOption("remove-option", cl::sub(SC), cl::init(false));
   StackOption<bool> KeepOption("keep-option", cl::sub(SC), cl::init(false));
 
   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) {
   cl::ResetCommandLineParser();
 
   StackOption<bool> TopLevelRemove(
       "top-level-remove", cl::sub(*cl::TopLevelSubCommand), cl::init(false));
   StackOption<bool> TopLevelKeep(
       "top-level-keep", cl::sub(*cl::TopLevelSubCommand), cl::init(false));
 
   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) {
   cl::ResetCommandLineParser();
 
   StackSubCommand SC1("sc1", "First Subcommand");
   StackSubCommand SC2("sc2", "Second Subcommand");
   StackOption<bool> RemoveOption("remove-option", cl::sub(*cl::AllSubCommands),
                                  cl::init(false));
   StackOption<bool> KeepOption("keep-option", cl::sub(*cl::AllSubCommands),
                                cl::init(false));
 
   const char *args0[] = {"prog", "-remove-option"};
   const char *args1[] = {"prog", "sc1", "-remove-option"};
   const char *args2[] = {"prog", "sc2", "-remove-option"};
 
   // 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) {
   cl::ResetCommandLineParser();
 
   StackSubCommand SC1("sc1", "First Subcommand");
   StackOption<bool> Opt1("opt1", cl::sub(SC1), cl::init(false));
   StackSubCommand SC2("sc2", "Second subcommand");
   StackOption<bool> Opt2("opt2", cl::sub(SC2), cl::init(false));
 
   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()) {
     if (*S)
       EXPECT_EQ("sc1", S->getName());
   }
 
   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()) {
     if (*S)
       EXPECT_EQ("sc2", S->getName());
   }
 }