Address review comments.

Author: Prabhuk

llvm/examples/OptSubcommand/llvm-hello-sub.cpp

@@ -44,8 +44,9 @@ static constexpr OptTable::Info InfoTable[] = {
 class HelloSubOptTable : public GenericOptTable {
 public:
   HelloSubOptTable()
-      : GenericOptTable(OptionStrTable, OptionPrefixesTable, InfoTable, false,
-                        OptionCommands, OptionCommandIDsTable) {}
+      : GenericOptTable(OptionStrTable, OptionPrefixesTable, InfoTable,
+                        /*IgnoreCase=*/false, OptionCommands,
+                        OptionCommandIDsTable) {}
 };
 } // namespace
 
@@ -97,9 +98,8 @@ int main(int argc, char **argv) {
     return 1;
   }
   if (Subcommand.empty()) {
-    if (Args.hasArg(OPT_version)) {
+    if (Args.hasArg(OPT_version))
       llvm::outs() << "LLVM Hello Subcommand Example 1.0\n";
-    }
   } else if (Subcommand == "foo") {
     if (Args.hasArg(OPT_uppercase))
       llvm::outs() << "FOO\n";

llvm/include/llvm/Option/OptTable.h

@@ -45,6 +45,8 @@ class Visibility {
   operator unsigned() const { return Mask; }
 };
 
+inline constexpr StringRef TopLevelCommandName = "TopLevelCommand";
+
 /// Provide access to the Option info table.
 ///
 /// The OptTable class provides a layer of indirection which allows Option

llvm/lib/Option/ArgList.cpp

@@ -218,7 +218,7 @@ StringRef ArgList::getSubcommand(
 
     size_t OldSize = SubCommands.size();
     for (const OptTable::Command &CMD : Commands) {
-      if (StringRef(CMD.Name) == "TopLevelCommand")
+      if (StringRef(CMD.Name) == opt::TopLevelCommandName)
         continue;
       if (StringRef(CMD.Name) == A->getValue())
         SubCommands.push_back(A->getValue());
@@ -227,12 +227,12 @@ StringRef ArgList::getSubcommand(
     if (SubCommands.size() == OldSize)
       OtherPositionals.push_back(A->getValue());
   }
-  if (SubCommands.size() > 1) {
+  // Invoke callbacks if necessary.
+  if (SubCommands.size() > 1)
     HandleMultipleSubcommands(SubCommands);
-  }
-  if (!OtherPositionals.empty()) {
+  if (!OtherPositionals.empty())
     HandleOtherPositionals(OtherPositionals);
-  }
+
   if (SubCommands.size() == 1)
     return SubCommands.front();
   return {}; // No valid usage of subcommand found.

llvm/lib/Option/OptTable.cpp

@@ -418,7 +418,7 @@ std::unique_ptr<Arg> OptTable::parseOneArgGrouped(InputArgList &Args,
 
 std::unique_ptr<Arg> OptTable::ParseOneArg(const ArgList &Args, unsigned &Index,
                                            Visibility VisibilityMask) const {
-  return internalParseOneArg(Args, Index, nullptr,
+  return internalParseOneArg(Args, Index, /*ActiveCommand=*/nullptr,
                              [VisibilityMask](const Option &Opt) {
                                return !Opt.hasVisibilityFlag(VisibilityMask);
                              });
@@ -428,7 +428,7 @@ std::unique_ptr<Arg> OptTable::ParseOneArg(const ArgList &Args, unsigned &Index,
                                            unsigned FlagsToInclude,
                                            unsigned FlagsToExclude) const {
   return internalParseOneArg(
-      Args, Index, nullptr,
+      Args, Index, /*ActiveCommand=*/nullptr,
       [FlagsToInclude, FlagsToExclude](const Option &Opt) {
         if (FlagsToInclude && !Opt.hasFlag(FlagsToInclude))
           return true;
@@ -766,7 +766,7 @@ void OptTable::printHelp(raw_ostream &OS, const char *Usage, const char *Title,
   bool ShowHidden = !(FlagsToExclude & HelpHidden);
   FlagsToExclude &= ~HelpHidden;
   return internalPrintHelp(
-      OS, Usage, Title, {}, ShowHidden, ShowAllAliases,
+      OS, Usage, Title, /*Subcommand=*/{}, ShowHidden, ShowAllAliases,
       [FlagsToInclude, FlagsToExclude](const Info &CandidateInfo) {
         if (FlagsToInclude && !(CandidateInfo.Flags & FlagsToInclude))
           return true;
@@ -800,7 +800,7 @@ void OptTable::internalPrintHelp(
       // This loop prints subcommands list and sets ActiveCommand to
       // TopLevelCommand while iterating over all commands.
       for (const auto &C : Commands) {
-        if (StringRef(C.Name) == "TopLevelCommand") {
+        if (StringRef(C.Name) == opt::TopLevelCommandName) {
           ActiveCommand = &C;
           continue;
         }

llvm/utils/TableGen/OptionParserEmitter.cpp

@@ -12,6 +12,7 @@
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/Twine.h"
+#include "llvm/Option/OptTable.h"
 #include "llvm/Support/InterleavedRange.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/TableGen/Record.h"
@@ -263,7 +264,7 @@ static void emitOptionParser(const RecordKeeper &Records, raw_ostream &OS) {
       Records.getAllDerivedDefinitions("Command");
   // TopLevelCommand should come first.
   std::stable_partition(Commands.begin(), Commands.end(), [](const Record *R) {
-    return R->getName() == "TopLevelCommand";
+    return R->getName() == opt::TopLevelCommandName;
   });
 
   emitSourceFileHeader("Option Parsing Definitions", OS);