Skip to content

[libc++] P2944R3: Constrained comparisions - variant #141396

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

H-G-Hristov
Copy link
Contributor

@H-G-Hristov H-G-Hristov commented May 25, 2025

This is a follow-up and depends on #139368 being merged first.

Implements P2944R3 partially, which adds constrained comparisons to std::variant

Closes #136769

Depends on #139368

References

variant.relops

Copy link

github-actions bot commented May 25, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

@Zingam Zingam linked an issue May 25, 2025 that may be closed by this pull request
@Zingam Zingam added libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. c++26 labels May 25, 2025
@llvmbot
Copy link
Member

llvmbot commented May 25, 2025

@llvm/pr-subscribers-libcxx

Author: Hristo Hristov (H-G-Hristov)

Changes

This is a follow-up and depends on #139368 being merged first.

Implements P2944R3 partially, which adds constrained comparisons to std::variant and std::tuple.

The missing overloads introduced in P2165R4 are not implemented.

Relates to #136765
Closes #136769

Relates to: #139368

References

tuple.rel
variant.relops


Full diff: https://github.com/llvm/llvm-project/pull/141396.diff

6 Files Affected:

  • (modified) libcxx/include/tuple (+7)
  • (modified) libcxx/include/variant (+31)
  • (modified) libcxx/test/std/utilities/tuple/tuple.tuple/tuple.rel/eq.pass.cpp (+21-2)
  • (modified) libcxx/test/std/utilities/tuple/tuple.tuple/tuple.rel/size_incompatible_comparison.verify.cpp (+6)
  • (modified) libcxx/test/std/utilities/variant/variant.relops/relops.pass.cpp (+49)
  • (modified) libcxx/test/support/test_comparisons.h (+65-1)
diff --git a/libcxx/include/tuple b/libcxx/include/tuple
index 8dd62ae624f5e..35ea37a5ea5ed 100644
--- a/libcxx/include/tuple
+++ b/libcxx/include/tuple
@@ -1161,9 +1161,16 @@ struct __tuple_equal<0> {
 };
 
 template <class... _Tp, class... _Up>
+#    if _LIBCPP_STD_VER >= 26
+  requires(requires(const _Tp& __t, const _Up& __u) {
+            { __t == __u } -> __boolean_testable;
+          } && ...) && (sizeof...(_Tp) == sizeof...(_Up))
+#    endif
 inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX14 bool
 operator==(const tuple<_Tp...>& __x, const tuple<_Up...>& __y) {
+#    if _LIBCPP_STD_VER < 26
   static_assert(sizeof...(_Tp) == sizeof...(_Up), "Can't compare tuples of different sizes");
+#    endif
   return __tuple_equal<sizeof...(_Tp)>()(__x, __y);
 }
 
