From 58114f6cb710b10ee5d7b20600df5eda16c0f507 Mon Sep 17 00:00:00 2001 From: Igor Kudrin Date: Fri, 15 Dec 2023 10:55:49 -0800 Subject: [PATCH 1/2] [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. --- llvm/lib/Support/CommandLine.cpp | 26 ++++++++++---------------- 1 file changed, 10 insertions(+), 16 deletions(-) diff --git a/llvm/lib/Support/CommandLine.cpp b/llvm/lib/Support/CommandLine.cpp index 368dead449149..32cdff6dc7f70 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(); @@ -348,15 +345,14 @@ 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()); + 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 +380,6 @@ class CommandLineParser { SubCommand::getTopLevel().reset(); SubCommand::getAll().reset(); registerSubCommand(&SubCommand::getTopLevel()); - registerSubCommand(&SubCommand::getAll()); DefaultOptions.clear(); } @@ -532,8 +527,7 @@ 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()); if (S->getName().empty()) continue; From a20bb15f78e183e2834ff1307cc237766c68a3b4 Mon Sep 17 00:00:00 2001 From: Igor Kudrin Date: Tue, 9 Jan 2024 07:47:49 -0800 Subject: [PATCH 2/2] fixup! [CommandLine][NFCI] Do not add 'All' to 'RegisteredSubCommands' Add assertion messages --- llvm/lib/Support/CommandLine.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/llvm/lib/Support/CommandLine.cpp b/llvm/lib/Support/CommandLine.cpp index 32cdff6dc7f70..6fd86e4761372 100644 --- a/llvm/lib/Support/CommandLine.cpp +++ b/llvm/lib/Support/CommandLine.cpp @@ -345,7 +345,8 @@ class CommandLineParser { // For all options that have been registered for all subcommands, add the // option to this subcommand now. - assert(sub != &SubCommand::getAll()); + 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()) || @@ -527,7 +528,8 @@ SubCommand *CommandLineParser::LookupSubCommand(StringRef Name, // Find a subcommand with the edit distance == 1. SubCommand *NearestMatch = nullptr; for (auto *S : RegisteredSubCommands) { - assert(S != &SubCommand::getAll()); + assert(S != &SubCommand::getAll() && + "SubCommand::getAll() is not expected in RegisteredSubCommands"); if (S->getName().empty()) continue;