[CommandLine] Remove OptionCategory and SubCommand caches from the Option class.

[hintonda]

Summary

This change processes OptionCategorys and SubCommands as they
are seen instead of caching them in the Option class and processing
them later. Doing so simplifies the work needed to be done by the Global
parser and significantly reduces the size of the Option class to a mere 64
bytes.

Removing the OptionCategory cache saved 24 bytes, and removing
the SubCommand cache saved an additional 48 bytes, for a total of a
72 byte reduction.

[hintonda]

The Option class started at 184 bytes, this change, along with D62091, gets it down to 88 types, and reduces bin/opt by over 90k.

[hintonda]

• Add comments, fix auto usage, and minor refactoring.

[hintonda]

• Remove Categories cache as well. This change reduces the size of the Option class down to 64 bytes.

[beanz]

Adding @zturner in case he has any thoughts.

[serge-sans-paille]

llvm/include/llvm/Support/CommandLine.h

312

Why do you need to make that getter public?

[hintonda]

llvm/include/llvm/Support/CommandLine.h

312

I think at some point I needed access, but as of now, FullyInitialized is only used within Option, so it can go away.

Thanks for catching that.

[hintonda]

• Removed unneeded public method from Option.

[hintonda]

ping...

[serge-sans-paille]

LGTM

[hintonda]

Since a SubCommand's OptionsMap key is a string, and multiple
SubCommands can have unrelated options of the same name,
getSubCommands needs to also make sure the found Option matches the
this pointer.

Also, hid the addArgument() method in Alias to prevent users from
trying to add it twice, once when specifying a SubCommand, and again
in the done() method.

Added tests to cover these two cases -- first discovered in llvm-xray,
which declares duplicate Options and Aliases in multiple SubCommands.

[hintonda]

llvm/include/llvm/Support/CommandLine.h

1797

Will fix this and related methods by making them virtual and adding asserts saying the operation isn't allowed/doesn't make sense in a separate patch.

[MaskRay]

llvm/include/llvm/Support/CommandLine.h

290

16. Does it have to be so large?

[hintonda]

llvm/include/llvm/Support/CommandLine.h

290

No, it should probably be 1 or 2, and getSubCommands should probably be 1.

Thanks for pointing this out.

[hintonda]

• Reduce compile-time container sizes.

[hintonda]

• Make Option::addArgument virtual.

[hintonda]

Even though this patch has been accepted, I've made 2 additional changes, and would
like to have them reviewed as well.

The first just reduces the compile-time default container sizes for the get* methods,
which shouldn't be too controversial.

However, the second is a bit more involved, and can be viewed in the following two
diffs -- the second of which just makes addArgument virtual.

https://reviews.llvm.org/D62105?vs=201049&id=202622&whitespace=ignore-most#toc
https://reviews.llvm.org/D62105?vs=204984&id=205092&whitespace=ignore-most#toc

It fixes bugs in the following two use cases (please see the two additional uinittests that
cover these):

• When a users incorrectly passes a cl::sub argument to an Alias ctor:
  
  Since Alias's use the SubCommands of the Option they alias, this argument should be ignored, or assert.

• When using SubCommands with duplicate Alias's, within the scope of a SubCommand:
  
  In order to distinguish which SubCommand the matching Alias belongs, it is necessary to compare the found option to this.

[serge-sans-paille]

llvm/include/llvm/Support/CommandLine.h

1797

Why do you want to do that in a separate patch?

[hintonda]

llvm/include/llvm/Support/CommandLine.h

1797

Sorry, that comment is OBE. I made it virtual instead.

My original reasoning was to make each patch smaller, but found breaking it up further un-workable.

[hintonda]

Reopening to track fix for buildbot failure -- need to make GeneralCategory a ManagedStatic.

[hintonda]

Patch is ready, so I'll post as soon as I complete rebase/build/test.

The fix is just making GeneralCategory a ManagedStatic, so without objection, I'll land it tomorrow and watch the bots.

[hintonda]

Make GeneralCategory a ManagedStatic.

llvm/include/llvm/Support/CommandLine.h

public:
   OptionCategory(StringRef const Name,
                  StringRef const Description = "")
       : Name(Name), Description(Description) {
     registerCategory();
   }
 
   StringRef getName() const { return Name; }
   StringRef getDescription() const { return Description; }
