[llvm] 2e912a2 - Revert "[Support]Look up in top-level subcommand as a fallback when looking options for a custom subcommand (#71975)
via llvm-commits
llvm-commits at lists.llvm.org
Fri Nov 10 11:44:34 PST 2023
Author: Mingming Liu
Date: 2023-11-10T11:44:30-08:00
New Revision: 2e912a2b82dde3940e77b62832629ecf0a8b9f14
URL: https://github.com/llvm/llvm-project/commit/2e912a2b82dde3940e77b62832629ecf0a8b9f14
DIFF: https://github.com/llvm/llvm-project/commit/2e912a2b82dde3940e77b62832629ecf0a8b9f14.diff
LOG: Revert "[Support]Look up in top-level subcommand as a fallback when looking options for a custom subcommand (#71975)
…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
```
Added:
Modified:
llvm/lib/Support/CommandLine.cpp
llvm/unittests/Support/CommandLineTest.cpp
Removed:
################################################################################
diff --git a/llvm/lib/Support/CommandLine.cpp b/llvm/lib/Support/CommandLine.cpp
index a7e0cae8b855d7c..55633d7cafa4791 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 4411ed0f83928ad..41cc8260acfedf7 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<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();
More information about the llvm-commits
mailing list