[Mlir-commits] [llvm] [mlir] [Support] Remove cl::Sink (PR #224953)
llvmlistbot at llvm.org
llvmlistbot at llvm.org
Sun Sep 20 11:44:13 PDT 2026
llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-mlir-core
Author: Fangrui Song (MaskRay)
<details>
<summary>Changes</summary>
Nothing uses it: the last users, clang's offloading linker wrappers,
moved to OptTable. The feature would block migration to TableGen based
representation. An unknown option is now always an error.
LLM-aided
---
Full diff: https://github.com/llvm/llvm-project/pull/224953.diff
5 Files Affected:
- (modified) llvm/docs/CommandLine.md (-8)
- (modified) llvm/include/llvm/Support/CommandLine.h (-3)
- (modified) llvm/lib/Support/CommandLine.cpp (+2-22)
- (modified) llvm/unittests/Support/CommandLineTest.cpp (+3-7)
- (modified) mlir/include/mlir/Pass/PassOptions.h (+2-4)
``````````diff
diff --git a/llvm/docs/CommandLine.md b/llvm/docs/CommandLine.md
index 2cf37ebc52d42..ab8531a173641 100644
--- a/llvm/docs/CommandLine.md
+++ b/llvm/docs/CommandLine.md
@@ -1249,14 +1249,6 @@ specify boolean properties that modify the option.
be applied to the "`-pos1`" option and the "`-bork`" string to be applied
to the "`-pos2`" option.
-(cl::Sink)=
-
-* The **cl::Sink** modifier is used to handle unknown options. If there is at
- least one option with `cl::Sink` modifier specified, the parser passes
- unrecognized option strings to it as values instead of signaling an error. As
- with `cl::CommaSeparated`, this modifier only makes sense with a {ref}`cl::list <cl::list>`
- option.
-
(response files)=
#### Response files
diff --git a/llvm/include/llvm/Support/CommandLine.h b/llvm/include/llvm/Support/CommandLine.h
index dd42c4bfb7f49..9118e48db7614 100644
--- a/llvm/include/llvm/Support/CommandLine.h
+++ b/llvm/include/llvm/Support/CommandLine.h
@@ -161,7 +161,6 @@ enum FormattingFlags {
enum MiscFlags { // Miscellaneous flags to adjust argument
CommaSeparated = 0x01, // Should this cl::list split between commas?
PositionalEatsArgs = 0x02, // Should this positional cl::list eat -args?
- Sink = 0x04, // Should this cl::list eat all unknown options?
// Can this option group with other options?
// If this is enabled, multiple letter options are allowed to bunch together
@@ -229,7 +228,6 @@ class SubCommand {
StringRef getDescription() const { return Description; }
SmallVector<Option *, 4> PositionalOpts;
- SmallVector<Option *, 4> SinkOpts;
DenseMap<StringRef, Option *> OptionsMap;
Option *ConsumeAfterOpt = nullptr; // The ConsumeAfter option if it exists.
@@ -308,7 +306,6 @@ class LLVM_ABI Option {
// Return true if the argstr != ""
bool hasArgStr() const { return !ArgStr.empty(); }
bool isPositional() const { return getFormattingFlag() == cl::Positional; }
- bool isSink() const { return getMiscFlags() & cl::Sink; }
bool isDefaultOption() const { return getMiscFlags() & cl::DefaultOption; }
bool isConsumeAfter() const {
diff --git a/llvm/lib/Support/CommandLine.cpp b/llvm/lib/Support/CommandLine.cpp
index 03ef86bb225d3..c5a9747e64322 100644
--- a/llvm/lib/Support/CommandLine.cpp
+++ b/llvm/lib/Support/CommandLine.cpp
@@ -247,8 +247,6 @@ class CommandLineParser {
// Remember information about positional options.
if (O->getFormattingFlag() == cl::Positional)
SC->PositionalOpts.push_back(O);
- else if (O->getMiscFlags() & cl::Sink) // Remember sink options
- SC->SinkOpts.push_back(O);
else if (O->getNumOccurrencesFlag() == cl::ConsumeAfter) {
if (SC->ConsumeAfterOpt) {
O->error("Cannot specify more than one option with cl::ConsumeAfter!");
@@ -296,13 +294,6 @@ class CommandLineParser {
break;
}
}
- else if (O->getMiscFlags() & cl::Sink)
- for (auto *Opt = Sub.SinkOpts.begin(); Opt != Sub.SinkOpts.end(); ++Opt) {
- if (*Opt == O) {
- Sub.SinkOpts.erase(Opt);
- break;
- }
- }
else if (O == Sub.ConsumeAfterOpt)
Sub.ConsumeAfterOpt = nullptr;
}
@@ -375,8 +366,7 @@ class CommandLineParser {
"SubCommand::getAll() should not be registered");
for (auto &E : SubCommand::getAll().OptionsMap) {
Option *O = E.second;
- if ((O->isPositional() || O->isSink() || O->isConsumeAfter()) ||
- O->hasArgStr())
+ if (O->isPositional() || O->isConsumeAfter() || O->hasArgStr())
addOption(O, sub);
else
addLiteralOption(*O, sub, E.first);
@@ -532,7 +522,6 @@ void SubCommand::unregisterSubCommand() {
void SubCommand::reset() {
PositionalOpts.clear();
- SinkOpts.clear();
OptionsMap.clear();
ConsumeAfterOpt = nullptr;
@@ -1525,8 +1514,6 @@ void CommandLineParser::ResetAllOptionOccurrences() {
O->reset();
for (Option *O : SC->PositionalOpts)
O->reset();
- for (Option *O : SC->SinkOpts)
- O->reset();
if (SC->ConsumeAfterOpt)
SC->ConsumeAfterOpt->reset();
}
@@ -1588,7 +1575,6 @@ bool CommandLineParser::ParseCommandLineOptions(
assert(ChosenSubCommand);
auto &ConsumeAfterOpt = ChosenSubCommand->ConsumeAfterOpt;
auto &PositionalOpts = ChosenSubCommand->PositionalOpts;
- auto &SinkOpts = ChosenSubCommand->SinkOpts;
auto &OptionsMap = ChosenSubCommand->OptionsMap;
for (auto *O: DefaultOptions) {
@@ -1725,17 +1711,11 @@ bool CommandLineParser::ParseCommandLineOptions(
// Otherwise, look for the closest available option to report to the user
// in the upcoming error.
- if (!Handler && SinkOpts.empty())
+ if (!Handler)
LookupNearestOption(ArgName, OptionsMap, NearestHandlerString);
}
if (!Handler) {
- if (!SinkOpts.empty()) {
- for (Option *SinkOpt : SinkOpts)
- SinkOpt->addOccurrence(i, "", StringRef(argv[i]));
- continue;
- }
-
auto ReportUnknownArgument = [&](bool IsArg,
StringRef NearestArgumentName) {
*Errs << ProgramName << ": Unknown "
diff --git a/llvm/unittests/Support/CommandLineTest.cpp b/llvm/unittests/Support/CommandLineTest.cpp
index 956d5b97c2703..8fa69ff9c2987 100644
--- a/llvm/unittests/Support/CommandLineTest.cpp
+++ b/llvm/unittests/Support/CommandLineTest.cpp
@@ -2298,23 +2298,20 @@ TEST(CommandLineTest, ResetAllOptionOccurrences) {
cl::values(clEnumValN(ValA, "enableA", "Enable A"),
clEnumValN(ValB, "enableB", "Enable B"),
clEnumValN(ValC, "enableC", "Enable C")));
- StackOption<std::string, cl::list<std::string>> Sink(cl::Sink);
StackOption<std::string> Input(cl::Positional);
StackOption<std::string, cl::list<std::string>> ExtraArgs(cl::ConsumeAfter);
- const char *Args[] = {"prog", "-option", "-str=STR", "-enableA",
- "-enableC", "-unknown", "input", "-arg"};
+ const char *Args[] = {"prog", "-option", "-str=STR", "-enableA",
+ "-enableC", "input", "-arg"};
std::string Errs;
raw_string_ostream OS(Errs);
- EXPECT_TRUE(cl::ParseCommandLineOptions(8, Args, StringRef(), &OS));
+ EXPECT_TRUE(cl::ParseCommandLineOptions(7, Args, StringRef(), &OS));
EXPECT_TRUE(OS.str().empty());
EXPECT_TRUE(Option);
EXPECT_EQ("STR", Str);
EXPECT_EQ((1u << ValA) | (1u << ValC), Bits.getBits());
- EXPECT_EQ(1u, Sink.size());
- EXPECT_EQ("-unknown", Sink[0]);
EXPECT_EQ("input", Input);
EXPECT_EQ(1u, ExtraArgs.size());
EXPECT_EQ("-arg", ExtraArgs[0]);
@@ -2323,7 +2320,6 @@ TEST(CommandLineTest, ResetAllOptionOccurrences) {
EXPECT_FALSE(Option);
EXPECT_EQ("", Str);
EXPECT_EQ(0u, Bits.getBits());
- EXPECT_EQ(0u, Sink.size());
EXPECT_EQ(0, Input.getNumOccurrences());
EXPECT_EQ(0u, ExtraArgs.size());
}
diff --git a/mlir/include/mlir/Pass/PassOptions.h b/mlir/include/mlir/Pass/PassOptions.h
index 4e4652ee8eef1..908e85830e5ac 100644
--- a/mlir/include/mlir/Pass/PassOptions.h
+++ b/mlir/include/mlir/Pass/PassOptions.h
@@ -197,8 +197,7 @@ class PassOptions : protected llvm::cl::SubCommand {
Option(PassOptions &parent, StringRef arg, Args &&...args)
: llvm::cl::opt<DataType, /*ExternalStorage=*/false, OptionParser>(
arg, llvm::cl::sub(parent), std::forward<Args>(args)...) {
- assert(!this->isPositional() && !this->isSink() &&
- "sink and positional options are not supported");
+ assert(!this->isPositional() && "positional options are not supported");
parent.options.push_back(this);
// Set a callback to track if this option has a value.
@@ -245,8 +244,7 @@ class PassOptions : protected llvm::cl::SubCommand {
: llvm::cl::list<DataType, /*StorageClass=*/bool, OptionParser>(
arg, llvm::cl::sub(parent), std::forward<Args>(args)...),
elementParser(*this) {
- assert(!this->isPositional() && !this->isSink() &&
- "sink and positional options are not supported");
+ assert(!this->isPositional() && "positional options are not supported");
assert(!(this->getMiscFlags() & llvm::cl::MiscFlags::CommaSeparated) &&
"ListOption is implicitly comma separated, specifying "
"CommaSeparated is extraneous");
``````````
</details>
https://github.com/llvm/llvm-project/pull/224953
More information about the Mlir-commits
mailing list