From 59f4b18a903892c305631931920d6a7df2226f63 Mon Sep 17 00:00:00 2001 From: Igor Kudrin Date: Thu, 11 Jan 2024 03:45:13 +0700 Subject: [PATCH] [CommandLine][NFCI] Do not add 'All' to 'RegisteredSubCommands' 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. --- llvm/lib/Support/CommandLine.cpp | 29 ++++++++++------------ llvm/unittests/Support/CommandLineTest.cpp | 5 ++++ 2 files changed, 18 insertions(+), 16 deletions(-) diff --git a/llvm/lib/Support/CommandLine.cpp b/llvm/lib/Support/CommandLine.cpp index 7360d733d96e7..553669ce8ee37 100644 --- a/llvm/lib/Support/CommandLine.cpp +++ b/llvm/lib/Support/CommandLine.cpp @@ -164,10 +164,7 @@ class CommandLineParser { // This collects the different subcommands that have been registered. SmallPtrSet 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 99d99c74c128b..bbeb9d5dc19bd 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"};