Revert "[Support]Look up in top-level subcommand as a fallback when looking options for a custom subcommand #71975
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-llvm-support Author: Mingming Liu (minglotus-6) Changes…ooking options for a custom subcommand. (#71776)" This reverts commit b88308b. The build-bot is unhappy (https://lab.llvm.org/buildbot/#/builders/186/builds/13096), Revert while I look into this (might be related with test sharding but not sure) Full diff: https://github.com/llvm/llvm-project/pull/71975.diff 2 Files Affected:
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();
|
mingmingl-llvm
changed the title
|
I realized I might not need a review for a revert like this. So just went ahead. Sorry for the noise! |
|
Clicking the "revert" button gives a new PR which runs the premerge bot. For a recently landed commit that caused failures, I just run |
Thanks for sharing this! I clicked the revert button and use Github UI to merge it. I think using |
zahiraam
pushed a commit
to zahiraam/llvm-project
that referenced
this pull request
Nov 20, 2023
…ooking options for a custom subcommand (llvm#71975) …ooking options for a custom subcommand. (llvm#71776)" This reverts commit b88308b. 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) llvm#1 0x00ba5914 llvm::sys::RunSignalHandlers() (/home/tcwg-buildbot/worker/clang-armv7-global-isel/stage1/unittests/Support/./SupportTests+0x591914) llvm#2 0x00ba89c4 SignalHandler(int) (/home/tcwg-buildbot/worker/clang-armv7-global-isel/stage1/unittests/Support/./SupportTests+0x5949c4) llvm#3 0xf7828530 __default_sa_restorer /build/glibc-9MGTF6/glibc-2.31/signal/../sysdeps/unix/sysv/linux/arm/sigrestorer.S:67:0 llvm#4 0x00af91f0 (anonymous namespace)::CommandLineParser::ResetAllOptionOccurrences() (/home/tcwg-buildbot/worker/clang-armv7-global-isel/stage1/unittests/Support/./SupportTests+0x4e51f0) llvm#5 0x00af8e1c llvm::cl::ResetCommandLineParser() (/home/tcwg-buildbot/worker/clang-armv7-global-isel/stage1/unittests/Support/./SupportTests+0x4e4e1c) llvm#6 0x0077cda0 (anonymous namespace)::CommandLineTest_GroupingAndPrefix_Test::TestBody() (/home/tcwg-buildbot/worker/clang-armv7-global-isel/stage1/unittests/Support/./SupportTests+0x168da0) llvm#7 0x00bc5adc testing::Test::Run() (/home/tcwg-buildbot/worker/clang-armv7-global-isel/stage1/unittests/Support/./SupportTests+0x5b1adc) llvm#8 0x00bc6cc0 testing::TestInfo::Run() (/home/tcwg-buildbot/worker/clang-armv7-global-isel/stage1/unittests/Support/./SupportTests+0x5b2cc0) llvm#9 0x00bc7880 testing::TestSuite::Run() (/home/tcwg-buildbot/worker/clang-armv7-global-isel/stage1/unittests/Support/./SupportTests+0x5b3880) llvm#10 0x00bd7974 testing::internal::UnitTestImpl::RunAllTests() (/home/tcwg-buildbot/worker/clang-armv7-global-isel/stage1/unittests/Support/./SupportTests+0x5c3974) llvm#11 0x00bd6ebc testing::UnitTest::Run() (/home/tcwg-buildbot/worker/clang-armv7-global-isel/stage1/unittests/Support/./SupportTests+0x5c2ebc) llvm#12 0x00bb1058 main (/home/tcwg-buildbot/worker/clang-armv7-global-isel/stage1/unittests/Support/./SupportTests+0x59d058) llvm#13 0xf78185a4 __libc_start_main /build/glibc-9MGTF6/glibc-2.31/csu/libc-start.c:342:3 ```
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
…ooking options for a custom subcommand. (#71776)"
This reverts commit b88308b.
The build-bot is unhappy (https://lab.llvm.org/buildbot/#/builders/186/builds/13096),
GroupingAndPrefixfails afterTopLevelOptInSubcommand(the newly added test).Revert while I look into this (might be related with test sharding but not sure)