Refactoring llvm command line parsing and option registration.

[beanz]

Summary

The primary goal of this patch is to remove the need for MarkOptionsChanged(). That goal is accomplished by having addOption and removeOption properly sort the options.

This patch puts the new add and remove functionality on a CommandLineParser class that is a placeholder. Some of the functionality in this class will need to be merged into the OptionRegistry, and other bits can hopefully be in a better abstraction.

This patch also removes the RegisteredOptionList global, and the need for cl::Option objects to be linked list nodes.

The changes in CommandLineTest.cpp are required because these changes shift when we validate that options are not duplicated. Before this change duplicate options were only found during certain cl API calls (like cl::ParseCommandLine). With this change duplicate options are found during option construction.

[majnemer]

lib/Support/CommandLine.cpp

1495 ↗

(On Diff #18639)

Range-based for might be more concise.

1516–1519 ↗

(On Diff #18639)

Range-based for might be more concise.

[beanz]

Converted two for loops to range-based for loops based on majnemer's feedback.

[beanz]

Rebasing patches so they apply cleanly.

[pete]

LGTM.

[Ilod]

Hello,

This breaks the build on Visual 2013, with:
C:\Users\Ilod\pdb\llvm\lib\Support\CommandLine.cpp(90): error C2536: 'CommandLineParser::CommandLineParser::ProgramName' : cannot specify explicit initializer for arrays

You can't have an initializer for array or static array in the class definition in Visual 2013, you must do it in the constructor.

[beanz]

I’ll put together a patch for this shortly.

-Chris

[beanz]

Committed a fix for Visual Studio r227385.

-Chris

[rmaprath]

In D7132#114838, @beanz wrote:

Committed a fix for Visual Studio r227385.

-Chris

Hi Chris,

This doesn't seem to have fixed the failure:

(ClCompile target) -> 
  F:\work\src\llvm\lib\Support\CommandLine.cpp(91): error C2864: 'CommandLineParser::ProgramOverview' : only static const integral data members can be initialized within a class  ...
  F:\work\src\llvm\lib\Support\CommandLine.cpp(100): error C2864: 'CommandLineParser::ConsumeAfterOpt' : only static const integral data members can be initialized within a class ...

Moving the initializer to the (default) constructor does it:

diff --git a/lib/Support/CommandLine.cpp b/lib/Support/CommandLine.cpp
index 2ca9caa..a3d7d06 100644
--- a/lib/Support/CommandLine.cpp
+++ b/lib/Support/CommandLine.cpp
@@ -88,7 +88,7 @@ public:
   // Globals for name and overview of program.  Program name is not a string to
   // avoid static ctor/dtor issues.
   std::string ProgramName;
-  const char *ProgramOverview = nullptr;
+  const char *ProgramOverview;
 
   // This collects additional help to be printed.
   std::vector<const char *> MoreHelp;
@@ -97,7 +97,9 @@ public:
   SmallVector<Option *, 4> SinkOpts;
   StringMap<Option *> OptionsMap;
 
-  Option *ConsumeAfterOpt = nullptr; // The ConsumeAfter option if it exists.
+  Option *ConsumeAfterOpt; // The ConsumeAfter option if it exists.
+
+  CommandLineParser():ProgramOverview(nullptr), ConsumeAfterOpt(nullptr){}
 
   void ParseCommandLineOptions(int argc, const char *const *argv,
                                const char *Overview);

• Asiri

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

 void AddExtraVersionPrinter(void (*func)());
 
 // PrintOptionValues - Print option values.
 // With -print-options print the difference between option values and defaults.
 // With -print-all-options print all option values.
 // (Currently not perfect, but best-effort.)
 void PrintOptionValues();
 
-// MarkOptionsChanged - Internal helper function.
-void MarkOptionsChanged();
-
 //===----------------------------------------------------------------------===//
 // Flags permitted to be passed to command line arguments
 //
 
 enum NumOccurrencesFlag { // Flags for the number of occurrences allowed
   Optional = 0x00,        // Zero or One occurrence
   ZeroOrMore = 0x01,      // Zero or more occurrences allowed
   Required = 0x02,        // One occurrence required
class Option {
   // not using the enum type for 'Value' because zero is an implementation
   // detail representing the non-value
   unsigned Value : 2;
   unsigned HiddenFlag : 2; // enum OptionHidden
   unsigned Formatting : 2; // enum FormattingFlags
   unsigned Misc : 3;
   unsigned Position;       // Position of last occurrence of the option
   unsigned AdditionalVals; // Greater than 0 for multi-valued option.
-  Option *NextRegistered;  // Singly linked list of registered options.
 
 public:
   const char *ArgStr;   // The argument string itself (ex: "help", "o")
   const char *HelpStr;  // The descriptive text message for -help
   const char *ValueStr; // String describing what the value of this option is
   OptionCategory *Category; // The Category this option belongs to
+  bool FullyInitialized;    // Has addArguemnt been called?
 
   inline enum NumOccurrencesFlag getNumOccurrencesFlag() const {
     return (enum NumOccurrencesFlag)Occurrences;
   }
   inline enum ValueExpected getValueExpectedFlag() const {
     return Value ? ((enum ValueExpected)Value) : getValueExpectedFlagDefault();
   }
   inline enum OptionHidden getOptionHiddenFlag() const {
     return (enum OptionHidden)HiddenFlag;
   }
   inline enum FormattingFlags getFormattingFlag() const {
     return (enum FormattingFlags)Formatting;
   }
   inline unsigned getMiscFlags() const { return Misc; }
   inline unsigned getPosition() const { return Position; }
   inline unsigned getNumAdditionalVals() const { return AdditionalVals; }
 
   // hasArgStr - Return true if the argstr != ""
   bool hasArgStr() const { return ArgStr[0] != 0; }
 
   //-------------------------------------------------------------------------===
   // Accessor functions set by OptionModifiers
   //
-  void setArgStr(const char *S) { ArgStr = S; }
+  void setArgStr(const char *S);
   void setDescription(const char *S) { HelpStr = S; }
   void setValueStr(const char *S) { ValueStr = S; }
   void setNumOccurrencesFlag(enum NumOccurrencesFlag Val) { Occurrences = Val; }
   void setValueExpectedFlag(enum ValueExpected Val) { Value = Val; }
   void setHiddenFlag(enum OptionHidden Val) { HiddenFlag = Val; }
   void setFormattingFlag(enum FormattingFlags V) { Formatting = V; }
   void setMiscFlag(enum MiscFlags M) { Misc |= M; }
   void setPosition(unsigned pos) { Position = pos; }
   void setCategory(OptionCategory &C) { Category = &C; }
 
 protected:
   explicit Option(enum NumOccurrencesFlag OccurrencesFlag,
                   enum OptionHidden Hidden)
       : NumOccurrences(0), Occurrences(OccurrencesFlag), Value(0),
         HiddenFlag(Hidden), Formatting(NormalFormatting), Misc(0), Position(0),
-        AdditionalVals(0), NextRegistered(nullptr), ArgStr(""), HelpStr(""),
-        ValueStr(""), Category(&GeneralCategory) {}
+        AdditionalVals(0), ArgStr(""), HelpStr(""), ValueStr(""),
+        Category(&GeneralCategory), FullyInitialized(false) {}
 
   inline void setNumAdditionalVals(unsigned n) { AdditionalVals = n; }
 
 public:
   // addArgument - Register this argument with the commandline system.
   //
   void addArgument();
 
   /// Unregisters this option from the CommandLine system.
   ///
   /// This option must have been the last option registered.
   /// For testing purposes only.
   void removeArgument();
 
-  Option *getNextRegisteredOption() const { return NextRegistered; }
-
   // Return the width of the option tag for printing...
   virtual size_t getOptionWidth() const = 0;
 
   // printOptionInfo - Print out information about this option.  The
   // to-be-maintained width is specified.
   //
   virtual void printOptionInfo(size_t GlobalWidth) const = 0;
 
public:
 
   /// addLiteralOption - Add an entry to the mapping table.
   ///
   template <class DT>
   void addLiteralOption(const char *Name, const DT &V, const char *HelpStr) {
     assert(findOption(Name) == Values.size() && "Option already exists!");
     OptionInfo X(Name, static_cast<DataType>(V), HelpStr);
     Values.push_back(X);
-    MarkOptionsChanged();
+    AddLiteralOption(Owner, Name);
   }
 
   /// removeLiteralOption - Remove the specified option.
   ///
   void removeLiteralOption(const char *Name) {
     unsigned N = findOption(Name);
     assert(N != Values.size() && "Option not found!");
     Values.erase(Values.begin() + N);

 
 //===----------------------------------------------------------------------===//
 // Public interface for accessing registered options.
 //
 
 /// \brief Use this to get a StringMap to all registered named options
 /// (e.g. -help). Note \p Map Should be an empty StringMap.
 ///
-/// \param [out] Map will be filled with mappings where the key is the
-/// Option argument string (e.g. "help") and value is the corresponding
-/// Option*.
+/// \return A reference to the StringMap used by the cl APIs to parse options.
 ///
 /// Access to unnamed arguments (i.e. positional) are not provided because
 /// it is expected that the client already has access to these.
 ///
 /// Typical usage:
 /// \code
 /// main(int argc,char* argv[]) {
-/// StringMap<llvm::cl::Option*> opts;
-/// llvm::cl::getRegisteredOptions(opts);
+/// StringMap<llvm::cl::Option*> &opts = llvm::cl::getRegisteredOptions();
 /// assert(opts.count("help") == 1)
 /// opts["help"]->setDescription("Show alphabetical help information")
 /// // More code
 /// llvm::cl::ParseCommandLineOptions(argc,argv);
 /// //More code
 /// }
 /// \endcode
 ///
 /// This interface is useful for modifying options in libraries that are out of
 /// the control of the client. The options should be modified before calling
 /// llvm::cl::ParseCommandLineOptions().
-void getRegisteredOptions(StringMap<Option *> &Map);
+///
+/// Hopefully this API can be depricated soon. Any situation where options need
+/// to be modified by tools or libraries should be handled by sane APIs rather
+/// than just handing around a global list.
+StringMap<Option *> &getRegisteredOptions();
 
 //===----------------------------------------------------------------------===//
 // Standalone command line processing utilities.
 //
 
 /// \brief Saves strings in the inheritor's stable storage and returns a stable
 /// raw character pointer.
 class StringSaver {

 ///
 /// \param Categories the categories of options to keep displaying.
 ///
 /// Some tools (like clang-format) like to be able to hide all options that are
 /// not specific to the tool. This function allows a tool to specify a single
 /// option category to display in the -help output.
 void HideUnrelatedOptions(ArrayRef<const cl::OptionCategory *> Categories);
 
+/// \brief Adds a new option for parsing and provides the option it refers to.
+///
+/// \param O pointer to the option
+/// \param Name the string name for the option to handle during parsing
+///
+/// Literal options are used by some parsers to register special option values.
+/// This is how the PassNameParser registers pass names for opt.
+void AddLiteralOption(Option &O, const char *Name);
+
 } // End namespace cl
 
 } // End namespace llvm
 
 #endif

llvm/trunk/lib/Support/CommandLine.cpp

 void parser<double>::anchor() {}
 void parser<float>::anchor() {}
 void parser<std::string>::anchor() {}
 void parser<char>::anchor() {}
 void StringSaver::anchor() {}
 
 //===----------------------------------------------------------------------===//
 
+class CommandLineParser {
+public:
   // Globals for name and overview of program.  Program name is not a string to
   // avoid static ctor/dtor issues.
-static char ProgramName[80] = "<premain>";
-static const char *ProgramOverview = nullptr;
+  char ProgramName[80] = "<premain>";
+  const char *ProgramOverview = nullptr;
 
   // This collects additional help to be printed.
-static ManagedStatic<std::vector<const char *>> MoreHelp;
+  std::vector<const char *> MoreHelp;
 
-extrahelp::extrahelp(const char *Help) : morehelp(Help) {
-  MoreHelp->push_back(Help);
+  SmallVector<Option *, 4> PositionalOpts;
+  SmallVector<Option *, 4> SinkOpts;
+  StringMap<Option *> OptionsMap;
+
+  Option *ConsumeAfterOpt = nullptr; // The ConsumeAfter option if it exists.
+
+  void ParseCommandLineOptions(int argc, const char *const *argv,
+                               const char *Overview);
+
+  void addLiteralOption(Option &Opt, const char *Name) {
+    if (!Opt.hasArgStr()) {
+      if (!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");
+      }
+    }
   }
 
-static bool OptionListChanged = false;
+  void addOption(Option *O) {
+    bool HadErrors = false;
+    if (O->ArgStr[0]) {
+      // Add argument to the argument map!
+      if (!OptionsMap.insert(std::make_pair(O->ArgStr, O)).second) {
+        errs() << ProgramName << ": CommandLine Error: Option '" << O->ArgStr
+               << "' registered more than once!\n";
+        HadErrors = true;
+      }
+    }
 
-// MarkOptionsChanged - Internal helper function.
-void cl::MarkOptionsChanged() { OptionListChanged = true; }
+    // Remember information about positional options.
+    if (O->getFormattingFlag() == cl::Positional)
+      PositionalOpts.push_back(O);
+    else if (O->getMiscFlags() & cl::Sink) // Remember sink options
+      SinkOpts.push_back(O);
+    else if (O->getNumOccurrencesFlag() == cl::ConsumeAfter) {
+      if (ConsumeAfterOpt) {
+        O->error("Cannot specify more than one option with cl::ConsumeAfter!");
+        HadErrors = true;
+      }
+      ConsumeAfterOpt = O;
+    }
 
-/// RegisteredOptionList - This is the list of the command line options that
-/// have statically constructed themselves.
-static Option *RegisteredOptionList = nullptr;
+    // Fail hard if there were errors. These are strictly unrecoverable and
+    // indicate serious issues such as conflicting option names or an
+    // incorrectly
+    // linked LLVM distribution.
+    if (HadErrors)
+      report_fatal_error("inconsistency in registered CommandLine options");
+  }
 
-void Option::addArgument() {
-  assert(!NextRegistered && "argument multiply registered!");
+  void removeOption(Option *O) {
+    SmallVector<const char *, 16> OptionNames;
+    O->getExtraOptionNames(OptionNames);
+    if (O->ArgStr[0])
+      OptionNames.push_back(O->ArgStr);
+    for (auto Name : OptionNames)
+      OptionsMap.erase(StringRef(Name));
 
-  NextRegistered = RegisteredOptionList;
-  RegisteredOptionList = this;
-  MarkOptionsChanged();
+    if (O->getFormattingFlag() == cl::Positional)
+      for (auto Opt = PositionalOpts.begin(); Opt != PositionalOpts.end();
+           ++Opt) {
+        if (*Opt == O) {
+          PositionalOpts.erase(Opt);
+          break;
+        }
+      }
+    else if (O->getMiscFlags() & cl::Sink)
+      for (auto Opt = SinkOpts.begin(); Opt != SinkOpts.end(); ++Opt) {
+        if (*Opt == O) {
+          SinkOpts.erase(Opt);
+          break;
+        }
+      }
+    else if (O == ConsumeAfterOpt)
+      ConsumeAfterOpt = nullptr;
   }
 
-void Option::removeArgument() {
-  if (RegisteredOptionList == this) {
-    RegisteredOptionList = NextRegistered;
-    MarkOptionsChanged();
-    return;
+  bool hasOptions() {
+    return (!OptionsMap.empty() || !PositionalOpts.empty() ||
+            nullptr != ConsumeAfterOpt);
+  }
+
+  void updateArgStr(Option *O, const char* NewName) {
+    if (!OptionsMap.insert(std::make_pair(NewName, O)).second) {
+      errs() << ProgramName << ": CommandLine Error: Option '" << O->ArgStr
+             << "' registered more than once!\n";
+      report_fatal_error("inconsistency in registered CommandLine options");
     }
-  Option *O = RegisteredOptionList;
-  for (; O->NextRegistered != this; O = O->NextRegistered)
-    ;
-  O->NextRegistered = NextRegistered;
-  MarkOptionsChanged();
+    OptionsMap.erase(StringRef(O->ArgStr));
+  }
+};
+
+static ManagedStatic<CommandLineParser> GlobalParser;
+
+void cl::AddLiteralOption(Option &O, const char *Name) {
+  GlobalParser->addLiteralOption(O, Name);
+}
+
+extrahelp::extrahelp(const char *Help) : morehelp(Help) {
+  GlobalParser->MoreHelp.push_back(Help);
+}
+
+void Option::addArgument() {
+  GlobalParser->addOption(this);
+  FullyInitialized = true;
+}
+
+void Option::removeArgument() { GlobalParser->removeOption(this); }
+
+void Option::setArgStr(const char *S) {
+  if (FullyInitialized)
+    GlobalParser->updateArgStr(this, S);
+  ArgStr = S;
 }
 
 // This collects the different option categories that have been registered.
 typedef SmallPtrSet<OptionCategory *, 16> OptionCatSet;
 static ManagedStatic<OptionCatSet> RegisteredOptionCategories;
 
 // Initialise the general option category.
 OptionCategory llvm::cl::GeneralCategory("General options");
 
 void OptionCategory::registerCategory() {
   assert(std::count_if(RegisteredOptionCategories->begin(),
                        RegisteredOptionCategories->end(),
                        [this](const OptionCategory *Category) {
                          return getName() == Category->getName();
                        }) == 0 &&
          "Duplicate option categories");
 
   RegisteredOptionCategories->insert(this);
 }
 
 //===----------------------------------------------------------------------===//
 // Basic, shared command line option processing machinery.
 //
 
-/// GetOptionInfo - Scan the list of registered options, turning them into data
-/// structures that are easier to handle.
-static void GetOptionInfo(SmallVectorImpl<Option *> &PositionalOpts,
-                          SmallVectorImpl<Option *> &SinkOpts,
-                          StringMap<Option *> &OptionsMap) {
-  bool HadErrors = false;
-  SmallVector<const char *, 16> OptionNames;
-  Option *CAOpt = nullptr; // The ConsumeAfter option if it exists.
-  for (Option *O = RegisteredOptionList; O; O = O->getNextRegisteredOption()) {
-    // If this option wants to handle multiple option names, get the full set.
-    // This handles enum options like "-O1 -O2" etc.
-    O->getExtraOptionNames(OptionNames);
-    if (O->ArgStr[0])
-      OptionNames.push_back(O->ArgStr);
-
-    // Handle named options.
-    for (size_t i = 0, e = OptionNames.size(); i != e; ++i) {
-      // Add argument to the argument map!
-      if (!OptionsMap.insert(std::make_pair(OptionNames[i], O)).second) {
-        errs() << ProgramName << ": CommandLine Error: Option '"
-               << OptionNames[i] << "' registered more than once!\n";
-        HadErrors = true;
-      }
-    }
-
-    OptionNames.clear();
-
-    // Remember information about positional options.
-    if (O->getFormattingFlag() == cl::Positional)
-      PositionalOpts.push_back(O);
-    else if (O->getMiscFlags() & cl::Sink) // Remember sink options
-      SinkOpts.push_back(O);
-    else if (O->getNumOccurrencesFlag() == cl::ConsumeAfter) {
-      if (CAOpt) {
-        O->error("Cannot specify more than one option with cl::ConsumeAfter!");
-        HadErrors = true;
-      }
-      CAOpt = O;
-    }
-  }
-
-  if (CAOpt)
-    PositionalOpts.push_back(CAOpt);
-
-  // Make sure that they are in order of registration not backwards.
-  std::reverse(PositionalOpts.begin(), PositionalOpts.end());
-
-  // Fail hard if there were errors. These are strictly unrecoverable and
-  // indicate serious issues such as conflicting option names or an incorrectly
-  // linked LLVM distribution.
-  if (HadErrors)
-    report_fatal_error("inconsistency in registered CommandLine options");
-}
-
 /// LookupOption - Lookup the option specified by the specified option on the
 /// command line.  If there is a value specified (after an equal sign) return
 /// that as well.  This assumes that leading dashes have already been stripped.
 static Option *LookupOption(StringRef &Arg, StringRef &Value,
                             const StringMap<Option *> &OptionsMap) {
   // Reject all dashes.
   if (Arg.empty())
     return nullptr;
void cl::ParseEnvironmentOptions(const char *progName, const char *envVar,
   // and hand it off to ParseCommandLineOptions().
   TokenizeGNUCommandLine(envValue, Saver, newArgv);
   int newArgc = static_cast<int>(newArgv.size());
   ParseCommandLineOptions(newArgc, &newArgv[0], Overview);
 }
 
 void cl::ParseCommandLineOptions(int argc, const char *const *argv,
                                  const char *Overview) {
-  // Process all registered options.
-  SmallVector<Option *, 4> PositionalOpts;
-  SmallVector<Option *, 4> SinkOpts;
-  StringMap<Option *> Opts;
-  GetOptionInfo(PositionalOpts, SinkOpts, Opts);
+  GlobalParser->ParseCommandLineOptions(argc, argv, Overview);
+}
 
-  assert((!Opts.empty() || !PositionalOpts.empty()) && "No options specified!");
+void CommandLineParser::ParseCommandLineOptions(int argc,
+                                                const char *const *argv,
+                                                const char *Overview) {
+  assert(hasOptions() && "No options specified!");
 
   // Expand response files.
   SmallVector<const char *, 20> newArgv;
   for (int i = 0; i != argc; ++i)
     newArgv.push_back(argv[i]);
   StrDupSaver Saver;
   ExpandResponseFiles(Saver, TokenizeGNUCommandLine, newArgv);
   argv = &newArgv[0];
void CommandLineParser::ParseCommandLineOptions(int argc,
   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;
 
-  Option *ConsumeAfterOpt = nullptr;
-  if (!PositionalOpts.empty()) {
-    if (PositionalOpts[0]->getNumOccurrencesFlag() == cl::ConsumeAfter) {
-      assert(PositionalOpts.size() > 1 &&
-             "Cannot specify cl::ConsumeAfter without a positional argument!");
-      ConsumeAfterOpt = PositionalOpts[0];
-    }
+  if (ConsumeAfterOpt) {
+    assert(PositionalOpts.size() > 0 &&
+  if (!PositionalOpts.empty()) {
 
     // Calculate how many positional values are _required_.
     bool UnboundedFound = false;
-    for (size_t i = ConsumeAfterOpt ? 1 : 0, e = PositionalOpts.size(); i != e;
-         ++i) {
+    for (size_t i = 0, e = PositionalOpts.size(); i != e; ++i) {
       Option *Opt = PositionalOpts[i];
       if (RequiresValue(Opt))
         ++NumPositionalRequired;
       else if (ConsumeAfterOpt) {
         // ConsumeAfter cannot be combined with "optional" positional options
         // unless there is only one positional argument...
-        if (PositionalOpts.size() > 2)
+        if (PositionalOpts.size() > 1)
           ErrorParsing |= Opt->error(
               "error - this positional option will never be matched, "
               "because it does not Require a value, and a "
               "cl::ConsumeAfter option is active!");
       } else if (UnboundedFound && !Opt->ArgStr[0]) {
         // 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!
         //
         ErrorParsing |= 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();
       }
       UnboundedFound |= EatsUnboundedNumberOfValues(Opt);
     }
     HasUnlimitedPositionals = UnboundedFound || ConsumeAfterOpt;
   }
 
   // PositionalVals - A vector of "positional" arguments we accumulate into
   // the process at the end.
void CommandLineParser::ParseCommandLineOptions(int argc,
   bool DashDashFound = false; // Have we read '--'?
   for (int i = 1; i < argc; ++i) {
     Option *Handler = nullptr;
     Option *NearestHandler = nullptr;
     std::string NearestHandlerString;
     StringRef Value;
     StringRef ArgName = "";
 
-    // If the option list changed, this means that some command line
-    // option has just been registered or deregistered.  This can occur in
-    // response to things like -load, etc.  If this happens, rescan the options.
-    if (OptionListChanged) {
-      PositionalOpts.clear();
-      SinkOpts.clear();
-      Opts.clear();
-      GetOptionInfo(PositionalOpts, SinkOpts, Opts);
-      OptionListChanged = false;
-    }
-
     // Check to see if this is a positional argument.  This argument is
     // considered to be positional if it doesn't start with '-', if it is "-"
     // itself, or if we have seen "--" already.
     //
     if (argv[i][0] != '-' || argv[i][1] == 0 || DashDashFound) {
       // Positional argument!
       if (ActivePositionalArg) {
         ProvidePositionalOption(ActivePositionalArg, argv[i], i);
if (argv[i][0] != '-' || argv[i][1] == 0 || DashDashFound) {
       // If there is a positional argument eating options, check to see if this
       // option is another positional argument.  If so, treat it as an argument,
       // otherwise feed it to the eating positional.
       ArgName = argv[i] + 1;
       // Eat leading dashes.
       while (!ArgName.empty() && ArgName[0] == '-')
         ArgName = ArgName.substr(1);
 
-      Handler = LookupOption(ArgName, Value, Opts);
+      Handler = LookupOption(ArgName, Value, OptionsMap);
       if (!Handler || Handler->getFormattingFlag() != cl::Positional) {
         ProvidePositionalOption(ActivePositionalArg, argv[i], i);
         continue; // We are done!
       }
 
     } else { // We start with a '-', must be an argument.
       ArgName = argv[i] + 1;
       // Eat leading dashes.
       while (!ArgName.empty() && ArgName[0] == '-')
         ArgName = ArgName.substr(1);
 
-      Handler = LookupOption(ArgName, Value, Opts);
+      Handler = LookupOption(ArgName, Value, OptionsMap);
 
       // Check to see if this "option" is really a prefixed or grouped argument.
       if (!Handler)
-        Handler =
-            HandlePrefixedOrGroupedOption(ArgName, Value, ErrorParsing, Opts);
+        Handler = HandlePrefixedOrGroupedOption(ArgName, Value, ErrorParsing,
+                                                OptionsMap);
 
       // Otherwise, look for the closest available option to report to the user
       // in the upcoming error.
       if (!Handler && SinkOpts.empty())
         NearestHandler =
-            LookupNearestOption(ArgName, Opts, NearestHandlerString);
+            LookupNearestOption(ArgName, OptionsMap, NearestHandlerString);
     }
 
     if (!Handler) {
       if (SinkOpts.empty()) {
         errs() << ProgramName << ": Unknown command line argument '" << argv[i]
                << "'.  Try: '" << argv[0] << " -help'\n";
 
         if (NearestHandler) {
for (size_t j = 1, e = PositionalOpts.size(); j != e; ++j)
         ValNo++;
       }
 
     // Handle the case where there is just one positional option, and it's
     // optional.  In this case, we want to give JUST THE FIRST option to the
     // positional option and keep the rest for the consume after.  The above
     // loop would have assigned no values to positional options in this case.
     //
-    if (PositionalOpts.size() == 2 && ValNo == 0 && !PositionalVals.empty()) {
-      ErrorParsing |= ProvidePositionalOption(PositionalOpts[1],
+    if (PositionalOpts.size() == 1 && ValNo == 0 && !PositionalVals.empty()) {
+      ErrorParsing |= ProvidePositionalOption(PositionalOpts[0],
                                               PositionalVals[ValNo].first,
                                               PositionalVals[ValNo].second);
       ValNo++;
     }
 
     // Handle over all of the rest of the arguments to the
     // cl::ConsumeAfter command line option...
     for (; ValNo != PositionalVals.size(); ++ValNo)
       ErrorParsing |=
           ProvidePositionalOption(ConsumeAfterOpt, PositionalVals[ValNo].first,
                                   PositionalVals[ValNo].second);
   }
 
   // Loop over args and make sure all required args are specified!
-  for (const auto &Opt : Opts) {
+  for (const auto &Opt : OptionsMap) {
     switch (Opt.second->getNumOccurrencesFlag()) {
     case Required:
     case OneOrMore:
       if (Opt.second->getNumOccurrences() == 0) {
         Opt.second->error("must be specified at least once!");
         ErrorParsing = true;
       }
     // Fall through
     default:
       break;
     }
   }
 
   // Now that we know if -debug is specified, we can use it.
   // Note that if ReadResponseFiles == true, this must be done before the
   // memory allocated for the expanded command line is free()d below.
   DEBUG(dbgs() << "Args: ";
         for (int i = 0; i < argc; ++i) dbgs() << argv[i] << ' ';
         dbgs() << '\n';);
 
   // Free all of the memory allocated to the map.  Command line options may only
   // be processed once!
-  Opts.clear();
-  PositionalOpts.clear();
-  MoreHelp->clear();
+  MoreHelp.clear();
 
   // If we had an error processing our arguments, don't let the program execute
   if (ErrorParsing)
     exit(1);
 }
 
 //===----------------------------------------------------------------------===//
 // Option Base class implementation
 //
 
 bool Option::error(const Twine &Message, StringRef ArgName) {
   if (!ArgName.data())
     ArgName = ArgStr;
   if (ArgName.empty())
     errs() << HelpStr; // Be nice for positional arguments
   else
-    errs() << ProgramName << ": for the -" << ArgName;
+    errs() << GlobalParser->ProgramName << ": for the -" << ArgName;
 
   errs() << " option: " << Message << "\n";
   return true;
 }
 
 bool Option::addOccurrence(unsigned pos, StringRef ArgName, StringRef Value,
                            bool MultiArg) {
   if (!MultiArg)
public:
   explicit HelpPrinter(bool showHidden) : ShowHidden(showHidden) {}
   virtual ~HelpPrinter() {}
 
   // Invoke the printer.
   void operator=(bool Value) {
     if (Value == false)
       return;
 
-    // Get all the options.
-    SmallVector<Option *, 4> PositionalOpts;
-    SmallVector<Option *, 4> SinkOpts;
-    StringMap<Option *> OptMap;
-    GetOptionInfo(PositionalOpts, SinkOpts, OptMap);
-
     StrOptionPairVector Opts;
-    sortOpts(OptMap, Opts, ShowHidden);
+    sortOpts(GlobalParser->OptionsMap, Opts, ShowHidden);
 
-    if (ProgramOverview)
-      outs() << "OVERVIEW: " << ProgramOverview << "\n";
+    if (GlobalParser->ProgramOverview)
+      outs() << "OVERVIEW: " << GlobalParser->ProgramOverview << "\n";
 
-    outs() << "USAGE: " << ProgramName << " [options]";
+    outs() << "USAGE: " << GlobalParser->ProgramName << " [options]";
 
-    // Print out the positional options.
-    Option *CAOpt = nullptr; // The cl::ConsumeAfter option, if it exists...
-    if (!PositionalOpts.empty() &&
-        PositionalOpts[0]->getNumOccurrencesFlag() == ConsumeAfter)
-      CAOpt = PositionalOpts[0];
-
-    for (size_t i = CAOpt != nullptr, e = PositionalOpts.size(); i != e; ++i) {
-      if (PositionalOpts[i]->ArgStr[0])
-        outs() << " --" << PositionalOpts[i]->ArgStr;
-      outs() << " " << PositionalOpts[i]->HelpStr;
+    for (auto Opt : GlobalParser->PositionalOpts) {
+      if (Opt->ArgStr[0])
+        outs() << " --" << Opt->ArgStr;
+      outs() << " " << Opt->HelpStr;
     }
 
     // Print the consume after option info if it exists...
-    if (CAOpt)
-      outs() << " " << CAOpt->HelpStr;
+    if (GlobalParser->ConsumeAfterOpt)
+      outs() << " " << GlobalParser->ConsumeAfterOpt->HelpStr;
 
     outs() << "\n\n";
 
     // Compute the maximum argument length...
     size_t MaxArgLen = 0;
     for (size_t i = 0, e = Opts.size(); i != e; ++i)
       MaxArgLen = std::max(MaxArgLen, Opts[i].second->getOptionWidth());
 
     outs() << "OPTIONS:\n";
     printOptions(Opts, MaxArgLen);
 
     // Print any extra help the user has declared.
-    for (std::vector<const char *>::iterator I = MoreHelp->begin(),
-                                             E = MoreHelp->end();
-         I != E; ++I)
-      outs() << *I;
-    MoreHelp->clear();
+    for (auto I : GlobalParser->MoreHelp)
+      outs() << I;
+    GlobalParser->MoreHelp.clear();
 
     // Halt the program since help information was printed
     exit(0);
   }
 };
 
 class CategorizedHelpPrinter : public HelpPrinter {
 public:
if (RegisteredOptionCategories->size() > 1) {
     UncategorizedPrinter = true; // Invoke uncategorized printer
 }
 
 // Print the value of each option.
 void cl::PrintOptionValues() {
   if (!PrintOptions && !PrintAllOptions)
     return;
 
-  // Get all the options.
-  SmallVector<Option *, 4> PositionalOpts;
-  SmallVector<Option *, 4> SinkOpts;
-  StringMap<Option *> OptMap;
-  GetOptionInfo(PositionalOpts, SinkOpts, OptMap);
-
   SmallVector<std::pair<const char *, Option *>, 128> Opts;
-  sortOpts(OptMap, Opts, /*ShowHidden*/ true);
+  sortOpts(GlobalParser->OptionsMap, Opts, /*ShowHidden*/ true);
 
   // Compute the maximum argument length...
   size_t MaxArgLen = 0;
   for (size_t i = 0, e = Opts.size(); i != e; ++i)
     MaxArgLen = std::max(MaxArgLen, Opts[i].second->getOptionWidth());
 
   for (size_t i = 0, e = Opts.size(); i != e; ++i)
     Opts[i].second->printOptionValue(MaxArgLen, PrintAllOptions);

 
 void cl::AddExtraVersionPrinter(void (*func)()) {
   if (!ExtraVersionPrinters)
     ExtraVersionPrinters = new std::vector<void (*)()>;
 
   ExtraVersionPrinters->push_back(func);
 }
 
-void cl::getRegisteredOptions(StringMap<Option *> &Map) {
-  // Get all the options.
-  SmallVector<Option *, 4> PositionalOpts; // NOT USED
-  SmallVector<Option *, 4> SinkOpts;       // NOT USED
-  assert(Map.size() == 0 && "StringMap must be empty");
-  GetOptionInfo(PositionalOpts, SinkOpts, Map);
-  return;
+StringMap<Option *> &cl::getRegisteredOptions() {
+  return GlobalParser->OptionsMap;
 }
 
 void cl::HideUnrelatedOptions(cl::OptionCategory &Category) {
-  StringMap<cl::Option *> Options;
-  cl::getRegisteredOptions(Options);
-  for (auto &I : Options) {
+  for (auto &I : GlobalParser->OptionsMap) {
     if (I.second->Category != &Category &&
         I.second->Category != &GenericCategory)
       I.second->setHiddenFlag(cl::ReallyHidden);
   }
 }
 
 void cl::HideUnrelatedOptions(ArrayRef<const cl::OptionCategory *> Categories) {
   auto CategoriesBegin = Categories.begin();
   auto CategoriesEnd = Categories.end();
-  StringMap<cl::Option *> Options;
-  cl::getRegisteredOptions(Options);
-  for (auto &I : Options) {
+  for (auto &I : GlobalParser->OptionsMap) {
     if (std::find(CategoriesBegin, CategoriesEnd, I.second->Category) ==
             CategoriesEnd &&
         I.second->Category != &GenericCategory)
       I.second->setHiddenFlag(cl::ReallyHidden);
   }
 }
 
 void LLVMParseCommandLineOptions(int argc, const char *const *argv,
                                  const char *Overview) {
   llvm::cl::ParseCommandLineOptions(argc, argv, Overview);
 }

llvm/trunk/unittests/Support/CommandLineTest.cpp

public:
 
   ~StackOption() {
     this->removeArgument();
   }
 };
 
 
 cl::OptionCategory TestCategory("Test Options", "Description");
-cl::opt<int> TestOption("test-option", cl::desc("old description"));
 TEST(CommandLineTest, ModifyExisitingOption) {
+  StackOption<int> TestOption("test-option", cl::desc("old description"));
+
   const char Description[] = "New description";
   const char ArgString[] = "new-test-option";
   const char ValueString[] = "Integer";
 
-  StringMap<cl::Option*> Map;
-  cl::getRegisteredOptions(Map);
+  StringMap<cl::Option *> &Map = cl::getRegisteredOptions();
 
   ASSERT_TRUE(Map.count("test-option") == 1) <<
     "Could not find option in map.";
 
   cl::Option *Retrieved = Map["test-option"];
   ASSERT_EQ(&TestOption, Retrieved) << "Retrieved wrong option.";
 
   ASSERT_EQ(&cl::GeneralCategory,Retrieved->Category) <<
TEST(CommandLineTest, HideUnrelatedOptions) {
 
   cl::HideUnrelatedOptions(TestCategory);
 
   ASSERT_EQ(cl::ReallyHidden, TestOption1.getOptionHiddenFlag())
       << "Failed to hide extra option.";
   ASSERT_EQ(cl::NotHidden, TestOption2.getOptionHiddenFlag())
       << "Hid extra option that should be visable.";
 
-  StringMap<cl::Option *> Map;
-  cl::getRegisteredOptions(Map);
+  StringMap<cl::Option *> &Map = cl::getRegisteredOptions();
   ASSERT_EQ(cl::NotHidden, Map["help"]->getOptionHiddenFlag())
       << "Hid default option that should be visable.";
 }
 
 cl::OptionCategory TestCategory2("Test Options set 2", "Description");
 
 TEST(CommandLineTest, HideUnrelatedOptionsMulti) {
   StackOption<int> TestOption1("multi-hide-option-1");
   StackOption<int> TestOption2("multi-hide-option-2", cl::cat(TestCategory));
   StackOption<int> TestOption3("multi-hide-option-3", cl::cat(TestCategory2));
 
   const cl::OptionCategory *VisibleCategories[] = {&TestCategory,
                                                    &TestCategory2};
 
   cl::HideUnrelatedOptions(makeArrayRef(VisibleCategories));
 
   ASSERT_EQ(cl::ReallyHidden, TestOption1.getOptionHiddenFlag())
       << "Failed to hide extra option.";
   ASSERT_EQ(cl::NotHidden, TestOption2.getOptionHiddenFlag())
       << "Hid extra option that should be visable.";
   ASSERT_EQ(cl::NotHidden, TestOption3.getOptionHiddenFlag())
       << "Hid extra option that should be visable.";
 
-  StringMap<cl::Option *> Map;
-  cl::getRegisteredOptions(Map);
+  StringMap<cl::Option *> &Map = cl::getRegisteredOptions();
   ASSERT_EQ(cl::NotHidden, Map["help"]->getOptionHiddenFlag())
       << "Hid default option that should be visable.";
 }
 
 }  // anonymous namespace