From 1c4cbb5f06604c76b3170cefe19f9424051ca2e0 Mon Sep 17 00:00:00 2001 From: Igor Kudrin Date: Wed, 13 Dec 2023 20:01:34 -0800 Subject: [PATCH] [CommandLine][NFCI] Simplify enumerating subcommands of an option 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. --- llvm/include/llvm/Support/CommandLine.h | 4 -- llvm/lib/Support/CommandLine.cpp | 79 ++++++++----------------- 2 files changed, 24 insertions(+), 59 deletions(-) diff --git a/llvm/include/llvm/Support/CommandLine.h b/llvm/include/llvm/Support/CommandLine.h index 58ef176551b68..5d733eeee6d39 100644 --- a/llvm/include/llvm/Support/CommandLine.h +++ b/llvm/include/llvm/Support/CommandLine.h @@ -314,10 +314,6 @@ class Option { return getNumOccurrencesFlag() == cl::ConsumeAfter; } - bool isInAllSubCommands() const { - return Subs.contains(&SubCommand::getAll()); - } - //-------------------------------------------------------------------------=== // Accessor functions set by OptionModifiers // diff --git a/llvm/lib/Support/CommandLine.cpp b/llvm/lib/Support/CommandLine.cpp index 088b4e4d755cb..00179bc32551f 100644 --- a/llvm/lib/Support/CommandLine.cpp +++ b/llvm/lib/Support/CommandLine.cpp @@ -175,6 +175,24 @@ class CommandLineParser { StringRef Overview, raw_ostream *Errs = nullptr, bool LongOptionsUseDoubleDash = false); + void forEachSubCommand(Option &Opt, + std::function Action) { + if (Opt.Subs.empty()) { + Action(SubCommand::getTopLevel()); + return; + } + if (Opt.Subs.size() == 1 && *Opt.Subs.begin() == &SubCommand::getAll()) { + for (auto *SC : RegisteredSubCommands) + Action(*SC); + return; + } + for (auto *SC : Opt.Subs) { + assert(SC != &SubCommand::getAll() && + "SubCommand::getAll() should not be used with other subcommands"); + Action(*SC); + } + } + void addLiteralOption(Option &Opt, SubCommand *SC, StringRef Name) { if (Opt.hasArgStr()) return; @@ -183,25 +201,11 @@ class CommandLineParser { << "' registered more than once!\n"; report_fatal_error("inconsistency in registered CommandLine options"); } - - // If we're adding this to all sub-commands, add it to the ones that have - // already been registered. - if (SC == &SubCommand::getAll()) { - for (auto *Sub : RegisteredSubCommands) { - if (SC == Sub) - continue; - addLiteralOption(Opt, Sub, Name); - } - } } void addLiteralOption(Option &Opt, StringRef Name) { - if (Opt.Subs.empty()) - addLiteralOption(Opt, &SubCommand::getTopLevel(), Name); - else { - for (auto *SC : Opt.Subs) - addLiteralOption(Opt, SC, Name); - } + forEachSubCommand( + Opt, [&](SubCommand &SC) { addLiteralOption(Opt, &SC, Name); }); } void addOption(Option *O, SubCommand *SC) { @@ -238,16 +242,6 @@ class CommandLineParser { // linked LLVM distribution. if (HadErrors) report_fatal_error("inconsistency in registered CommandLine options"); - - // If we're adding this to all sub-commands, add it to the ones that have - // already been registered. - if (SC == &SubCommand::getAll()) { - for (auto *Sub : RegisteredSubCommands) { - if (SC == Sub) - continue; - addOption(O, Sub); - } - } } void addOption(Option *O, bool ProcessDefaultOption = false) { @@ -255,13 +249,7 @@ class CommandLineParser { DefaultOptions.push_back(O); return; } - - if (O->Subs.empty()) { - addOption(O, &SubCommand::getTopLevel()); - } else { - for (auto *SC : O->Subs) - addOption(O, SC); - } + forEachSubCommand(*O, [&](SubCommand &SC) { addOption(O, &SC); }); } void removeOption(Option *O, SubCommand *SC) { @@ -298,17 +286,7 @@ class CommandLineParser { } void removeOption(Option *O) { - if (O->Subs.empty()) - removeOption(O, &SubCommand::getTopLevel()); - else { - if (O->isInAllSubCommands()) { - for (auto *SC : RegisteredSubCommands) - removeOption(O, SC); - } else { - for (auto *SC : O->Subs) - removeOption(O, SC); - } - } + forEachSubCommand(*O, [&](SubCommand &SC) { removeOption(O, &SC); }); } bool hasOptions(const SubCommand &Sub) const { @@ -344,17 +322,8 @@ class CommandLineParser { } void updateArgStr(Option *O, StringRef NewName) { - if (O->Subs.empty()) - updateArgStr(O, NewName, &SubCommand::getTopLevel()); - else { - if (O->isInAllSubCommands()) { - for (auto *SC : RegisteredSubCommands) - updateArgStr(O, NewName, SC); - } else { - for (auto *SC : O->Subs) - updateArgStr(O, NewName, SC); - } - } + forEachSubCommand(*O, + [&](SubCommand &SC) { updateArgStr(O, NewName, &SC); }); } void printOptionValues();