From b88308b1b4813e55ce8f54ceff6e57736328fb58 Mon Sep 17 00:00:00 2001 From: Mingming Liu Date: Fri, 10 Nov 2023 09:22:00 -0800 Subject: [PATCH] [Support]Look up in top-level subcommand as a fallback when looking options for a custom subcommand. (#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 into a special 'top level' subcommand. **Motivating Use Case:** - The motivating use case is to refactor `llvm-profdata` to use `cl::SubCommand` to organize subcommands. See https://github.com/llvm/llvm-project/pull/71328. A valid use case that's not supported before this patch is shown below ``` // 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 | 53 ++++++++++++++++++++++ 2 files changed, 60 insertions(+) diff --git a/llvm/lib/Support/CommandLine.cpp b/llvm/lib/Support/CommandLine.cpp index 55633d7cafa47..a7e0cae8b855d 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 41cc8260acfed..4411ed0f83928 100644 --- a/llvm/unittests/Support/CommandLineTest.cpp +++ b/llvm/unittests/Support/CommandLineTest.cpp @@ -525,6 +525,59 @@ TEST(CommandLineTest, LookupFailsInWrongSubCommand) { EXPECT_FALSE(Errs.empty()); } +TEST(CommandLineTest, TopLevelOptInSubcommand) { + 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 TopLevelOpt("str", cl::init("txt"), + cl::desc("A top-level option.")); + + StackSubCommand SC("sc", "Subcommand"); + StackOption PositionalOpt( + cl::Positional, cl::desc("positional argument test coverage"), + cl::sub(SC)); + StackOption 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 EnableOpt("enable", cl::sub(SC), cl::init(false)); + StackOption ThresholdOpt("threshold", cl::sub(SC), cl::init(1)); + + const char *PositionalOptVal = "input-file"; + const char *args[] = {"prog", "sc", PositionalOptVal, + "-enable", "--str=csv", "--threshold=2"}; + + // cl::ParseCommandLineOptions returns true on success. Otherwise, it will + // print the error message to stderr and exit in this setting (`Errs` ostream + // is not set). + ASSERT_TRUE(cl::ParseCommandLineOptions(sizeof(args) / sizeof(args[0]), args, + StringRef())); + EXPECT_STREQ(PositionalOpt.getValue().c_str(), PositionalOptVal); + EXPECT_TRUE(EnableOpt); + // Tests that the value of `str` option is `csv` as specified. + EXPECT_STREQ(TopLevelOpt.getValue().c_str(), "csv"); + EXPECT_EQ(ThresholdOpt, 2); + + for (auto &[LiteralOptVal, WantLiteralOpt] : + {std::pair{"--bar", bar}, {"--foo", foo}, {"--baz", baz}}) { + const char *args[] = {"prog", "sc", LiteralOptVal}; + ASSERT_TRUE(cl::ParseCommandLineOptions(sizeof(args) / sizeof(args[0]), + args, StringRef())); + + // Tests that literal options are parsed correctly. + EXPECT_EQ(LiteralOpt, WantLiteralOpt); + } +} + TEST(CommandLineTest, AddToAllSubCommands) { cl::ResetCommandLineParser();