Skip to content

Commit 9f8c818

Browse files
authored
[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.
1 parent 85b7d54 commit 9f8c818

File tree

2 files changed

+18
-16
lines changed

2 files changed

+18
-16
lines changed

llvm/lib/Support/CommandLine.cpp

Lines changed: 13 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -164,10 +164,7 @@ class CommandLineParser {
164164
// This collects the different subcommands that have been registered.
165165
SmallPtrSet<SubCommand *, 4> RegisteredSubCommands;
166166

167-
CommandLineParser() {
168-
registerSubCommand(&SubCommand::getTopLevel());
169-
registerSubCommand(&SubCommand::getAll());
170-
}
167+
CommandLineParser() { registerSubCommand(&SubCommand::getTopLevel()); }
171168

172169
void ResetAllOptionOccurrences();
173170

@@ -183,6 +180,7 @@ class CommandLineParser {
183180
if (Opt.Subs.size() == 1 && *Opt.Subs.begin() == &SubCommand::getAll()) {
184181
for (auto *SC : RegisteredSubCommands)
185182
Action(*SC);
183+
Action(SubCommand::getAll());
186184
return;
187185
}
188186
for (auto *SC : Opt.Subs) {
@@ -348,15 +346,15 @@ class CommandLineParser {
348346

349347
// For all options that have been registered for all subcommands, add the
350348
// option to this subcommand now.
351-
if (sub != &SubCommand::getAll()) {
352-
for (auto &E : SubCommand::getAll().OptionsMap) {
353-
Option *O = E.second;
354-
if ((O->isPositional() || O->isSink() || O->isConsumeAfter()) ||
355-
O->hasArgStr())
356-
addOption(O, sub);
357-
else
358-
addLiteralOption(*O, sub, E.first());
359-
}
349+
assert(sub != &SubCommand::getAll() &&
350+
"SubCommand::getAll() should not be registered");
351+
for (auto &E : SubCommand::getAll().OptionsMap) {
352+
Option *O = E.second;
353+
if ((O->isPositional() || O->isSink() || O->isConsumeAfter()) ||
354+
O->hasArgStr())
355+
addOption(O, sub);
356+
else
357+
addLiteralOption(*O, sub, E.first());
360358
}
361359
}
362360

@@ -384,7 +382,6 @@ class CommandLineParser {
384382
SubCommand::getTopLevel().reset();
385383
SubCommand::getAll().reset();
386384
registerSubCommand(&SubCommand::getTopLevel());
387-
registerSubCommand(&SubCommand::getAll());
388385

389386
DefaultOptions.clear();
390387
}
@@ -532,8 +529,8 @@ SubCommand *CommandLineParser::LookupSubCommand(StringRef Name,
532529
// Find a subcommand with the edit distance == 1.
533530
SubCommand *NearestMatch = nullptr;
534531
for (auto *S : RegisteredSubCommands) {
535-
if (S == &SubCommand::getAll())
536-
continue;
532+
assert(S != &SubCommand::getAll() &&
533+
"SubCommand::getAll() is not expected in RegisteredSubCommands");
537534
if (S->getName().empty())
538535
continue;
539536

llvm/unittests/Support/CommandLineTest.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -585,6 +585,11 @@ TEST(CommandLineTest, AddToAllSubCommands) {
585585
cl::init(false));
586586
StackSubCommand SC2("sc2", "Second subcommand");
587587

588+
EXPECT_TRUE(cl::SubCommand::getTopLevel().OptionsMap.contains("everywhere"));
589+
EXPECT_TRUE(cl::SubCommand::getAll().OptionsMap.contains("everywhere"));
590+
EXPECT_TRUE(SC1.OptionsMap.contains("everywhere"));
591+
EXPECT_TRUE(SC2.OptionsMap.contains("everywhere"));
592+
588593
const char *args[] = {"prog", "-everywhere"};
589594
const char *args2[] = {"prog", "sc1", "-everywhere"};
590595
const char *args3[] = {"prog", "sc2", "-everywhere"};

0 commit comments

Comments
 (0)