[CommandLine] Add subcommand groups

[igorkudrin]

The patch introduces a SubCommandGroup class which represents a list of subcommands. An option can be added to all these subcommands using one cl::sub(group) command. This simplifies the declaration of options that are shared across multiple subcommands of a tool.

[llvmbot]

@llvm/pr-subscribers-llvm-support

Author: Igor Kudrin (igorkudrin)

Changes

The patch introduces a SubCommandGroup class which represents a list of subcommands. An option can be added to all these subcommands using one cl::sub(group) command. This simplifies the declaration of options that are shared across multiple subcommands of a tool.

---

Full diff: https://github.com/llvm/llvm-project/pull/75678.diff

2 Files Affected:

• (modified) llvm/include/llvm/Support/CommandLine.h (+19-3)
• (modified) llvm/unittests/Support/CommandLineTest.cpp (+63)

diff --git a/llvm/include/llvm/Support/CommandLine.h b/llvm/include/llvm/Support/CommandLine.h
index 58ef176551b685..58969c04f3bec9 100644
--- a/llvm/include/llvm/Support/CommandLine.h
+++ b/llvm/include/llvm/Support/CommandLine.h
@@ -243,6 +243,14 @@ extern ManagedStatic<SubCommand> TopLevelSubCommand;
 // A special subcommand that can be used to put an option into all subcommands.
 extern ManagedStatic<SubCommand> AllSubCommands;
 
+class SubCommandGroup {
+  SmallVector<SubCommand *, 4> Subs;
+public:
+  SubCommandGroup(std::initializer_list<SubCommand *> IL) : Subs(IL) {}
+
+  ArrayRef<SubCommand *> getSubCommands() const { return Subs; }
+};
+
 //===----------------------------------------------------------------------===//
 //
 class Option {
@@ -477,11 +485,19 @@ struct cat {
 
 // Specify the subcommand that this option belongs to.
 struct sub {
-  SubCommand &Sub;
+  SubCommand *Sub = nullptr;
+  SubCommandGroup *Group = nullptr;
 
-  sub(SubCommand &S) : Sub(S) {}
+  sub(SubCommand &S) : Sub(&S) {}
+  sub(SubCommandGroup &G) : Group(&G) {}
 
-  template <class Opt> void apply(Opt &O) const { O.addSubCommand(Sub); }
+  template <class Opt> void apply(Opt &O) const {
+    if (Sub)
+      O.addSubCommand(*Sub);
+    else if (Group)
+      for (SubCommand *SC : Group->getSubCommands())
+        O.addSubCommand(*SC);
+  }
 };
 
 // Specify a callback function to be called when an option is seen.
diff --git a/llvm/unittests/Support/CommandLineTest.cpp b/llvm/unittests/Support/CommandLineTest.cpp
index ae80490a33734a..192a7dfa6be899 100644
--- a/llvm/unittests/Support/CommandLineTest.cpp
+++ b/llvm/unittests/Support/CommandLineTest.cpp
@@ -2274,4 +2274,67 @@ TEST(CommandLineTest, UnknownCommands) {
   EXPECT_EQ(Errs, "prog: Unknown subcommand 'faz'.  Try: 'prog --help'\n");
 }
 
+TEST(CommandLineTest, SubCommandGroups) {
+  cl::ResetCommandLineParser();
+
+  StackSubCommand SC1("sc1", "SC1 subcommand");
+  StackSubCommand SC2("sc2", "SC2 subcommand");
+  StackSubCommand SC3("sc3", "SC3 subcommand");
+  cl::SubCommandGroup Group12 = {&SC1, &SC2};
+  cl::SubCommandGroup Group23 = {&SC2, &SC3};
+
+  StackOption<bool> Opt1("opt1", cl::sub(SC1), cl::init(false));
+  StackOption<bool> Opt2("opt2", cl::sub(SC2), cl::init(false));
+  StackOption<bool> Opt3("opt3", cl::sub(SC3), cl::init(false));
+  StackOption<bool> Opt12("opt12", cl::sub(Group12), cl::init(false));
+  StackOption<bool> Opt23("opt23", cl::sub(Group23), cl::init(false));
+
+  EXPECT_EQ(2, SC1.OptionsMap.size());
+  EXPECT_TRUE(SC1.OptionsMap.contains("opt1"));
+  EXPECT_TRUE(SC1.OptionsMap.contains("opt12"));
+
+  EXPECT_EQ(3, SC2.OptionsMap.size());
+  EXPECT_TRUE(SC2.OptionsMap.contains("opt2"));
+  EXPECT_TRUE(SC2.OptionsMap.contains("opt12"));
+  EXPECT_TRUE(SC2.OptionsMap.contains("opt23"));
+
+  EXPECT_EQ(2, SC3.OptionsMap.size());
+  EXPECT_TRUE(SC3.OptionsMap.contains("opt3"));
+  EXPECT_TRUE(SC3.OptionsMap.contains("opt23"));
+
+  const char *Args1[] = {"prog", "--opt1"};
+  EXPECT_FALSE(cl::ParseCommandLineOptions(std::size(Args1), Args1, StringRef(),
+                                           &llvm::nulls()));
+
+  cl::ResetAllOptionOccurrences();
+
+  const char *Args2[] = {"prog", "sc2", "--opt2"};
+  EXPECT_TRUE(cl::ParseCommandLineOptions(std::size(Args2), Args2, StringRef(),
+                                          &llvm::nulls()));
+  EXPECT_FALSE(SC1);
+  EXPECT_TRUE(SC2);
+  EXPECT_FALSE(SC3);
+
+  EXPECT_FALSE(Opt1);
+  EXPECT_TRUE(Opt2);
+  EXPECT_FALSE(Opt3);
+  EXPECT_FALSE(Opt12);
+  EXPECT_FALSE(Opt23);
+
+  cl::ResetAllOptionOccurrences();
+
+  const char *Args3[] = {"prog", "sc3", "--opt23"};
+  EXPECT_TRUE(cl::ParseCommandLineOptions(std::size(Args3), Args3, StringRef(),
+                                          &llvm::nulls()));
+  EXPECT_FALSE(SC1);
+  EXPECT_FALSE(SC2);
+  EXPECT_TRUE(SC3);
+
+  EXPECT_FALSE(Opt1);
+  EXPECT_FALSE(Opt2);
+  EXPECT_FALSE(Opt3);
+  EXPECT_FALSE(Opt12);
+  EXPECT_TRUE(Opt23);
+}
+
 } // anonymous namespace

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[jh7370]

I assume you have a concrete use case for this in mind?

[jh7370]

I think it might be slightly easier to follow the test by dividing it up into three individual tests, using a shared test fixture to do the set up per test. This then means you can name the inidivdual tests distinctly to describe the behaviour that is intended to be tested.

[igorkudrin]

I have simplified the test and added some comments. Is it better now? It doesn't seem to me that this tiny feature deserves its own test fixture.

[igorkudrin]

I assume you have a concrete use case for this in mind?

A tool can have options like 'input files", "output file", "log", etc. that are shared by all/most of its subcommands. At the same time, it is not possible to use a pseudo-subcommand cl::AllSubCommands, because the options make no sense if the tool is called without any subcommand specified. We use the subcommand groups for our downstream tool, and this helps to simplify the code a bit.

[jh7370]

LGTM.