From 2e912a2b82dde3940e77b62832629ecf0a8b9f14 Mon Sep 17 00:00:00 2001 From: Mingming Liu Date: Fri, 10 Nov 2023 11:44:30 -0800 Subject: [PATCH] Revert "[Support]Look up in top-level subcommand as a fallback when looking options for a custom subcommand (#71975) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit …ooking options for a custom subcommand. (#71776)" This reverts commit b88308b1b4813e55ce8f54ceff6e57736328fb58. The build-bot is unhappy (https://lab.llvm.org/buildbot/#/builders/186/builds/13096), `GroupingAndPrefix` fails after `TopLevelOptInSubcommand` (the newly added test). Revert while I look into this (might be related with test sharding but not sure) ``` [----------] 3 tests from CommandLineTest [ RUN ] CommandLineTest.TokenizeWindowsCommandLine2 [ OK ] CommandLineTest.TokenizeWindowsCommandLine2 (0 ms) [ RUN ] CommandLineTest.TopLevelOptInSubcommand [ OK ] CommandLineTest.TopLevelOptInSubcommand (0 ms) [ RUN ] CommandLineTest.GroupingAndPrefix #0 0x00ba8118 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) (/home/tcwg-buildbot/worker/clang-armv7-global-isel/stage1/unittests/Support/./SupportTests+0x594118) #1 0x00ba5914 llvm::sys::RunSignalHandlers() (/home/tcwg-buildbot/worker/clang-armv7-global-isel/stage1/unittests/Support/./SupportTests+0x591914) #2 0x00ba89c4 SignalHandler(int) (/home/tcwg-buildbot/worker/clang-armv7-global-isel/stage1/unittests/Support/./SupportTests+0x5949c4) #3 0xf7828530 __default_sa_restorer /build/glibc-9MGTF6/glibc-2.31/signal/../sysdeps/unix/sysv/linux/arm/sigrestorer.S:67:0 #4 0x00af91f0 (anonymous namespace)::CommandLineParser::ResetAllOptionOccurrences() (/home/tcwg-buildbot/worker/clang-armv7-global-isel/stage1/unittests/Support/./SupportTests+0x4e51f0) #5 0x00af8e1c llvm::cl::ResetCommandLineParser() (/home/tcwg-buildbot/worker/clang-armv7-global-isel/stage1/unittests/Support/./SupportTests+0x4e4e1c) #6 0x0077cda0 (anonymous namespace)::CommandLineTest_GroupingAndPrefix_Test::TestBody() (/home/tcwg-buildbot/worker/clang-armv7-global-isel/stage1/unittests/Support/./SupportTests+0x168da0) #7 0x00bc5adc testing::Test::Run() (/home/tcwg-buildbot/worker/clang-armv7-global-isel/stage1/unittests/Support/./SupportTests+0x5b1adc) #8 0x00bc6cc0 testing::TestInfo::Run() (/home/tcwg-buildbot/worker/clang-armv7-global-isel/stage1/unittests/Support/./SupportTests+0x5b2cc0) #9 0x00bc7880 testing::TestSuite::Run() (/home/tcwg-buildbot/worker/clang-armv7-global-isel/stage1/unittests/Support/./SupportTests+0x5b3880) #10 0x00bd7974 testing::internal::UnitTestImpl::RunAllTests() (/home/tcwg-buildbot/worker/clang-armv7-global-isel/stage1/unittests/Support/./SupportTests+0x5c3974) #11 0x00bd6ebc testing::UnitTest::Run() (/home/tcwg-buildbot/worker/clang-armv7-global-isel/stage1/unittests/Support/./SupportTests+0x5c2ebc) #12 0x00bb1058 main (/home/tcwg-buildbot/worker/clang-armv7-global-isel/stage1/unittests/Support/./SupportTests+0x59d058) #13 0xf78185a4 __libc_start_main /build/glibc-9MGTF6/glibc-2.31/csu/libc-start.c:342:3 ``` --- llvm/lib/Support/CommandLine.cpp | 7 --- llvm/unittests/Support/CommandLineTest.cpp | 53 ---------------------- 2 files changed, 60 deletions(-) diff --git a/llvm/lib/Support/CommandLine.cpp b/llvm/lib/Support/CommandLine.cpp index a7e0cae8b855d7..55633d7cafa479 100644 --- a/llvm/lib/Support/CommandLine.cpp +++ b/llvm/lib/Support/CommandLine.cpp @@ -1667,13 +1667,6 @@ 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 4411ed0f83928a..41cc8260acfedf 100644 --- a/llvm/unittests/Support/CommandLineTest.cpp +++ b/llvm/unittests/Support/CommandLineTest.cpp @@ -525,59 +525,6 @@ 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();