[llvm] [mlir] [Support] Accept only true, false, 1 and 0 for cl::opt<bool> (PR #224975)
Fangrui Song via llvm-commits
llvm-commits at lists.llvm.org
Mon Sep 21 00:28:44 PDT 2026
https://github.com/MaskRay updated https://github.com/llvm/llvm-project/pull/224975
>From 0d0359edf151ab2be4812804b855661203dd9f52 Mon Sep 17 00:00:00 2001
From: Fangrui Song <i at maskray.me>
Date: Sun, 20 Sep 2026 11:45:12 -0700
Subject: [PATCH 1/2] [Support] Accept only true, false, 1 and 0 for
cl::opt<bool>
For these internal developer options, accepting
`-flag=True`/`-flag=FALSE` is a weird quirk. To the best of my
knowledge, they are nearly never used. Drop the spellings. In the
future, we will migrate to `-flag`/`-no-flag` (or their double-dash
form).
LLM-aided
---
llvm/docs/CommandLine.md | 8 ++---
llvm/lib/Support/CommandLine.cpp | 6 ++--
llvm/unittests/Support/CommandLineTest.cpp | 29 +++++++++++++++++++
.../Dialect/SparseTensor/external_direct.mlir | 2 +-
4 files changed, 37 insertions(+), 8 deletions(-)
diff --git a/llvm/docs/CommandLine.md b/llvm/docs/CommandLine.md
index 2cf37ebc52d42..04ed3c2739f0d 100644
--- a/llvm/docs/CommandLine.md
+++ b/llvm/docs/CommandLine.md
@@ -226,8 +226,8 @@ specified, allowing any of the following inputs:
```
compiler -f # No value, 'Force' == true
compiler -f=true # Value specified, 'Force' == true
-compiler -f=TRUE # Value specified, 'Force' == true
-compiler -f=FALSE # Value specified, 'Force' == false
+compiler -f=1 # Value specified, 'Force' == true
+compiler -f=false # Value specified, 'Force' == false
```
... you get the idea. The {ref}`bool parser <bool parser>` just turns the string values into
@@ -1515,8 +1515,8 @@ work with new data types and new ways of interpreting the same data. See the
(bool parser)=
* The **parser<bool> specialization** is used to convert boolean strings to a
- boolean value. Currently accepted strings are "`true`", "`TRUE`",
- "`True`", "`1`", "`false`", "`FALSE`", "`False`", and "`0`".
+ boolean value. Currently accepted strings are "`true`", "`1`",
+ "`false`", and "`0`".
* The **parser<boolOrDefault> specialization** is used for cases where the value
is boolean, but we also need to know whether the option was specified at all.
diff --git a/llvm/lib/Support/CommandLine.cpp b/llvm/lib/Support/CommandLine.cpp
index 03ef86bb225d3..014b90cbd0093 100644
--- a/llvm/lib/Support/CommandLine.cpp
+++ b/llvm/lib/Support/CommandLine.cpp
@@ -439,13 +439,13 @@ static CommandLineParser &globalParser() {
template <typename T, T TrueVal, T FalseVal>
static bool parseBool(Option &O, StringRef ArgName, StringRef Arg, T &Value) {
- if (Arg == "" || Arg == "true" || Arg == "TRUE" || Arg == "True" ||
- Arg == "1") {
+ // A bare -flag passes a null Arg; -flag= passes an empty one.
+ if (!Arg.data() || Arg == "true" || Arg == "1") {
Value = TrueVal;
return false;
}
- if (Arg == "false" || Arg == "FALSE" || Arg == "False" || Arg == "0") {
+ if (Arg == "false" || Arg == "0") {
Value = FalseVal;
return false;
}
diff --git a/llvm/unittests/Support/CommandLineTest.cpp b/llvm/unittests/Support/CommandLineTest.cpp
index 956d5b97c2703..dbf7ffbe076f8 100644
--- a/llvm/unittests/Support/CommandLineTest.cpp
+++ b/llvm/unittests/Support/CommandLineTest.cpp
@@ -1666,6 +1666,35 @@ TEST_F(GetOptionWidthTest,
ExpectedStrSize);
}
+TEST(CommandLineTest, BoolValues) {
+ cl::ResetCommandLineParser();
+
+ StackOption<bool> OptF("f", cl::init(true));
+ StackOption<bool> OptFlag("flag");
+
+ const char *args1[] = {"prog", "-flag", "--f=false"};
+ EXPECT_TRUE(
+ cl::ParseCommandLineOptions(3, args1, StringRef(), &llvm::nulls()));
+ EXPECT_TRUE(OptFlag);
+ EXPECT_FALSE(OptF);
+ cl::ResetAllOptionOccurrences();
+
+ // An empty value is not the same as no value.
+ const char *args2[] = {"prog", "-flag="};
+ EXPECT_FALSE(
+ cl::ParseCommandLineOptions(2, args2, StringRef(), &llvm::nulls()));
+ cl::ResetAllOptionOccurrences();
+
+ const char *args3[] = {"prog", "-flag=yes"};
+ EXPECT_FALSE(
+ cl::ParseCommandLineOptions(2, args3, StringRef(), &llvm::nulls()));
+ cl::ResetAllOptionOccurrences();
+
+ const char *args4[] = {"prog", "-flag=True"};
+ EXPECT_FALSE(
+ cl::ParseCommandLineOptions(2, args4, StringRef(), &llvm::nulls()));
+}
+
TEST(CommandLineTest, PrefixOptions) {
cl::ResetCommandLineParser();
diff --git a/mlir/test/Dialect/SparseTensor/external_direct.mlir b/mlir/test/Dialect/SparseTensor/external_direct.mlir
index 78c4a295686b3..0980f8bf9e992 100644
--- a/mlir/test/Dialect/SparseTensor/external_direct.mlir
+++ b/mlir/test/Dialect/SparseTensor/external_direct.mlir
@@ -1,4 +1,4 @@
-// RUN: mlir-opt %s --sparse-assembler="direct-out=True" -split-input-file | FileCheck %s
+// RUN: mlir-opt %s --sparse-assembler="direct-out=true" -split-input-file | FileCheck %s
// -----
>From dfea1dd4ca5cf0a9f9a389bcfe77c4724d45eb4e Mon Sep 17 00:00:00 2001
From: Fangrui Song <i at maskray.me>
Date: Mon, 21 Sep 2026 00:28:27 -0700
Subject: [PATCH 2/2] improve comment
---
llvm/lib/Support/CommandLine.cpp | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/llvm/lib/Support/CommandLine.cpp b/llvm/lib/Support/CommandLine.cpp
index 82bf701356c8d..b7ca1e6c19143 100644
--- a/llvm/lib/Support/CommandLine.cpp
+++ b/llvm/lib/Support/CommandLine.cpp
@@ -429,7 +429,8 @@ static CommandLineParser &globalParser() {
template <typename T, T TrueVal, T FalseVal>
static bool parseBool(Option &O, StringRef ArgName, StringRef Arg, T &Value) {
- // A bare -flag passes a null Arg; -flag= passes an empty one.
+ // ProvideOption passes a null Arg for a bare -flag (treated as true) and an
+ // empty one for -flag= (treated as invalid).
if (!Arg.data() || Arg == "true" || Arg == "1") {
Value = TrueVal;
return false;
More information about the llvm-commits
mailing list