Unverified Commit 38b34c61 authored by Mingming Liu's avatar Mingming Liu Committed by GitHub
Browse files

Reland "[Support]Look up in top-level subcommand as a fallback when looking... 

Reland "[Support]Look up in top-level subcommand as a fallback when looking options for a custom subcommand (#71981)

Fixed build bot errors. 

- Use `StackOption<std::string>` type for the top level option. This
way, a per test-case option is unregistered when destructor of
`StackOption` cleans up state for subsequent test cases.
- Repro the crash with no test sharding `/usr/bin/python3
/path/to/llvm-project/build/./bin/llvm-lit -vv --no-gtest-sharding -j128
/path/to/llvm-project/llvm/test/Unit`. The crash is gone with the fix
(same no-sharding repro)

**Original commit message:**
**Context:**

- In https://lists.llvm.org/pipermail/llvm-dev/2016-June/101804.html and
commit
https://github.com/llvm/llvm-project/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.
parent 58bb2d19

llvm/lib/Support/CommandLine.cpp

+7 −0
Original line number Diff line number Diff line
@@ -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,

llvm/unittests/Support/CommandLineTest.cpp

+53 −0
Original line number Diff line number Diff line
@@ -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.

  StackOption<std::string> TopLevelOpt("str", cl::init("txt"),

                                       cl::desc("A top-level option."));



  StackSubCommand SC("sc", "Subcommand");

  StackOption<std::string> PositionalOpt(

      cl::Positional, cl::desc("positional argument test coverage"),

      cl::sub(SC));

  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> EnableOpt("enable", cl::sub(SC), cl::init(false));

  StackOption<int> 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();