+
+  SmallPtrSet<Option *, 16> MemberOptions;
 };
 
 // The general Option Category (used as default category).
 extern OptionCategory GeneralCategory;
 
 //===----------------------------------------------------------------------===//
 // SubCommand class
 //
class Option {
   uint16_t FullyInitialized : 1; // Has addArgument been called?
   uint16_t Position;             // Position of last occurrence of the option
   uint16_t AdditionalVals;       // Greater than 0 for multi-valued option.
 
 public:
   StringRef ArgStr;   // The argument string itself (ex: "help", "o")
   StringRef HelpStr;  // The descriptive text message for -help
   StringRef ValueStr; // String describing what the value of this option is
-  SmallVector<OptionCategory *, 1>
-      Categories;                    // The Categories this option belongs to
-  SmallPtrSet<SubCommand *, 1> Subs; // The subcommands this option belongs to.
+
+  // Return the set of OptionCategories that this Option belongs to.
+  SmallPtrSet<OptionCategory *, 1> getCategories() const;
+
+  // Return the set of SubCommands that this Option belongs to.
+  SmallPtrSet<SubCommand *, 1> getSubCommands() const;
 
   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.empty(); }
   bool isPositional() const { return getFormattingFlag() == cl::Positional; }
   bool isSink() const { return getMiscFlags() & cl::Sink; }
   bool isDefaultOption() const { return getMiscFlags() & cl::DefaultOption; }
 
   bool isConsumeAfter() const {
     return getNumOccurrencesFlag() == cl::ConsumeAfter;
   }
 
-  bool isInAllSubCommands() const {
-    return any_of(Subs, [](const SubCommand *SC) {
-      return SC == &*AllSubCommands;
-    });
-  }
-
   //-------------------------------------------------------------------------===
   // Accessor functions set by OptionModifiers
   //
   void setArgStr(StringRef S);
   void setDescription(StringRef S) { HelpStr = S; }
   void setValueStr(StringRef 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 addCategory(OptionCategory &C);
-  void addSubCommand(SubCommand &S) { Subs.insert(&S); }
 
 protected:
   explicit Option(enum NumOccurrencesFlag OccurrencesFlag,
                   enum OptionHidden Hidden)
       : NumOccurrences(0), Occurrences(OccurrencesFlag), Value(0),
         HiddenFlag(Hidden), Formatting(NormalFormatting), Misc(0),
-        FullyInitialized(false), Position(0), AdditionalVals(0) {
-    Categories.push_back(&GeneralCategory);
-  }
+        FullyInitialized(false), Position(0), AdditionalVals(0) {}
 
   inline void setNumAdditionalVals(unsigned n) { AdditionalVals = n; }
 
 public:
   virtual ~Option() = default;
 
   // addArgument - Register this argument with the commandline system.
   //
-  void addArgument();
+  virtual void addArgument(SubCommand &SC);
+
+  // addArgument - Only called in done() method to add default
+  // TopLevelSubCommand.
+  void addArgument() {
+    if (!FullyInitialized)
+      addArgument(*TopLevelSubCommand);
+  }
 
   /// Unregisters this option from the CommandLine system.
   ///
   /// This option must have been the last option registered.
   /// For testing purposes only.
   void removeArgument();
 
   // Return the width of the option tag for printing...

 };
 
 // sub - Specify the subcommand that this option belongs to.
 struct sub {
   SubCommand &Sub;
 
   sub(SubCommand &S) : Sub(S) {}
 
-  template <class Opt> void apply(Opt &O) const { O.addSubCommand(Sub); }
+  template <class Opt> void apply(Opt &O) const { O.addArgument(Sub); }
 };
 
 //===----------------------------------------------------------------------===//
 // OptionValue class
 
 // Support value comparison outside the template.
 struct GenericOptionValue {
   virtual bool compare(const GenericOptionValue &V) const = 0;
ValueExpected getValueExpectedFlagDefault() const override {
     return AliasFor->getValueExpectedFlag();
   }
 
   void done() {
     if (!hasArgStr())
       error("cl::alias must have argument name specified!");
     if (!AliasFor)
       error("cl::alias must have an cl::aliasopt(option) specified!");
-    if (!Subs.empty())
-      error("cl::alias must not have cl::sub(), aliased option's cl::sub() will be used!");
-    Subs = AliasFor->Subs;
-    Categories = AliasFor->Categories;
-    addArgument();
+    for(OptionCategory *Cat: AliasFor->getCategories())
+      addCategory(*Cat);
+    for(SubCommand *SC: AliasFor->getSubCommands())
+      Option::addArgument(*SC);
   }
 
 public:
   // Command line options should not be copyable
   alias(const alias &) = delete;
   alias &operator=(const alias &) = delete;
 
   void setAliasFor(Option &O) {
     if (AliasFor)
       error("cl::alias must only have one cl::aliasopt(...) specified!");
     AliasFor = &O;
   }
 
+  // Does nothing when called via apply.  Aliases call Option::addArgument
+  // directly in the done() method to actually add the option..
+  void addArgument(SubCommand &SC) override {}
+
   template <class... Mods>
   explicit alias(const Mods &... Ms)
       : Option(Optional, Hidden), AliasFor(nullptr) {
     apply(this, Ms...);
     done();
   }
 };
 