diff --git a/libcxx/include/variant b/libcxx/include/variant
index bf611aec704ca..f33058b708528 100644
--- a/libcxx/include/variant
+++ b/libcxx/include/variant
@@ -242,6 +242,7 @@ namespace std {
 #  include <__type_traits/is_assignable.h>
 #  include <__type_traits/is_constructible.h>
 #  include <__type_traits/is_convertible.h>
+#  include <__type_traits/is_core_convertible.h>
 #  include <__type_traits/is_destructible.h>
 #  include <__type_traits/is_nothrow_assignable.h>
 #  include <__type_traits/is_nothrow_constructible.h>
@@ -1453,6 +1454,11 @@ struct __convert_to_bool {
 };
 
 template <class... _Types>
+#    if _LIBCPP_STD_VER >= 26
+  requires(requires(const _Types& __t) {
+    { __t == __t } -> __core_convertible_to<bool>;
+  } && ...)
+#    endif
 _LIBCPP_HIDE_FROM_ABI constexpr bool operator==(const variant<_Types...>& __lhs, const variant<_Types...>& __rhs) {
   using __variant_detail::__visitation::__variant;
   if (__lhs.index() != __rhs.index())
@@ -1485,6 +1491,11 @@ operator<=>(const variant<_Types...>& __lhs, const variant<_Types...>& __rhs) {
 #    endif // _LIBCPP_STD_VER >= 20
 
 template <class... _Types>
+#    if _LIBCPP_STD_VER >= 26
+  requires(requires(const _Types& __t) {
+    { __t != __t } -> __core_convertible_to<bool>;
+  } && ...)
+#    endif
 _LIBCPP_HIDE_FROM_ABI constexpr bool operator!=(const variant<_Types...>& __lhs, const variant<_Types...>& __rhs) {
   using __variant_detail::__visitation::__variant;
   if (__lhs.index() != __rhs.index())
@@ -1495,6 +1506,11 @@ _LIBCPP_HIDE_FROM_ABI constexpr bool operator!=(const variant<_Types...>& __lhs,
 }
 
 template <class... _Types>
+#    if _LIBCPP_STD_VER >= 26
+  requires(requires(const _Types& __t) {
+    { __t < __t } -> __core_convertible_to<bool>;
+  } && ...)
+#    endif
 _LIBCPP_HIDE_FROM_ABI constexpr bool operator<(const variant<_Types...>& __lhs, const variant<_Types...>& __rhs) {
   using __variant_detail::__visitation::__variant;
   if (__rhs.valueless_by_exception())
@@ -1509,6 +1525,11 @@ _LIBCPP_HIDE_FROM_ABI constexpr bool operator<(const variant<_Types...>& __lhs,
 }
 
 template <class... _Types>
+#    if _LIBCPP_STD_VER >= 26
+  requires(requires(const _Types& __t) {
+    { __t > __t } -> __core_convertible_to<bool>;
+  } && ...)
+#    endif
 _LIBCPP_HIDE_FROM_ABI constexpr bool operator>(const variant<_Types...>& __lhs, const variant<_Types...>& __rhs) {
   using __variant_detail::__visitation::__variant;
   if (__lhs.valueless_by_exception())
@@ -1523,6 +1544,11 @@ _LIBCPP_HIDE_FROM_ABI constexpr bool operator>(const variant<_Types...>& __lhs,
 }
 
 template <class... _Types>
+#    if _LIBCPP_STD_VER >= 26
+  requires(requires(const _Types& __t) {
+    { __t <= __t } -> __core_convertible_to<bool>;
+  } && ...)
+#    endif
 _LIBCPP_HIDE_FROM_ABI constexpr bool operator<=(const variant<_Types...>& __lhs, const variant<_Types...>& __rhs) {
   using __variant_detail::__visitation::__variant;
   if (__lhs.valueless_by_exception())
@@ -1537,6 +1563,11 @@ _LIBCPP_HIDE_FROM_ABI constexpr bool operator<=(const variant<_Types...>& __lhs,
 }
 
 template <class... _Types>
+#    if _LIBCPP_STD_VER >= 26
+  requires(requires(const _Types& __t) {
+    { __t >= __t } -> __core_convertible_to<bool>;
+  } && ...)
+#    endif
 _LIBCPP_HIDE_FROM_ABI constexpr bool operator>=(const variant<_Types...>& __lhs, const variant<_Types...>& __rhs) {
   using __variant_detail::__visitation::__variant;
   if (__rhs.valueless_by_exception())
diff --git a/libcxx/test/std/utilities/tuple/tuple.tuple/tuple.rel/eq.pass.cpp b/libcxx/test/std/utilities/tuple/tuple.tuple/tuple.rel/eq.pass.cpp
index a8de656313d45..baca362f7783b 100644
--- a/libcxx/test/std/utilities/tuple/tuple.tuple/tuple.rel/eq.pass.cpp
+++ b/libcxx/test/std/utilities/tuple/tuple.tuple/tuple.rel/eq.pass.cpp
@@ -16,12 +16,31 @@
 
 // UNSUPPORTED: c++03
 
-#include <tuple>
-#include <string>
+#include <array>
 #include <cassert>
+#include <tuple>
 
+#include "test_comparisons.h"
 #include "test_macros.h"
 
+#if TEST_STD_VER >= 26
+
+// Test SFINAE.
+
+static_assert(std::equality_comparable<std::tuple<EqualityComparable>>);
+static_assert(std::equality_comparable<std::tuple<EqualityComparable, EqualityComparable>>);
+
+static_assert(!std::equality_comparable<std::tuple<NonComparable>>);
+static_assert(!std::equality_comparable<std::tuple<EqualityComparable, NonComparable>>);
+static_assert(!std::equality_comparable<std::tuple<NonComparable, EqualityComparable>>);
+static_assert(!std::equality_comparable_with<std::tuple<EqualityComparable>, std::tuple<NonComparable>>);
+static_assert(!std::equality_comparable_with<std::tuple<NonComparable>, std::tuple<EqualityComparable>>);
+// Size mismatch.
+static_assert(!std::equality_comparable_with<std::tuple<EqualityComparable>, std::tuple<EqualityComparable, EqualityComparable>>);
+static_assert(!std::equality_comparable_with<std::tuple<EqualityComparable, EqualityComparable>, std::tuple<EqualityComparable>>);
+
+#endif
+
 int main(int, char**)
 {
     {
diff --git a/libcxx/test/std/utilities/tuple/tuple.tuple/tuple.rel/size_incompatible_comparison.verify.cpp b/libcxx/test/std/utilities/tuple/tuple.tuple/tuple.rel/size_incompatible_comparison.verify.cpp
index 851f6fcd1fbac..a5dbb6b313e6a 100644
--- a/libcxx/test/std/utilities/tuple/tuple.tuple/tuple.rel/size_incompatible_comparison.verify.cpp
+++ b/libcxx/test/std/utilities/tuple/tuple.tuple/tuple.rel/size_incompatible_comparison.verify.cpp
@@ -21,9 +21,15 @@
 
 #include <tuple>
 
+#include "test_macros.h"
+
+#if TEST_STD_VER >= 26
+// expected-no-diagnostics
+#else
 void f(std::tuple<int> t1, std::tuple<int, long> t2) {
   // We test only the core comparison operators and trust that the others
   // fall back on the same implementations prior to C++20.
   static_cast<void>(t1 == t2); // expected-error@*:* {{}}
   static_cast<void>(t1 < t2); // expected-error@*:* {{}}
 }
+#endif
diff --git a/libcxx/test/std/utilities/variant/variant.relops/relops.pass.cpp b/libcxx/test/std/utilities/variant/variant.relops/relops.pass.cpp
index c1a5b8e474a74..2c00703662687 100644
--- a/libcxx/test/std/utilities/variant/variant.relops/relops.pass.cpp
+++ b/libcxx/test/std/utilities/variant/variant.relops/relops.pass.cpp
@@ -39,8 +39,57 @@
 #include <utility>
 #include <variant>
 
+#include "test_comparisons.h"
 #include "test_macros.h"
 
+#if TEST_STD_VER >= 26
+
+// Test SFINAE.
+
+// ==
+static_assert(HasOperatorEqual<std::variant<EqualityComparable>>);
+static_assert(HasOperatorEqual<std::variant<EqualityComparable, int, long>>);
+
+static_assert(!HasOperatorEqual<std::variant<NonComparable>>);
+static_assert(!HasOperatorEqual<std::variant<NonComparable, EqualityComparable>>);
+
+// >
+static_assert(HasOperatorGreaterThan<std::variant<ThreeWayComparable>>);
+static_assert(HasOperatorGreaterThan<std::variant<ThreeWayComparable, int, long>>);
+
+static_assert(!HasOperatorGreaterThan<std::variant<NonComparable>>);
+static_assert(!HasOperatorGreaterThan<std::variant<NonComparable, ThreeWayComparable>>);
+
+// >=
+static_assert(HasOperatorGreaterThanEqual<std::variant<ThreeWayComparable>>);
+static_assert(HasOperatorGreaterThanEqual<std::variant<ThreeWayComparable, int, long>>);
+
+static_assert(!HasOperatorGreaterThanEqual<std::variant<NonComparable>>);
+static_assert(!HasOperatorGreaterThanEqual<std::variant<NonComparable, ThreeWayComparable>>);
+
+// <
+static_assert(HasOperatorLessThan<std::variant<ThreeWayComparable>>);
+static_assert(HasOperatorLessThan<std::variant<ThreeWayComparable, int, long>>);
+
+static_assert(!HasOperatorLessThan<std::variant<NonComparable>>);
+static_assert(!HasOperatorLessThan<std::variant<NonComparable, ThreeWayComparable>>);
+
+// <=
+static_assert(HasOperatorLessThanEqual<std::variant<ThreeWayComparable>>);
+static_assert(HasOperatorLessThanEqual<std::variant<ThreeWayComparable, int, long>>);
+
+static_assert(!HasOperatorLessThanEqual<std::variant<NonComparable>>);
+static_assert(!HasOperatorLessThanEqual<std::variant<NonComparable, ThreeWayComparable>>);
+
+// !=
+static_assert(HasOperatorNotEqual<std::variant<EqualityComparable>>);
+static_assert(HasOperatorNotEqual<std::variant<EqualityComparable, int, long>>);
+
+static_assert(!HasOperatorNotEqual<std::variant<NonComparable>>);
+static_assert(!HasOperatorNotEqual<std::variant<NonComparable, EqualityComparable>>);
+
+#endif
+
 #ifndef TEST_HAS_NO_EXCEPTIONS
 struct MakeEmptyT {
   MakeEmptyT() = default;
diff --git a/libcxx/test/support/test_comparisons.h b/libcxx/test/support/test_comparisons.h
index db6977a96a2fe..e37ab44828c70 100644
--- a/libcxx/test/support/test_comparisons.h
+++ b/libcxx/test/support/test_comparisons.h
@@ -268,6 +268,70 @@ struct PartialOrder {
   }
 };
 
-#endif
+template <typename T1, typename T2 = T1>
+concept HasOperatorEqual = requires(T1 t1, T2 t2) { t1 == t2; };
+
+template <typename T1, typename T2 = T1>
+concept HasOperatorGreaterThan = requires(T1 t1, T2 t2) { t1 > t2; };
+
+template <typename T1, typename T2 = T1>
+concept HasOperatorGreaterThanEqual = requires(T1 t1, T2 t2) { t1 >= t2; };
+template <typename T1, typename T2 = T1>
+concept HasOperatorLessThan = requires(T1 t1, T2 t2) { t1 < t2; };
+
+template <typename T1, typename T2 = T1>
+concept HasOperatorLessThanEqual = requires(T1 t1, T2 t2) { t1 <= t2; };
+
+template <typename T1, typename T2 = T1>
+concept HasOperatorNotEqual = requires(T1 t1, T2 t2) { t1 != t2; };
+
+template <typename T1, typename T2 = T1>
+concept HasOperatorSpaceship = requires(T1 t1, T2 t2) { t1 <=> t2; };
+
+struct NonComparable {};
+static_assert(!std::equality_comparable<NonComparable>);
+static_assert(!HasOperatorEqual<NonComparable>);
+static_assert(!HasOperatorGreaterThan<NonComparable>);
+static_assert(!HasOperatorGreaterThanEqual<NonComparable>);
+static_assert(!HasOperatorLessThan<NonComparable>);
+static_assert(!HasOperatorLessThanEqual<NonComparable>);
+static_assert(!HasOperatorNotEqual<NonComparable>);
+static_assert(!HasOperatorSpaceship<NonComparable>);
+
+class EqualityComparable {
+public:
+  constexpr EqualityComparable(int value) : value_{value} {};
+
+  friend constexpr bool operator==(const EqualityComparable&, const EqualityComparable&) noexcept = default;
+
+private:
+  int value_;
+};
+static_assert(std::equality_comparable<EqualityComparable>);
+static_assert(HasOperatorEqual<EqualityComparable>);
+static_assert(HasOperatorNotEqual<EqualityComparable>);
+
+class ThreeWayComparable {
+public:
+  constexpr ThreeWayComparable(int value) : value_{value} {};
+
+  friend constexpr bool operator==(const ThreeWayComparable&, const ThreeWayComparable&) noexcept = default;
+  friend constexpr std::strong_ordering
+  operator<=>(const ThreeWayComparable&, const ThreeWayComparable&) noexcept = default;
+
+private:
+  int value_;
+};
+static_assert(std::equality_comparable<ThreeWayComparable>);
+static_assert(std::three_way_comparable<ThreeWayComparable>);
+static_assert(HasOperatorEqual<ThreeWayComparable>);
+static_assert(HasOperatorGreaterThan<ThreeWayComparable>);
+static_assert(HasOperatorGreaterThanEqual<ThreeWayComparable>);
+static_assert(HasOperatorLessThan<ThreeWayComparable>);
+static_assert(HasOperatorLessThanEqual<ThreeWayComparable>);
+static_assert(HasOperatorNotEqual<ThreeWayComparable>);
+static_assert(HasOperatorSpaceship<ThreeWayComparable>);
+
+#endif // TEST_STD_VER >= 20
 
 #endif // TEST_COMPARISONS_H

@H-G-Hristov H-G-Hristov changed the title WIP [libc++]P2944R3: Constrained comparisions - variant and tuple [libc++]P2944R3: Constrained comparisions - variant and tuple Jun 15, 2025
@H-G-Hristov H-G-Hristov changed the title [libc++]P2944R3: Constrained comparisions - variant and tuple [libc++] P2944R3: Constrained comparisions - variant and tuple Jun 15, 2025
@H-G-Hristov H-G-Hristov marked this pull request as ready for review June 15, 2025 19:57
@H-G-Hristov H-G-Hristov requested a review from a team as a code owner June 15, 2025 19:57
Comment on lines 1164 to 1166
# if _LIBCPP_STD_VER < 26
static_assert(sizeof...(_Tp) == sizeof...(_Up), "Can't compare tuples of different sizes");
# endif
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure guarding this buys us much.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe guarding this doesn't won't noticeably improve throughput because sizeof...(_Tp) == sizeof...(_Up) is already checked. But it seems to me that the guarding will clearly tell code readers that there won't be redundant size check, and possibly reduce mental burden.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm seeing this from the other side actually. It's guarded, so my first instinct is to wonder whether this restriction has been lifted in C++26. Just having the static_assert unconditionally tells the reader that it's always a requirement. Whether this is enforced through a constraint as well doesn't really bother me that much.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From a maintainace point of view IMO this guard tells that if in year 2042, C++29 becomes the min supported mode, this could be easily removed.
Anyway I will remove the guard.

# if _LIBCPP_STD_VER >= 26
requires(requires(const _Tp& __t, const _Up& __u) {
{ __t == __u } -> __boolean_testable;
} && ...) && (sizeof...(_Tp) == sizeof...(_Up))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should use __all here. Otherwise we're potentially breaking currently working code, since fold expressions have a significantly lower limit than the number of allowed template arguments.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't this patch resolve your concerns: #132021

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does, but it's not yet available in most compilers. Note that I'm only asking for tuple to be changed, since it's notorious for being instantiated with a huge number of arguments.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK. In that case. I'll split tuple in a separate patch.

@H-G-Hristov H-G-Hristov requested a review from philnik777 June 24, 2025 22:01
Copy link
Contributor

@philnik777 philnik777 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please update the title. Otherwise LGTM

@Zingam Zingam changed the title [libc++] P2944R3: Constrained comparisions - variant and tuple [libc++] P2944R3: Constrained comparisions - variant Jun 25, 2025
@Zingam
Copy link
Contributor

Zingam commented Jun 25, 2025

Thanks! The test failures seem unrelated. Merging.

@Zingam Zingam merged commit f2f4b55 into llvm:main Jun 25, 2025
61 of 64 checks passed
@H-G-Hristov H-G-Hristov deleted the hgh/libcxx/P2944R3-constrained-equality-variant branch June 25, 2025 10:06
anthonyhatran pushed a commit to anthonyhatran/llvm-project that referenced this pull request Jun 26, 2025
This is a follow-up and depends on llvm#139368 being merged first.

Implements [P2944R3](https://wg21.link/P2944R3) partially, which adds
constrained comparisons to `std::variant`

Closes llvm#136769

Depends on llvm#139368

# References

[variant.relops](https://wg21.link/variant.relops)
Zingam pushed a commit that referenced this pull request Jun 27, 2025
Implements P2944R3 partially, which adds constrained comparisons
`std::tuple`.
The missing overloads introduced in [P2165R4](https://wg21.link/P2165R4)
are not implemented.

Uses
[`__all`](https://github.com/llvm/llvm-project/blob/f7af33a9eb5b3876f219075023dc9c565d75849b/libcxx/include/__type_traits/conjunction.h#L45)
instead of a fold expression, see comment:
#141396 (comment)

Relates to #136765

# References

[tuple.rel](https://wg21.link//tuple.rel)
llvm-sync bot pushed a commit to arm/arm-toolchain that referenced this pull request Jun 27, 2025
…677)

Implements P2944R3 partially, which adds constrained comparisons
`std::tuple`.
The missing overloads introduced in [P2165R4](https://wg21.link/P2165R4)
are not implemented.

Uses
[`__all`](https://github.com/llvm/llvm-project/blob/f7af33a9eb5b3876f219075023dc9c565d75849b/libcxx/include/__type_traits/conjunction.h#L45)
instead of a fold expression, see comment:
llvm/llvm-project#141396 (comment)

Relates to #136765

# References

[tuple.rel](https://wg21.link//tuple.rel)
rlavaee pushed a commit to rlavaee/llvm-project that referenced this pull request Jul 1, 2025
This is a follow-up and depends on llvm#139368 being merged first.

Implements [P2944R3](https://wg21.link/P2944R3) partially, which adds
constrained comparisons to `std::variant`

Closes llvm#136769

Depends on llvm#139368

# References

[variant.relops](https://wg21.link/variant.relops)
rlavaee pushed a commit to rlavaee/llvm-project that referenced this pull request Jul 1, 2025
Implements P2944R3 partially, which adds constrained comparisons
`std::tuple`.
The missing overloads introduced in [P2165R4](https://wg21.link/P2165R4)
are not implemented.

Uses
[`__all`](https://github.com/llvm/llvm-project/blob/f7af33a9eb5b3876f219075023dc9c565d75849b/libcxx/include/__type_traits/conjunction.h#L45)
instead of a fold expression, see comment:
llvm#141396 (comment)

Relates to llvm#136765

# References

[tuple.rel](https://wg21.link//tuple.rel)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++26 libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

P2944R3: Constrained equality - std::variant
5 participants