[llvm] 9f8c818 - [CommandLine][NFCI] Do not add 'All' to 'RegisteredSubCommands' (#77722)

[via llvm-commits]

Author: Igor Kudrin
Date: 2024-01-13T02:19:42+07:00
New Revision: 9f8c818141720c1bb69f9e023396a9aa0733ccf6

URL: https://github.com/llvm/llvm-project/commit/9f8c818141720c1bb69f9e023396a9aa0733ccf6
DIFF: https://github.com/llvm/llvm-project/commit/9f8c818141720c1bb69f9e023396a9aa0733ccf6.diff

LOG: [CommandLine][NFCI] Do not add 'All' to 'RegisteredSubCommands' (#77722)

After #75679, it is no longer necessary to add the `All` pseudo
subcommand to the list of registered subcommands. The change causes the
list to contain only real subcommands, i.e. an unnamed top-level
subcommand and named ones. This simplifies the code a bit by removing
some checks for this special case.

This is a fixed version of #77041, where options of the 'All' subcommand
were not added to subcommands defined after them.

Added: 
    

Modified: 
    llvm/lib/Support/CommandLine.cpp
    llvm/unittests/Support/CommandLineTest.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Support/CommandLine.cpp b/llvm/lib/Support/CommandLine.cpp
index 7360d733d96e7b..553669ce8ee37c 100644
--- a/llvm/lib/Support/CommandLine.cpp
+++ b/llvm/lib/Support/CommandLine.cpp
@@ -164,10 +164,7 @@ class CommandLineParser {
   // This collects the 
diff erent subcommands that have been registered.
   SmallPtrSet<SubCommand *, 4> RegisteredSubCommands;
 
-  CommandLineParser() {
-    registerSubCommand(&SubCommand::getTopLevel());
-    registerSubCommand(&SubCommand::getAll());
-  }
+  CommandLineParser() { registerSubCommand(&SubCommand::getTopLevel()); }
 
   void ResetAllOptionOccurrences();
 
@@ -183,6 +180,7 @@ class CommandLineParser {
     if (Opt.Subs.size() == 1 && *Opt.Subs.begin() == &SubCommand::getAll()) {
       for (auto *SC : RegisteredSubCommands)
         Action(*SC);
+      Action(SubCommand::getAll());
       return;
     }
     for (auto *SC : Opt.Subs) {
@@ -348,15 +346,15 @@ class CommandLineParser {
 
     // For all options that have been registered for all subcommands, add the
     // option to this subcommand now.
-    if (sub != &SubCommand::getAll()) {
-      for (auto &E : SubCommand::getAll().OptionsMap) {
-        Option *O = E.second;
-        if ((O->isPositional() || O->isSink() || O->isConsumeAfter()) ||
-            O->hasArgStr())
-          addOption(O, sub);
-        else
-          addLiteralOption(*O, sub, E.first());
-      }
+    assert(sub != &SubCommand::getAll() &&
+           "SubCommand::getAll() should not be registered");
+    for (auto &E : SubCommand::getAll().OptionsMap) {
+      Option *O = E.second;
+      if ((O->isPositional() || O->isSink() || O->isConsumeAfter()) ||
+          O->hasArgStr())
+        addOption(O, sub);
+      else
+        addLiteralOption(*O, sub, E.first());
     }
   }
 
@@ -384,7 +382,6 @@ class CommandLineParser {
     SubCommand::getTopLevel().reset();
     SubCommand::getAll().reset();
     registerSubCommand(&SubCommand::getTopLevel());
-    registerSubCommand(&SubCommand::getAll());
 
     DefaultOptions.clear();
   }
@@ -532,8 +529,8 @@ SubCommand *CommandLineParser::LookupSubCommand(StringRef Name,
   // Find a subcommand with the edit distance == 1.
   SubCommand *NearestMatch = nullptr;
   for (auto *S : RegisteredSubCommands) {
-    if (S == &SubCommand::getAll())
-      continue;
+    assert(S != &SubCommand::getAll() &&
+           "SubCommand::getAll() is not expected in RegisteredSubCommands");
     if (S->getName().empty())
       continue;
 

diff  --git a/llvm/unittests/Support/CommandLineTest.cpp b/llvm/unittests/Support/CommandLineTest.cpp
index 99d99c74c128b0..bbeb9d5dc19bda 100644
--- a/llvm/unittests/Support/CommandLineTest.cpp
+++ b/llvm/unittests/Support/CommandLineTest.cpp
@@ -585,6 +585,11 @@ TEST(CommandLineTest, AddToAllSubCommands) {
                            cl::init(false));
   StackSubCommand SC2("sc2", "Second subcommand");
 
+  EXPECT_TRUE(cl::SubCommand::getTopLevel().OptionsMap.contains("everywhere"));
+  EXPECT_TRUE(cl::SubCommand::getAll().OptionsMap.contains("everywhere"));
+  EXPECT_TRUE(SC1.OptionsMap.contains("everywhere"));
+  EXPECT_TRUE(SC2.OptionsMap.contains("everywhere"));
+
   const char *args[] = {"prog", "-everywhere"};
   const char *args2[] = {"prog", "sc1", "-everywhere"};
   const char *args3[] = {"prog", "sc2", "-everywhere"};