Skip to content

Commit 6a2a99f

Browse files
authored
[CommandLine][NFCI] Simplify enumerating subcommands of an option (#75679)
The patch adds a helper method to iterate over all subcommands to which an option belongs. Duplicate code is removed and replaced with calls to this new method.
1 parent 315a5cc commit 6a2a99f

File tree

2 files changed

+24
-59
lines changed

2 files changed

+24
-59
lines changed

llvm/include/llvm/Support/CommandLine.h

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -314,10 +314,6 @@ class Option {
314314
return getNumOccurrencesFlag() == cl::ConsumeAfter;
315315
}
316316

317-
bool isInAllSubCommands() const {
318-
return Subs.contains(&SubCommand::getAll());
319-
}
320-
321317
//-------------------------------------------------------------------------===
322318
// Accessor functions set by OptionModifiers
323319
//

llvm/lib/Support/CommandLine.cpp

Lines changed: 24 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -175,6 +175,24 @@ class CommandLineParser {
175175
StringRef Overview, raw_ostream *Errs = nullptr,
176176
bool LongOptionsUseDoubleDash = false);
177177

178+
void forEachSubCommand(Option &Opt,
179+
std::function<void(SubCommand &)> Action) {
180+
if (Opt.Subs.empty()) {
181+
Action(SubCommand::getTopLevel());
182+
return;
183+
}
184+
if (Opt.Subs.size() == 1 && *Opt.Subs.begin() == &SubCommand::getAll()) {
185+
for (auto *SC : RegisteredSubCommands)
186+
Action(*SC);
187+
return;
188+
}
189+
for (auto *SC : Opt.Subs) {
190+
assert(SC != &SubCommand::getAll() &&
191+
"SubCommand::getAll() should not be used with other subcommands");
192+
Action(*SC);
193+
}
194+
}
195+
178196
void addLiteralOption(Option &Opt, SubCommand *SC, StringRef Name) {
179197
if (Opt.hasArgStr())
180198
return;
@@ -183,25 +201,11 @@ class CommandLineParser {
183201
<< "' registered more than once!\n";
184202
report_fatal_error("inconsistency in registered CommandLine options");
185203
}
186-
187-
// If we're adding this to all sub-commands, add it to the ones that have
188-
// already been registered.
189-
if (SC == &SubCommand::getAll()) {
190-
for (auto *Sub : RegisteredSubCommands) {
191-
if (SC == Sub)
192-
continue;
193-
addLiteralOption(Opt, Sub, Name);
194-
}
195-
}
196204
}
197205

198206
void addLiteralOption(Option &Opt, StringRef Name) {
199-
if (Opt.Subs.empty())
200-
addLiteralOption(Opt, &SubCommand::getTopLevel(), Name);
201-
else {
202-
for (auto *SC : Opt.Subs)
203-
addLiteralOption(Opt, SC, Name);
204-
}
207+
forEachSubCommand(
208+
Opt, [&](SubCommand &SC) { addLiteralOption(Opt, &SC, Name); });
205209
}
206210

207211
void addOption(Option *O, SubCommand *SC) {
@@ -238,30 +242,14 @@ class CommandLineParser {
238242
// linked LLVM distribution.
239243
if (HadErrors)
240244
report_fatal_error("inconsistency in registered CommandLine options");
241-
242-
// If we're adding this to all sub-commands, add it to the ones that have
243-
// already been registered.
244-
if (SC == &SubCommand::getAll()) {
245-
for (auto *Sub : RegisteredSubCommands) {
246-
if (SC == Sub)
247-
continue;
248-
addOption(O, Sub);
249-
}
250-
}
251245
}
252246

253247
void addOption(Option *O, bool ProcessDefaultOption = false) {
254248
if (!ProcessDefaultOption && O->isDefaultOption()) {
255249
DefaultOptions.push_back(O);
256250
return;
257251
}
258-
259-
if (O->Subs.empty()) {
260-
addOption(O, &SubCommand::getTopLevel());
261-
} else {
262-
for (auto *SC : O->Subs)
263-
addOption(O, SC);
264-
}
252+
forEachSubCommand(*O, [&](SubCommand &SC) { addOption(O, &SC); });
265253
}
266254

267255
void removeOption(Option *O, SubCommand *SC) {
@@ -298,17 +286,7 @@ class CommandLineParser {
298286
}
299287

300288
void removeOption(Option *O) {
301-
if (O->Subs.empty())
302-
removeOption(O, &SubCommand::getTopLevel());
303-
else {
304-
if (O->isInAllSubCommands()) {
305-
for (auto *SC : RegisteredSubCommands)
306-
removeOption(O, SC);
307-
} else {
308-
for (auto *SC : O->Subs)
309-
removeOption(O, SC);
310-
}
311-
}
289+
forEachSubCommand(*O, [&](SubCommand &SC) { removeOption(O, &SC); });
312290
}
313291

314292
bool hasOptions(const SubCommand &Sub) const {
@@ -344,17 +322,8 @@ class CommandLineParser {
344322
}
345323

346324
void updateArgStr(Option *O, StringRef NewName) {
347-
if (O->Subs.empty())
348-
updateArgStr(O, NewName, &SubCommand::getTopLevel());
349-
else {
350-
if (O->isInAllSubCommands()) {
351-
for (auto *SC : RegisteredSubCommands)
352-
updateArgStr(O, NewName, SC);
353-
} else {
354-
for (auto *SC : O->Subs)
355-
updateArgStr(O, NewName, SC);
356-
}
357-
}
325+
forEachSubCommand(*O,
326+
[&](SubCommand &SC) { updateArgStr(O, NewName, &SC); });
358327
}
359328

360329
void printOptionValues();

0 commit comments

Comments
 (0)