[llvm] [CommandLine] Make options of copyable class types get reset to their provided initial values (PR #173026)
Benjamin Stott via llvm-commits
llvm-commits at lists.llvm.org
Mon Dec 22 02:25:12 PST 2025
https://github.com/BStott6 updated https://github.com/llvm/llvm-project/pull/173026
>From 59eb23dd5d4e2e575f429929d5742c3db0f5dc66 Mon Sep 17 00:00:00 2001
From: BStott <Benjamin.Stott at sony.com>
Date: Fri, 19 Dec 2025 14:31:49 +0000
Subject: [PATCH 1/4] Introduce unit test for resetting option of class type to
initial value
---
llvm/unittests/Support/CommandLineTest.cpp | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/llvm/unittests/Support/CommandLineTest.cpp b/llvm/unittests/Support/CommandLineTest.cpp
index 7f538f155be15..a2a774200ebce 100644
--- a/llvm/unittests/Support/CommandLineTest.cpp
+++ b/llvm/unittests/Support/CommandLineTest.cpp
@@ -2348,4 +2348,15 @@ TEST(CommandLineTest, HelpWithEmptyCategory) {
cl::ResetCommandLineParser();
}
+class CopyableClass {
+public:
+ int Val;
+};
+TEST(CommandLineTest, ResetClassTypeOptionToInitialValue) {
+ CopyableClass InitialValue{42};
+ StackOption<CopyableClass> O("a", cl::init(InitialValue));
+ O.reset();
+ EXPECT_EQ(O.getValue().Val, InitialValue.Val)
+ << "Option should be reset to its initial value.";
+}
} // anonymous namespace
>From 6603f76351f2fef57c9bf8827bf33d395f2d1829 Mon Sep 17 00:00:00 2001
From: BStott <Benjamin.Stott at sony.com>
Date: Fri, 19 Dec 2025 15:24:05 +0000
Subject: [PATCH 2/4] Also test options of scalar types
---
llvm/unittests/Support/CommandLineTest.cpp | 23 ++++++++++++++++++----
1 file changed, 19 insertions(+), 4 deletions(-)
diff --git a/llvm/unittests/Support/CommandLineTest.cpp b/llvm/unittests/Support/CommandLineTest.cpp
index a2a774200ebce..b64be7c094e46 100644
--- a/llvm/unittests/Support/CommandLineTest.cpp
+++ b/llvm/unittests/Support/CommandLineTest.cpp
@@ -2352,11 +2352,26 @@ class CopyableClass {
public:
int Val;
};
-TEST(CommandLineTest, ResetClassTypeOptionToInitialValue) {
+TEST(CommandLineTest, ResetOptionsToInitialValue) {
+ // Option of scalar type.
+ StackOption<int> O1("opt1", cl::init(12));
+ O1.reset();
+ EXPECT_EQ(O1.getValue(), 12)
+ << "Option should be reset to its initial value.";
+
+ // Option of copyable class type.
CopyableClass InitialValue{42};
- StackOption<CopyableClass> O("a", cl::init(InitialValue));
- O.reset();
- EXPECT_EQ(O.getValue().Val, InitialValue.Val)
+ StackOption<CopyableClass> O2("opt2", cl::init(InitialValue));
+ O2.reset();
+ EXPECT_EQ(O2.getValue().Val, InitialValue.Val)
+ << "Option should be reset to its initial value.";
+
+ // Option of string type (most important case of copyable class).
+ StackOption<std::string> O3("opt3", cl::init("hello"));
+ O3.reset();
+ EXPECT_EQ(O3.getValue(), "hello")
<< "Option should be reset to its initial value.";
+
+ cl::ResetCommandLineParser();
}
} // anonymous namespace
>From 9b7d877621552eb5f1851bf07bf4e7e5aad02ada Mon Sep 17 00:00:00 2001
From: BStott <Benjamin.Stott at sony.com>
Date: Fri, 19 Dec 2025 15:42:09 +0000
Subject: [PATCH 3/4] [CommandLine] Make options of copyable class types get
reset to their provided initial values
---
llvm/include/llvm/Support/CommandLine.h | 61 ++++++++-----------------
llvm/lib/Support/CommandLine.cpp | 2 -
2 files changed, 20 insertions(+), 43 deletions(-)
diff --git a/llvm/include/llvm/Support/CommandLine.h b/llvm/include/llvm/Support/CommandLine.h
index d737fbcf891b3..742f6d497545f 100644
--- a/llvm/include/llvm/Support/CommandLine.h
+++ b/llvm/include/llvm/Support/CommandLine.h
@@ -548,7 +548,7 @@ template <class DataType> struct OptionValue;
// The default value safely does nothing. Option value printing is only
// best-effort.
-template <class DataType, bool isClass>
+template <class DataType, bool isCopyable>
struct OptionValueBase : GenericOptionValue {
// Temporary storage for argument passing.
using WrapperType = OptionValue<DataType>;
@@ -591,13 +591,24 @@ template <class DataType> class OptionValueCopy : public GenericOptionValue {
return Value;
}
- void setValue(const DataType &V) {
+ template <class DT, std::enable_if_t<std::is_assignable_v<DataType &, DT>,
+ std::nullptr_t> = nullptr>
+ void setValue(const DT &V) {
Valid = true;
Value = V;
}
// Returns whether this instance matches V.
- bool compare(const DataType &V) const { return Valid && (Value == V); }
+ bool compare(const DataType &V) const {
+ // FIXME: With C++23, use `std::equality_comparable` to see if `DataType`
+ // may be a class with `operator==` and, if so, use it instead of silently
+ // returning false.
+ if constexpr (std::is_class_v<DataType>) {
+ return false;
+ } else {
+ return Valid && (Value == V);
+ }
+ }
bool compare(const GenericOptionValue &V) const override {
const OptionValueCopy<DataType> &VC =
@@ -608,9 +619,9 @@ template <class DataType> class OptionValueCopy : public GenericOptionValue {
}
};
-// Non-class option values.
+// Copyable option values.
template <class DataType>
-struct OptionValueBase<DataType, false> : OptionValueCopy<DataType> {
+struct OptionValueBase<DataType, true> : OptionValueCopy<DataType> {
using WrapperType = DataType;
protected:
@@ -623,7 +634,10 @@ struct OptionValueBase<DataType, false> : OptionValueCopy<DataType> {
// Top-level option class.
template <class DataType>
struct OptionValue final
- : OptionValueBase<DataType, std::is_class_v<DataType>> {
+ : OptionValueBase<DataType, std::conjunction_v<
+ std::is_copy_constructible<DataType>,
+ std::is_copy_assignable<DataType>,
+ std::is_default_constructible<DataType>>> {
OptionValue() = default;
OptionValue(const DataType &V) { this->setValue(V); }
@@ -635,42 +649,7 @@ struct OptionValue final
}
};
-// Other safe-to-copy-by-value common option types.
enum boolOrDefault { BOU_UNSET, BOU_TRUE, BOU_FALSE };
-template <>
-struct LLVM_ABI OptionValue<cl::boolOrDefault> final
- : OptionValueCopy<cl::boolOrDefault> {
- using WrapperType = cl::boolOrDefault;
-
- OptionValue() = default;
-
- OptionValue(const cl::boolOrDefault &V) { this->setValue(V); }
-
- OptionValue<cl::boolOrDefault> &operator=(const cl::boolOrDefault &V) {
- setValue(V);
- return *this;
- }
-
-private:
- void anchor() override;
-};
-
-template <>
-struct LLVM_ABI OptionValue<std::string> final : OptionValueCopy<std::string> {
- using WrapperType = StringRef;
-
- OptionValue() = default;
-
- OptionValue(const std::string &V) { this->setValue(V); }
-
- OptionValue<std::string> &operator=(const std::string &V) {
- setValue(V);
- return *this;
- }
-
-private:
- void anchor() override;
-};
//===----------------------------------------------------------------------===//
// Enum valued command line option
diff --git a/llvm/lib/Support/CommandLine.cpp b/llvm/lib/Support/CommandLine.cpp
index 5095b298fd42d..6f58d1e3fe092 100644
--- a/llvm/lib/Support/CommandLine.cpp
+++ b/llvm/lib/Support/CommandLine.cpp
@@ -86,8 +86,6 @@ template class LLVM_EXPORT_TEMPLATE opt<unsigned>;
// Pin the vtables to this file.
void GenericOptionValue::anchor() {}
-void OptionValue<boolOrDefault>::anchor() {}
-void OptionValue<std::string>::anchor() {}
void Option::anchor() {}
void basic_parser_impl::anchor() {}
void parser<bool>::anchor() {}
>From 9c6ee92c8495a2a2559b1480d859f57052c18931 Mon Sep 17 00:00:00 2001
From: BStott <Benjamin.Stott at sony.com>
Date: Mon, 22 Dec 2025 10:24:49 +0000
Subject: [PATCH 4/4] Apply suggestions, add test cases for comparing option
values
---
llvm/include/llvm/ADT/STLExtras.h | 11 +++++
llvm/include/llvm/Support/CommandLine.h | 13 +++---
llvm/unittests/Support/CommandLineTest.cpp | 52 ++++++++++++++++++++++
3 files changed, 68 insertions(+), 8 deletions(-)
diff --git a/llvm/include/llvm/ADT/STLExtras.h b/llvm/include/llvm/ADT/STLExtras.h
index d4632ab72e563..b14514b33b389 100644
--- a/llvm/include/llvm/ADT/STLExtras.h
+++ b/llvm/include/llvm/ADT/STLExtras.h
@@ -2641,6 +2641,17 @@ template <typename T> using has_sizeof = decltype(sizeof(T));
template <typename T>
constexpr bool is_incomplete_v = !is_detected<detail::has_sizeof, T>::value;
+// Detect types with equality comparison operators.
+namespace detail {
+template <class T>
+using has_equality_comparison =
+ decltype(std::declval<T &>() == std::declval<T &>());
+} // namespace detail
+
+/// Detects whether type `T` can be compared for equality with itself.
+template <typename T>
+constexpr bool has_equality_comparison_v =
+ is_detected<detail::has_equality_comparison, T>::value;
} // end namespace llvm
namespace std {
diff --git a/llvm/include/llvm/Support/CommandLine.h b/llvm/include/llvm/Support/CommandLine.h
index 742f6d497545f..5387b0a3e9b8c 100644
--- a/llvm/include/llvm/Support/CommandLine.h
+++ b/llvm/include/llvm/Support/CommandLine.h
@@ -591,8 +591,8 @@ template <class DataType> class OptionValueCopy : public GenericOptionValue {
return Value;
}
- template <class DT, std::enable_if_t<std::is_assignable_v<DataType &, DT>,
- std::nullptr_t> = nullptr>
+ template <class DT,
+ class = std::enable_if_t<std::is_assignable_v<DataType &, DT>>>
void setValue(const DT &V) {
Valid = true;
Value = V;
@@ -600,13 +600,10 @@ template <class DataType> class OptionValueCopy : public GenericOptionValue {
// Returns whether this instance matches V.
bool compare(const DataType &V) const {
- // FIXME: With C++23, use `std::equality_comparable` to see if `DataType`
- // may be a class with `operator==` and, if so, use it instead of silently
- // returning false.
- if constexpr (std::is_class_v<DataType>) {
- return false;
- } else {
+ if constexpr (has_equality_comparison_v<DataType>) {
return Valid && (Value == V);
+ } else {
+ return false;
}
}
diff --git a/llvm/unittests/Support/CommandLineTest.cpp b/llvm/unittests/Support/CommandLineTest.cpp
index b64be7c094e46..3b6da1fc49a44 100644
--- a/llvm/unittests/Support/CommandLineTest.cpp
+++ b/llvm/unittests/Support/CommandLineTest.cpp
@@ -2374,4 +2374,56 @@ TEST(CommandLineTest, ResetOptionsToInitialValue) {
cl::ResetCommandLineParser();
}
+
+class WithEqualityComparison {
+public:
+ bool operator==(const WithEqualityComparison &Other) const {
+ return Val == Other.Val;
+ }
+
+ int Val;
+};
+
+class NoEqualityComparison {
+public:
+ int Val;
+};
+
+TEST(CommandLineTest, CompareOptionValues) {
+ // Scalar option.
+ {
+ StackOption<int> O1("opt1", cl::init(42));
+ StackOption<int> O2("opt2", cl::init(42));
+ StackOption<int> O3("opt3", cl::init(3));
+ EXPECT_TRUE(O1.Default.compare(O2.Default));
+ EXPECT_FALSE(O1.Default.compare(O3.Default));
+ cl::ResetCommandLineParser();
+ }
+
+ // Class with equality comparison operator.
+ {
+ StackOption<WithEqualityComparison> O1(
+ "opt1", cl::init(WithEqualityComparison{42}));
+ StackOption<WithEqualityComparison> O2(
+ "opt2", cl::init(WithEqualityComparison{42}));
+ StackOption<WithEqualityComparison> O3("opt3",
+ cl::init(WithEqualityComparison{3}));
+ EXPECT_TRUE(O1.Default.compare(O2.Default));
+ EXPECT_FALSE(O1.Default.compare(O3.Default));
+ cl::ResetCommandLineParser();
+ }
+
+ // Class with no equality comparison operator.
+ {
+ StackOption<NoEqualityComparison> O1("opt1",
+ cl::init(NoEqualityComparison{42}));
+ StackOption<NoEqualityComparison> O2("opt2",
+ cl::init(NoEqualityComparison{42}));
+ StackOption<NoEqualityComparison> O3("opt3",
+ cl::init(NoEqualityComparison{3}));
+ EXPECT_FALSE(O1.Default.compare(O2.Default));
+ EXPECT_FALSE(O1.Default.compare(O3.Default));
+ cl::ResetCommandLineParser();
+ }
+}
} // anonymous namespace
More information about the llvm-commits
mailing list