llvm/lib/Support/CommandLine.cpp

public:
   StringRef ProgramOverview;
 
   // This collects additional help to be printed.
   std::vector<StringRef> MoreHelp;
 
   // This collects Options added with the cl::DefaultOption flag. Since they can
   // be overridden, they are not added to the appropriate SubCommands until
   // ParseCommandLineOptions actually runs.
-  SmallVector<Option*, 4> DefaultOptions;
+  SmallVector<std::pair<Option*, SubCommand*>, 4> DefaultOptions;
 
   // This collects the different option categories that have been registered.
   SmallPtrSet<OptionCategory *, 16> RegisteredOptionCategories;
 
   // This collects the different subcommands that have been registered.
   SmallPtrSet<SubCommand *, 4> RegisteredSubCommands;
 
   CommandLineParser() : ActiveSubCommand(nullptr) {
if (SC == &*AllSubCommands) {
         if (SC == Sub)
           continue;
         addLiteralOption(Opt, Sub, Name);
       }
     }
   }
 
   void addLiteralOption(Option &Opt, StringRef Name) {
-    if (Opt.Subs.empty())
-      addLiteralOption(Opt, &*TopLevelSubCommand, Name);
-    else {
-      for (auto SC : Opt.Subs)
-        addLiteralOption(Opt, SC, Name);
-    }
+    for(SubCommand *SC: Opt.getSubCommands())
+
+  void addOption(Option *O, SubCommand *SC, bool ProcessDefaultOptions = false) {
+    if (!ProcessDefaultOptions && O->isDefaultOption()) {
+      DefaultOptions.push_back(std::make_pair(O, SC));
+      return;
     }
 
-  void addOption(Option *O, SubCommand *SC) {
     bool HadErrors = false;
     if (O->hasArgStr()) {
       // If it's a DefaultOption, check to make sure it isn't already there.
       if (O->isDefaultOption() &&
           SC->OptionsMap.find(O->ArgStr) != SC->OptionsMap.end())
         return;
 
       // Add argument to the argument map!
if (HadErrors)
       report_fatal_error("inconsistency in registered CommandLine options");
 
     // If we're adding this to all sub-commands, add it to the ones that have
     // already been registered.
     if (SC == &*AllSubCommands) {
       for (const auto &Sub : RegisteredSubCommands) {
         if (SC == Sub)
           continue;
-        addOption(O, Sub);
+        addOption(O, Sub, ProcessDefaultOptions);
       }
     }
   }
 
-  void addOption(Option *O, bool ProcessDefaultOption = false) {
-    if (!ProcessDefaultOption && O->isDefaultOption()) {
-      DefaultOptions.push_back(O);
-      return;
-    }
-
-    if (O->Subs.empty()) {
-      addOption(O, &*TopLevelSubCommand);
-    } else {
-      for (auto SC : O->Subs)
-        addOption(O, SC);
+  void addDefaultOptions() {
+    for (std::pair<Option *, SubCommand *> &DO : DefaultOptions) {
+      addOption(DO.first, DO.second, true);
     }
   }
 
   void removeOption(Option *O, SubCommand *SC) {
     SmallVector<StringRef, 16> OptionNames;
     O->getExtraOptionNames(OptionNames);
     if (O->hasArgStr())
       OptionNames.push_back(O->ArgStr);
else if (O->getMiscFlags() & cl::Sink)
           break;
         }
       }
     else if (O == Sub.ConsumeAfterOpt)
       Sub.ConsumeAfterOpt = nullptr;
   }
 
   void removeOption(Option *O) {
-    if (O->Subs.empty())
-      removeOption(O, &*TopLevelSubCommand);
-    else {
-      if (O->isInAllSubCommands()) {
-        for (auto SC : RegisteredSubCommands)
-          removeOption(O, SC);
-      } else {
-        for (auto SC : O->Subs)
-          removeOption(O, SC);
-      }
-    }
   }
 
   bool hasOptions(const SubCommand &Sub) const {
     return (!Sub.OptionsMap.empty() || !Sub.PositionalOpts.empty() ||
             nullptr != Sub.ConsumeAfterOpt);
   }
 
   bool hasOptions() const {
if (!Sub.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");
     }
     Sub.OptionsMap.erase(O->ArgStr);
   }
 
   void updateArgStr(Option *O, StringRef NewName) {
-    if (O->Subs.empty())
-      updateArgStr(O, NewName, &*TopLevelSubCommand);
-    else {
-      if (O->isInAllSubCommands()) {
-        for (auto SC : RegisteredSubCommands)
-          updateArgStr(O, NewName, SC);
-      } else {
-        for (auto SC : O->Subs)
-          updateArgStr(O, NewName, SC);
-      }
-    }
   }
 
   void printOptionValues();
 
   void registerCategory(OptionCategory *cat) {
     assert(count_if(RegisteredOptionCategories,
                     [cat](const OptionCategory *Category) {
              return cat->getName() == Category->getName();

 void cl::AddLiteralOption(Option &O, StringRef Name) {
   GlobalParser->addLiteralOption(O, Name);
 }
 
 extrahelp::extrahelp(StringRef Help) : morehelp(Help) {
   GlobalParser->MoreHelp.push_back(Help);
 }
 
-void Option::addArgument() {
-  GlobalParser->addOption(this);
+void Option::addArgument(SubCommand &SC) {
+  GlobalParser->addOption(this, &SC);
   FullyInitialized = true;
 }
 
 void Option::removeArgument() { GlobalParser->removeOption(this); }
 
+SmallPtrSet<OptionCategory *, 1> Option::getCategories() const {
+  SmallPtrSet<OptionCategory *, 1> Cats;
+  for (OptionCategory *C: GlobalParser->RegisteredOptionCategories) {
+    if (C->MemberOptions.find(this) != C->MemberOptions.end())
+      Cats.insert(C);
+  }
+  if (Cats.empty())
+    Cats.insert(&GeneralCategory);
+  return Cats;
+}
+
+SmallPtrSet<SubCommand *, 1> Option::getSubCommands() const {
+  // This can happen for enums and literal options.
+  if (ArgStr.empty())
+    return SmallPtrSet<SubCommand *, 1>{&*TopLevelSubCommand};
+
+  SmallPtrSet<SubCommand *, 1> Subs;
+  for (SubCommand *SC : GlobalParser->getRegisteredSubcommands()) {
+    auto I = SC->OptionsMap.find(ArgStr);
+    if (I != SC->OptionsMap.end() && I->getValue() == this)
+      Subs.insert(SC);
+  }
+  return Subs;
+}
+
 void Option::setArgStr(StringRef S) {
   if (FullyInitialized)
     GlobalParser->updateArgStr(this, S);
   assert((S.empty() || S[0] != '-') && "Option can't start with '-");
   ArgStr = S;
   if (ArgStr.size() == 1)
     setMiscFlag(Grouping);
 }
 
 void Option::addCategory(OptionCategory &C) {
-  assert(!Categories.empty() && "Categories cannot be empty.");
-  // Maintain backward compatibility by replacing the default GeneralCategory
-  // if it's still set.  Otherwise, just add the new one.  The GeneralCategory
-  // must be explicitly added if you want multiple categories that include it.
-  if (&C != &GeneralCategory && Categories[0] == &GeneralCategory)
-    Categories[0] = &C;
-  else if (find(Categories, &C) == Categories.end())
-    Categories.push_back(&C);
+  C.MemberOptions.insert(this);
 }
 
 void Option::reset() {
   NumOccurrences = 0;
   setDefault();
   if (isDefaultOption())
     removeArgument();
 }
bool CommandLineParser::ParseCommandLineOptions(int argc,
   GlobalParser->ActiveSubCommand = ChosenSubCommand;
 
   assert(ChosenSubCommand);
   auto &ConsumeAfterOpt = ChosenSubCommand->ConsumeAfterOpt;
   auto &PositionalOpts = ChosenSubCommand->PositionalOpts;
   auto &SinkOpts = ChosenSubCommand->SinkOpts;
   auto &OptionsMap = ChosenSubCommand->OptionsMap;
 
-  for (auto O: DefaultOptions) {
-    addOption(O, true);
-  }
+  addDefaultOptions();
 
   if (ConsumeAfterOpt) {
     assert(PositionalOpts.size() > 0 &&
            "Cannot specify cl::ConsumeAfter without a positional argument!");
   }
   if (!PositionalOpts.empty()) {
 
     // Calculate how many positional values are _required_.
for (std::vector<OptionCategory *>::const_iterator
          I != E; ++I)
       CategorizedOptions[*I] = std::vector<Option *>();
 
     // Walk through pre-sorted options and assign into categories.
     // Because the options are already alphabetically sorted the
     // options within categories will also be alphabetically sorted.
     for (size_t I = 0, E = Opts.size(); I != E; ++I) {
       Option *Opt = Opts[I].second;
-      for (auto &Cat : Opt->Categories) {
+      for (auto *Cat : Opt->getCategories()) {
         assert(CategorizedOptions.count(Cat) > 0 &&
                "Option has an unregistered category");
         CategorizedOptions[Cat].push_back(Opt);
       }
     }
 
     // Now do printing.
     for (std::vector<OptionCategory *>::const_iterator

 
 iterator_range<typename SmallPtrSet<SubCommand *, 4>::iterator>
 cl::getRegisteredSubcommands() {
   return GlobalParser->getRegisteredSubcommands();
 }
 
 void cl::HideUnrelatedOptions(cl::OptionCategory &Category, SubCommand &Sub) {
   for (auto &I : Sub.OptionsMap) {
-    for (auto &Cat : I.second->Categories) {
+    for (OptionCategory *Cat : I.second->getCategories()) {
       if (Cat != &Category &&
           Cat != &GenericCategory)
         I.second->setHiddenFlag(cl::ReallyHidden);
     }
   }
 }
 
 void cl::HideUnrelatedOptions(ArrayRef<const cl::OptionCategory *> Categories,
                               SubCommand &Sub) {
   for (auto &I : Sub.OptionsMap) {
-    for (auto &Cat : I.second->Categories) {
+    for (OptionCategory *Cat : I.second->getCategories()) {
       if (find(Categories, Cat) == Categories.end() && Cat != &GenericCategory)
         I.second->setHiddenFlag(cl::ReallyHidden);
     }
   }
 }
 
 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),
                                     &llvm::nulls());
 }

llvm/unittests/Support/CommandLineTest.cpp

StringMap<cl::Option *> &Map =
       cl::getRegisteredOptions(*cl::TopLevelSubCommand);
 
   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_NE(Retrieved->Categories.end(),
-            find_if(Retrieved->Categories,
+  ASSERT_NE(Retrieved->getCategories().end(),
+            find_if(Retrieved->getCategories(),
                     [&](const llvm::cl::OptionCategory *Cat) {
                       return Cat == &cl::GeneralCategory;
                     }))
       << "Incorrect default option category.";
 
   Retrieved->addCategory(TestCategory);
-  ASSERT_NE(Retrieved->Categories.end(),
-            find_if(Retrieved->Categories,
+  ASSERT_NE(Retrieved->getCategories().end(),
+            find_if(Retrieved->getCategories(),
                     [&](const llvm::cl::OptionCategory *Cat) {
                       return Cat == &TestCategory;
                     }))
       << "Failed to modify option's option category.";
 
   Retrieved->setDescription(Description);
   ASSERT_STREQ(Retrieved->HelpStr.data(), Description)
       << "Changing option description failed.";
TEST(CommandLineTest, ParseEnvironmentToLocalVar) {
   EXPECT_EQ("hello-local", EnvironmentTestOptionLocal);
 }
 
 #endif  // SKIP_ENVIRONMENT_TESTS
 
 TEST(CommandLineTest, UseOptionCategory) {
   StackOption<int> TestOption2("test-option", cl::cat(TestCategory));
 
-  ASSERT_NE(TestOption2.Categories.end(),
-            find_if(TestOption2.Categories,
+  ASSERT_NE(TestOption2.getCategories().end(),
+            find_if(TestOption2.getCategories(),
                          [&](const llvm::cl::OptionCategory *Cat) {
                            return Cat == &TestCategory;
                          }))
       << "Failed to assign Option Category.";
 }
 
 TEST(CommandLineTest, UseMultipleCategories) {
   StackOption<int> TestOption2("test-option2", cl::cat(TestCategory),
                                cl::cat(cl::GeneralCategory),
                                cl::cat(cl::GeneralCategory));
+  auto TestOption2Categories = TestOption2.getCategories();
 
   // Make sure cl::GeneralCategory wasn't added twice.
-  ASSERT_EQ(TestOption2.Categories.size(), 2U);
+  ASSERT_EQ(TestOption2Categories.size(), 2U);
 
-  ASSERT_NE(TestOption2.Categories.end(),
-            find_if(TestOption2.Categories,
+  ASSERT_NE(TestOption2Categories.end(),
+            find_if(TestOption2Categories,
                          [&](const llvm::cl::OptionCategory *Cat) {
                            return Cat == &TestCategory;
                          }))
       << "Failed to assign Option Category.";
-  ASSERT_NE(TestOption2.Categories.end(),
-            find_if(TestOption2.Categories,
+  ASSERT_NE(TestOption2Categories.end(),
+            find_if(TestOption2Categories,
                          [&](const llvm::cl::OptionCategory *Cat) {
                            return Cat == &cl::GeneralCategory;
                          }))
       << "Failed to assign General Category.";
 
   cl::OptionCategory AnotherCategory("Additional test Options", "Description");
   StackOption<int> TestOption("test-option", cl::cat(TestCategory),
                               cl::cat(AnotherCategory));
-  ASSERT_EQ(TestOption.Categories.end(),
-            find_if(TestOption.Categories,
+  auto TestOptionCategories = TestOption.getCategories();
+  ASSERT_EQ(TestOptionCategories.end(),
+            find_if(TestOptionCategories,
                          [&](const llvm::cl::OptionCategory *Cat) {
                            return Cat == &cl::GeneralCategory;
                          }))
       << "Failed to remove General Category.";
-  ASSERT_NE(TestOption.Categories.end(),
-            find_if(TestOption.Categories,
+  ASSERT_NE(TestOptionCategories.end(),
+            find_if(TestOptionCategories,
                          [&](const llvm::cl::OptionCategory *Cat) {
                            return Cat == &TestCategory;
                          }))
       << "Failed to assign Option Category.";
-  ASSERT_NE(TestOption.Categories.end(),
-            find_if(TestOption.Categories,
+  ASSERT_NE(TestOptionCategories.end(),
+            find_if(TestOptionCategories,
                          [&](const llvm::cl::OptionCategory *Cat) {
                            return Cat == &AnotherCategory;
                          }))
       << "Failed to assign Another Category.";
 }
 
 typedef void ParserFunction(StringRef Source, StringSaver &Saver,
                             SmallVectorImpl<const char *> &NewArgv,

 
 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);
 }
 
+TEST(CommandLineTest, AliasWithSubCommand) {
+  StackSubCommand SC1("sc1", "Subcommand 1");
+  StackOption<std::string> Option1("option", cl::value_desc("output file"),
+                                   cl::init("-"), cl::desc("Option"),
+                                   cl::sub(SC1));
+  StackOption<std::string, cl::alias> Alias1("o", llvm::cl::aliasopt(Option1),
+                                             cl::desc("Alias for --option"),
+                                             cl::sub(SC1));
+}
+
+TEST(CommandLineTest, AliasWithMultipleSubCommandsWithSameOption) {
+  StackSubCommand SC1("sc1", "Subcommand 1");
+  StackOption<std::string> Option1("option", cl::value_desc("output file"),
+                                   cl::init("-"), cl::desc("Option"),
+                                   cl::sub(SC1));
+  StackSubCommand SC2("sc2", "Subcommand 2");
+  StackOption<std::string> Option2("option", cl::value_desc("output file"),
+                                   cl::init("-"), cl::desc("Option"),
+                                   cl::sub(SC2));
+
+  StackOption<std::string, cl::alias> Alias1("o", llvm::cl::aliasopt(Option1),
+                                             cl::desc("Alias for --option"));
+}
+
 TEST(CommandLineTest, HideUnrelatedOptions) {
   StackOption<int> TestOption1("hide-option-1");
   StackOption<int> TestOption2("hide-option-2", cl::cat(TestCategory));
 
   cl::HideUnrelatedOptions(TestCategory);
 
   ASSERT_EQ(cl::ReallyHidden, TestOption1.getOptionHiddenFlag())
       << "Failed to hide extra option.";