[llvm] [Support]Look up in top-level subcommand as a fallback when looking options for a custom subcommand. (PR #71776)

Mingming Liu via llvm-commits llvm-commits at lists.llvm.org
Wed Nov 8 21:48:50 PST 2023


https://github.com/minglotus-6 created https://github.com/llvm/llvm-project/pull/71776

**Context:**

- In https://lists.llvm.org/pipermail/llvm-dev/2016-June/101804.html and commit 07670b3e984db32f291373fe12c392959f2aff67, cl::SubCommand is introduced.
- Options that don't specify subcommand goes intoa special 'top level' subcommand.

**Motivating Use Case:**
- The motivating use case is to refactor llvm-profdata to use cl::SubCommand to organize subcommands. See pr/71328. A valid use case that's not supported before this patch

```
  // show-option{1,2} are associated with 'show' subcommand.
  // top-level-option3 is in top-level subcomand (e.g., `profile-isfs` in SampleProfReader.cpp)
  llvm-profdata show --show-option1 --show-option2 --top-level-option3
```

- Before this patch, option handler look-up will fail with the following error message "Unknown command line argument --top-level-option3".
- After this patch, option handler look-up will look up in sub-command options first, and use top-level subcommand as a fallback, so 'top-level-option3' is parsed correctly.

>From 444a635e530b72bc3ca40dd0984c1d4c8c5da0a3 Mon Sep 17 00:00:00 2001
From: Mingming Liu <mingmingl at google.com>
Date: Wed, 8 Nov 2023 21:34:04 -0800
Subject: [PATCH] [Support]Look up in top-level subcommand as a fallback when
 looking options for a custom subcommand.

Context:

  In https://lists.llvm.org/pipermail/llvm-dev/2016-June/101804.html and commit 07670b3e984db32f291373fe12c392959f2aff67, cl::SubCommand is introduced.

  Options that don't specify subcommand goes intoa special 'top level' subcommand.

Motivating Use Case:
  The motivating use case is to refactor llvm-profdata to use cl::SubCommand to organize subcommands. See pr/71328. A valid use case that's not supported before this patch

  // show-option{1,2} are associated with 'show' subcommand.
  // top-level-option3 is in top-level subcomand (e.g., `profile-isfs` in SampleProfReader.cpp)
  llvm-profdata show --show-option1 --show-option2 --top-level-option3

  - Before this patch, option handler look-up will fail with the following error message
    "Unknown command line argument --top-level-option3".
  - After this patch, option handler look-up will look up in sub-command options first,
    and use top-level subcommand as a fallback, so 'top-level-option3' is parsed correctly.
---
 llvm/lib/Support/CommandLine.cpp           |  7 +++
 llvm/unittests/Support/CommandLineTest.cpp | 68 ++++++++++++++++++++++
 2 files changed, 75 insertions(+)

diff --git a/llvm/lib/Support/CommandLine.cpp b/llvm/lib/Support/CommandLine.cpp
index 55633d7cafa4791..a7e0cae8b855d7c 100644
--- a/llvm/lib/Support/CommandLine.cpp
+++ b/llvm/lib/Support/CommandLine.cpp
@@ -1667,6 +1667,13 @@ bool CommandLineParser::ParseCommandLineOptions(int argc,
       Handler = LookupLongOption(*ChosenSubCommand, ArgName, Value,
                                  LongOptionsUseDoubleDash, HaveDoubleDash);
 
+      // If Handler is not found in a specialized subcommand, look up handler
+      // in the top-level subcommand.
+      // cl::opt without cl::sub belongs to top-level subcommand.
+      if (!Handler && ChosenSubCommand != &SubCommand::getTopLevel())
+        Handler = LookupLongOption(SubCommand::getTopLevel(), ArgName, Value,
+                                   LongOptionsUseDoubleDash, HaveDoubleDash);
+
       // Check to see if this "option" is really a prefixed or grouped argument.
       if (!Handler && !(LongOptionsUseDoubleDash && HaveDoubleDash))
         Handler = HandlePrefixedOrGroupedOption(ArgName, Value, ErrorParsing,
diff --git a/llvm/unittests/Support/CommandLineTest.cpp b/llvm/unittests/Support/CommandLineTest.cpp
index 41cc8260acfedf7..76589c7deed88e1 100644
--- a/llvm/unittests/Support/CommandLineTest.cpp
+++ b/llvm/unittests/Support/CommandLineTest.cpp
@@ -525,6 +525,74 @@ TEST(CommandLineTest, LookupFailsInWrongSubCommand) {
   EXPECT_FALSE(Errs.empty());
 }
 
+TEST(CommandLineTest, SubcommandOptions) {
+  enum LiteralOptionEnum {
+    foo,
+    bar,
+    baz,
+  };
+
+  cl::ResetCommandLineParser();
+
+  // This is a top-level option and not associated with a subcommand.
+  // A command line using subcommand should parse both subcommand options and
+  // top-level options.  A valid use case is that users of llvm command line
+  // tools should be able to specify top-level options defined in any library.
+  cl::opt<std::string> TopLevelOpt("str", cl::init("txt"),
+                                   cl::desc("A top-level option."));
+
+  StackSubCommand SC("sc", "Subcommand");
+
+  // The positional argument.
+  StackOption<std::string> PositionalOpt(
+      cl::Positional, cl::desc("positional argument test coverage"),
+      cl::sub(SC));
+  // Thel literal argument.
+  StackOption<LiteralOptionEnum> LiteralOpt(
+      cl::desc("literal argument test coverage"), cl::sub(SC), cl::init(bar),
+      cl::values(clEnumVal(foo, "foo"), clEnumVal(bar, "bar"),
+                 clEnumVal(baz, "baz")));
+  StackOption<bool> BoolOpt("enable", cl::sub(SC), cl::init(false));
+
+  std::string Errs;
+  raw_string_ostream OS(Errs);
+
+  for (const char *literalArg : {"--bar", "--foo", "--baz"}) {
+    const char *args[] = {"prog",     "sc",      "input-file",
+                          literalArg, "-enable", "--str=csv"};
+
+    // cl::ParseCommandLineOptions returns true on success. it returns false
+    // and prints errors to caller provided error stream (&OS in this setting).
+    EXPECT_TRUE(cl::ParseCommandLineOptions(6, args, StringRef(), &OS));
+
+    // Tests that the value of `str` option is `csv` as specified.
+    EXPECT_EQ(strcmp(TopLevelOpt.getValue().c_str(), "csv"), 0);
+
+    const char *parsedLiteralOpt;
+    switch (LiteralOpt) {
+    case baz:
+      parsedLiteralOpt = "baz";
+      break;
+    case bar:
+      parsedLiteralOpt = "bar";
+      break;
+    case foo:
+      parsedLiteralOpt = "foo";
+      break;
+    default:
+      llvm_unreachable("unknown option for LiteralOpt");
+    }
+
+    // Tests that literal options are parsed correctly. Skip '--' prefix of
+    // literalArg.
+    EXPECT_EQ(strcmp(parsedLiteralOpt, literalArg + 2), 0);
+
+    // Flush and tests there is no error message.
+    OS.flush();
+    EXPECT_TRUE(Errs.empty());
+  }
+}
+
 TEST(CommandLineTest, AddToAllSubCommands) {
   cl::ResetCommandLineParser();
 



More information about the llvm-commits mailing list