Skip to content

Conversation

@H-G-Hristov
Copy link
Contributor

@H-G-Hristov H-G-Hristov commented Jun 15, 2025

Partially implements P2944R3 which adds constrained comparisons to std::optional.

Closes #136767

References

optional.relops
optional.comp.with.t

@H-G-Hristov H-G-Hristov changed the title WIP [libc+] Constrained eqaulity opational [libc++] P2944R3: Constrained comparisions - optional 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
@Zingam Zingam added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Jun 15, 2025
@llvmbot
Copy link
Member

llvmbot commented Jun 15, 2025

@llvm/pr-subscribers-libcxx

Author: Hristo Hristov (H-G-Hristov)

Changes

Partially implements P2944R3 which adds constrained comparisons to std::optional.

Closes #136767

References

optional.relops
optional.comp.with.t


Patch is 31.41 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/144249.diff

15 Files Affected:

  • (modified) libcxx/docs/Status/Cxx2cPapers.csv (+1-1)
  • (modified) libcxx/include/optional (+205-21)
  • (modified) libcxx/test/std/utilities/optional/optional.comp_with_t/equal.pass.cpp (+21)
  • (modified) libcxx/test/std/utilities/optional/optional.comp_with_t/greater.pass.cpp (+20)
  • (modified) libcxx/test/std/utilities/optional/optional.comp_with_t/greater_equal.pass.cpp (+20)
  • (modified) libcxx/test/std/utilities/optional/optional.comp_with_t/less_equal.pass.cpp (+20)
  • (modified) libcxx/test/std/utilities/optional/optional.comp_with_t/less_than.pass.cpp (+20)
  • (modified) libcxx/test/std/utilities/optional/optional.comp_with_t/not_equal.pass.cpp (+21)
  • (modified) libcxx/test/std/utilities/optional/optional.relops/equal.pass.cpp (+14)
  • (modified) libcxx/test/std/utilities/optional/optional.relops/greater_equal.pass.cpp (+13)
  • (modified) libcxx/test/std/utilities/optional/optional.relops/greater_than.pass.cpp (+13)
  • (modified) libcxx/test/std/utilities/optional/optional.relops/less_equal.pass.cpp (+13)
  • (modified) libcxx/test/std/utilities/optional/optional.relops/less_than.pass.cpp (+13)
  • (modified) libcxx/test/std/utilities/optional/optional.relops/not_equal.pass.cpp (+14)
  • (modified) libcxx/test/support/test_comparisons.h (+41)
diff --git a/libcxx/docs/Status/Cxx2cPapers.csv b/libcxx/docs/Status/Cxx2cPapers.csv
index 8a0417e120d75..77d09bebe3462 100644
--- a/libcxx/docs/Status/Cxx2cPapers.csv
+++ b/libcxx/docs/Status/Cxx2cPapers.csv
@@ -59,7 +59,7 @@
 "`P2248R8 <https://wg21.link/P2248R8>`__","Enabling list-initialization for algorithms","2024-03 (Tokyo)","","",""
 "`P2810R4 <https://wg21.link/P2810R4>`__","``is_debugger_present`` ``is_replaceable``","2024-03 (Tokyo)","","",""
 "`P1068R11 <https://wg21.link/P1068R11>`__","Vector API for random number generation","2024-03 (Tokyo)","","",""
-"`P2944R3 <https://wg21.link/P2944R3>`__","Comparisons for ``reference_wrapper``","2024-03 (Tokyo)","|Partial|","","The changes to ``optional``, ``tuple`` and ``variant`` are not yet implemented"
+"`P2944R3 <https://wg21.link/P2944R3>`__","Comparisons for ``reference_wrapper``","2024-03 (Tokyo)","|Partial|","","The changes to ``tuple`` and ``variant`` are not yet implemented"
 "`P2642R6 <https://wg21.link/P2642R6>`__","Padded ``mdspan`` layouts","2024-03 (Tokyo)","","",""
 "`P3029R1 <https://wg21.link/P3029R1>`__","Better ``mdspan``'s CTAD","2024-03 (Tokyo)","|Complete|","19",""
 "","","","","",""
diff --git a/libcxx/include/optional b/libcxx/include/optional
index fa32d75ef86dd..68053bdfb5df3 100644
--- a/libcxx/include/optional
+++ b/libcxx/include/optional
@@ -205,6 +205,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>
@@ -982,12 +983,23 @@ public:
 template <class _Tp>
 optional(_Tp) -> optional<_Tp>;
 
-// Comparisons between optionals
+// [optional.relops] Relational operators
+
+#    if _LIBCPP_STD_VER >= 26
+template < class _Tp, class _Up>
+#    else
 template <
     class _Tp,
     class _Up,
     enable_if_t<is_convertible_v<decltype(std::declval<const _Tp&>() == std::declval<const _Up&>()), bool>, int> = 0>
-_LIBCPP_HIDE_FROM_ABI constexpr bool operator==(const optional<_Tp>& __x, const optional<_Up>& __y) {
+#    endif
+_LIBCPP_HIDE_FROM_ABI constexpr bool operator==(const optional<_Tp>& __x, const optional<_Up>& __y)
+#    if _LIBCPP_STD_VER >= 26
+  requires requires {
+    { *__x == *__y } -> __core_convertible_to<bool>;
+  }
+#    endif
+{
   if (static_cast<bool>(__x) != static_cast<bool>(__y))
     return false;
   if (!static_cast<bool>(__x))
@@ -995,11 +1007,21 @@ _LIBCPP_HIDE_FROM_ABI constexpr bool operator==(const optional<_Tp>& __x, const
   return *__x == *__y;
 }
 
+#    if _LIBCPP_STD_VER >= 26
+template < class _Tp, class _Up>
+#    else
 template <
     class _Tp,
     class _Up,
     enable_if_t<is_convertible_v<decltype(std::declval<const _Tp&>() != std::declval<const _Up&>()), bool>, int> = 0>
-_LIBCPP_HIDE_FROM_ABI constexpr bool operator!=(const optional<_Tp>& __x, const optional<_Up>& __y) {
+#    endif
+_LIBCPP_HIDE_FROM_ABI constexpr bool operator!=(const optional<_Tp>& __x, const optional<_Up>& __y)
+#    if _LIBCPP_STD_VER >= 26
+  requires requires {
+    { *__x != *__y } -> __core_convertible_to<bool>;
+  }
+#    endif
+{
   if (static_cast<bool>(__x) != static_cast<bool>(__y))
     return true;
   if (!static_cast<bool>(__x))
@@ -1007,11 +1029,21 @@ _LIBCPP_HIDE_FROM_ABI constexpr bool operator!=(const optional<_Tp>& __x, const
   return *__x != *__y;
 }
 
+#    if _LIBCPP_STD_VER >= 26
+template < class _Tp, class _Up>
+#    else
 template <
     class _Tp,
     class _Up,
     enable_if_t<is_convertible_v<decltype(std::declval<const _Tp&>() < std::declval<const _Up&>()), bool>, int> = 0>
-_LIBCPP_HIDE_FROM_ABI constexpr bool operator<(const optional<_Tp>& __x, const optional<_Up>& __y) {
+#    endif
+_LIBCPP_HIDE_FROM_ABI constexpr bool operator<(const optional<_Tp>& __x, const optional<_Up>& __y)
+#    if _LIBCPP_STD_VER >= 26
+  requires requires {
+    { *__x < *__y } -> __core_convertible_to<bool>;
+  }
+#    endif
+{
   if (!static_cast<bool>(__y))
     return false;
   if (!static_cast<bool>(__x))
@@ -1019,11 +1051,21 @@ _LIBCPP_HIDE_FROM_ABI constexpr bool operator<(const optional<_Tp>& __x, const o
   return *__x < *__y;
 }
 
+#    if _LIBCPP_STD_VER >= 26
+template < class _Tp, class _Up>
+#    else
 template <
     class _Tp,
     class _Up,
     enable_if_t<is_convertible_v<decltype(std::declval<const _Tp&>() > std::declval<const _Up&>()), bool>, int> = 0>
-_LIBCPP_HIDE_FROM_ABI constexpr bool operator>(const optional<_Tp>& __x, const optional<_Up>& __y) {
+#    endif
+_LIBCPP_HIDE_FROM_ABI constexpr bool operator>(const optional<_Tp>& __x, const optional<_Up>& __y)
+#    if _LIBCPP_STD_VER >= 26
+  requires requires {
+    { *__x > *__y } -> __core_convertible_to<bool>;
+  }
+#    endif
+{
   if (!static_cast<bool>(__x))
     return false;
   if (!static_cast<bool>(__y))
@@ -1031,11 +1073,21 @@ _LIBCPP_HIDE_FROM_ABI constexpr bool operator>(const optional<_Tp>& __x, const o
   return *__x > *__y;
 }
 
+#    if _LIBCPP_STD_VER >= 26
+template < class _Tp, class _Up>
+#    else
 template <
     class _Tp,
     class _Up,
     enable_if_t<is_convertible_v<decltype(std::declval<const _Tp&>() <= std::declval<const _Up&>()), bool>, int> = 0>
-_LIBCPP_HIDE_FROM_ABI constexpr bool operator<=(const optional<_Tp>& __x, const optional<_Up>& __y) {
+#    endif
+_LIBCPP_HIDE_FROM_ABI constexpr bool operator<=(const optional<_Tp>& __x, const optional<_Up>& __y)
+#    if _LIBCPP_STD_VER >= 26
+  requires requires {
+    { *__x <= *__y } -> __core_convertible_to<bool>;
+  }
+#    endif
+{
   if (!static_cast<bool>(__x))
     return true;
   if (!static_cast<bool>(__y))
@@ -1043,11 +1095,21 @@ _LIBCPP_HIDE_FROM_ABI constexpr bool operator<=(const optional<_Tp>& __x, const
   return *__x <= *__y;
 }
 
+#    if _LIBCPP_STD_VER >= 26
+template < class _Tp, class _Up>
+#    else
 template <
     class _Tp,
     class _Up,
     enable_if_t<is_convertible_v<decltype(std::declval<const _Tp&>() >= std::declval<const _Up&>()), bool>, int> = 0>
-_LIBCPP_HIDE_FROM_ABI constexpr bool operator>=(const optional<_Tp>& __x, const optional<_Up>& __y) {
+#    endif
+_LIBCPP_HIDE_FROM_ABI constexpr bool operator>=(const optional<_Tp>& __x, const optional<_Up>& __y)
+#    if _LIBCPP_STD_VER >= 26
+  requires requires {
+    { *__x >= *__y } -> __core_convertible_to<bool>;
+  }
+#    endif
+{
   if (!static_cast<bool>(__y))
     return true;
   if (!static_cast<bool>(__x))
@@ -1067,7 +1129,8 @@ operator<=>(const optional<_Tp>& __x, const optional<_Up>& __y) {
 
 #    endif // _LIBCPP_STD_VER >= 20
 
-// Comparisons with nullopt
+// [optional.nullops] Comparison with nullopt
+
 template <class _Tp>
 _LIBCPP_HIDE_FROM_ABI constexpr bool operator==(const optional<_Tp>& __x, nullopt_t) noexcept {
   return !static_cast<bool>(__x);
@@ -1139,100 +1202,221 @@ _LIBCPP_HIDE_FROM_ABI constexpr strong_ordering operator<=>(const optional<_Tp>&
 
 #    endif // _LIBCPP_STD_VER <= 17
 
-// Comparisons with T
+// [optional.comp.with.t] Comparison with T
+
+#    if _LIBCPP_STD_VER >= 26
+template < class _Tp, class _Up>
+#    else
 template <
     class _Tp,
     class _Up,
     enable_if_t<is_convertible_v<decltype(std::declval<const _Tp&>() == std::declval<const _Up&>()), bool>, int> = 0>
-_LIBCPP_HIDE_FROM_ABI constexpr bool operator==(const optional<_Tp>& __x, const _Up& __v) {
+#    endif
+_LIBCPP_HIDE_FROM_ABI constexpr bool operator==(const optional<_Tp>& __x, const _Up& __v)
+#    if _LIBCPP_STD_VER >= 26
+  requires(!__is_std_optional<_Up>::value) && requires {
+    { *__x == __v } -> __core_convertible_to<bool>;
+  }
+#    endif
+{
   return static_cast<bool>(__x) ? *__x == __v : false;
 }
 
+#    if _LIBCPP_STD_VER >= 26
+template < class _Tp, class _Up>
+#    else
 template <
     class _Tp,
     class _Up,
     enable_if_t<is_convertible_v<decltype(std::declval<const _Tp&>() == std::declval<const _Up&>()), bool>, int> = 0>
-_LIBCPP_HIDE_FROM_ABI constexpr bool operator==(const _Tp& __v, const optional<_Up>& __x) {
+#    endif
+_LIBCPP_HIDE_FROM_ABI constexpr bool operator==(const _Tp& __v, const optional<_Up>& __x)
+#    if _LIBCPP_STD_VER >= 26
+  requires(!__is_std_optional<_Tp>::value) && requires {
+    { __v == *__x } -> __core_convertible_to<bool>;
+  }
+#    endif
+{
   return static_cast<bool>(__x) ? __v == *__x : false;
 }
 
+#    if _LIBCPP_STD_VER >= 26
+template < class _Tp, class _Up>
+#    else
 template <
     class _Tp,
     class _Up,
     enable_if_t<is_convertible_v<decltype(std::declval<const _Tp&>() != std::declval<const _Up&>()), bool>, int> = 0>
-_LIBCPP_HIDE_FROM_ABI constexpr bool operator!=(const optional<_Tp>& __x, const _Up& __v) {
+#    endif
+_LIBCPP_HIDE_FROM_ABI constexpr bool operator!=(const optional<_Tp>& __x, const _Up& __v)
+#    if _LIBCPP_STD_VER >= 26
+  requires(!__is_std_optional<_Up>::value) && requires {
+    { *__x != __v } -> __core_convertible_to<bool>;
+  }
+#    endif
+{
   return static_cast<bool>(__x) ? *__x != __v : true;
 }
 
+#    if _LIBCPP_STD_VER >= 26
+template < class _Tp, class _Up>
+#    else
 template <
     class _Tp,
     class _Up,
     enable_if_t<is_convertible_v<decltype(std::declval<const _Tp&>() != std::declval<const _Up&>()), bool>, int> = 0>
-_LIBCPP_HIDE_FROM_ABI constexpr bool operator!=(const _Tp& __v, const optional<_Up>& __x) {
+#    endif
+_LIBCPP_HIDE_FROM_ABI constexpr bool operator!=(const _Tp& __v, const optional<_Up>& __x)
+#    if _LIBCPP_STD_VER >= 26
+  requires(!__is_std_optional<_Tp>::value) && requires {
+    { __v != *__x } -> __core_convertible_to<bool>;
+  }
+#    endif
+{
   return static_cast<bool>(__x) ? __v != *__x : true;
 }
 
+#    if _LIBCPP_STD_VER >= 26
+template < class _Tp, class _Up>
+#    else
 template <
     class _Tp,
     class _Up,
     enable_if_t<is_convertible_v<decltype(std::declval<const _Tp&>() < std::declval<const _Up&>()), bool>, int> = 0>
-_LIBCPP_HIDE_FROM_ABI constexpr bool operator<(const optional<_Tp>& __x, const _Up& __v) {
+#    endif
+_LIBCPP_HIDE_FROM_ABI constexpr bool operator<(const optional<_Tp>& __x, const _Up& __v)
+#    if _LIBCPP_STD_VER >= 26
+  requires(!__is_std_optional<_Up>::value) && requires {
+    { *__x < __v } -> __core_convertible_to<bool>;
+  }
+#    endif
+{
   return static_cast<bool>(__x) ? *__x < __v : true;
 }
 
+#    if _LIBCPP_STD_VER >= 26
+template < class _Tp, class _Up>
+#    else
 template <
     class _Tp,
     class _Up,
     enable_if_t<is_convertible_v<decltype(std::declval<const _Tp&>() < std::declval<const _Up&>()), bool>, int> = 0>
-_LIBCPP_HIDE_FROM_ABI constexpr bool operator<(const _Tp& __v, const optional<_Up>& __x) {
+#    endif
+_LIBCPP_HIDE_FROM_ABI constexpr bool operator<(const _Tp& __v, const optional<_Up>& __x)
+#    if _LIBCPP_STD_VER >= 26
+  requires(!__is_std_optional<_Tp>::value) && requires {
+    { __v < *__x } -> __core_convertible_to<bool>;
+  }
+#    endif
+{
   return static_cast<bool>(__x) ? __v < *__x : false;
 }
 
+#    if _LIBCPP_STD_VER >= 26
+template < class _Tp, class _Up>
+#    else
 template <
     class _Tp,
     class _Up,
     enable_if_t<is_convertible_v<decltype(std::declval<const _Tp&>() <= std::declval<const _Up&>()), bool>, int> = 0>
-_LIBCPP_HIDE_FROM_ABI constexpr bool operator<=(const optional<_Tp>& __x, const _Up& __v) {
+#    endif
+_LIBCPP_HIDE_FROM_ABI constexpr bool operator<=(const optional<_Tp>& __x, const _Up& __v)
+#    if _LIBCPP_STD_VER >= 26
+  requires(!__is_std_optional<_Up>::value) && requires {
+    { *__x <= __v } -> __core_convertible_to<bool>;
+  }
+#    endif
+{
   return static_cast<bool>(__x) ? *__x <= __v : true;
 }
 
+#    if _LIBCPP_STD_VER >= 26
+template < class _Tp, class _Up>
+#    else
 template <
     class _Tp,
     class _Up,
     enable_if_t<is_convertible_v<decltype(std::declval<const _Tp&>() <= std::declval<const _Up&>()), bool>, int> = 0>
-_LIBCPP_HIDE_FROM_ABI constexpr bool operator<=(const _Tp& __v, const optional<_Up>& __x) {
+#    endif
+_LIBCPP_HIDE_FROM_ABI constexpr bool operator<=(const _Tp& __v, const optional<_Up>& __x)
+#    if _LIBCPP_STD_VER >= 26
+  requires(!__is_std_optional<_Tp>::value) && requires {
+    { __v <= *__x } -> __core_convertible_to<bool>;
+  }
+#    endif
+{
   return static_cast<bool>(__x) ? __v <= *__x : false;
 }
 
+#    if _LIBCPP_STD_VER >= 26
+template < class _Tp, class _Up>
+#    else
 template <
     class _Tp,
     class _Up,
     enable_if_t<is_convertible_v<decltype(std::declval<const _Tp&>() > std::declval<const _Up&>()), bool>, int> = 0>
-_LIBCPP_HIDE_FROM_ABI constexpr bool operator>(const optional<_Tp>& __x, const _Up& __v) {
+#    endif
+_LIBCPP_HIDE_FROM_ABI constexpr bool operator>(const optional<_Tp>& __x, const _Up& __v)
+#    if _LIBCPP_STD_VER >= 26
+  requires(!__is_std_optional<_Up>::value) && requires {
+    { *__x > __v } -> __core_convertible_to<bool>;
+  }
+#    endif
+{
   return static_cast<bool>(__x) ? *__x > __v : false;
 }
 
+#    if _LIBCPP_STD_VER >= 26
+template < class _Tp, class _Up>
+#    else
 template <
     class _Tp,
     class _Up,
     enable_if_t<is_convertible_v<decltype(std::declval<const _Tp&>() > std::declval<const _Up&>()), bool>, int> = 0>
-_LIBCPP_HIDE_FROM_ABI constexpr bool operator>(const _Tp& __v, const optional<_Up>& __x) {
+#    endif
+_LIBCPP_HIDE_FROM_ABI constexpr bool operator>(const _Tp& __v, const optional<_Up>& __x)
+#    if _LIBCPP_STD_VER >= 26
+  requires(!__is_std_optional<_Tp>::value) && requires {
+    { __v > *__x } -> __core_convertible_to<bool>;
+  }
+#    endif
+{
   return static_cast<bool>(__x) ? __v > *__x : true;
 }
 
+#    if _LIBCPP_STD_VER >= 26
+template < class _Tp, class _Up>
+#    else
 template <
     class _Tp,
     class _Up,
     enable_if_t<is_convertible_v<decltype(std::declval<const _Tp&>() >= std::declval<const _Up&>()), bool>, int> = 0>
-_LIBCPP_HIDE_FROM_ABI constexpr bool operator>=(const optional<_Tp>& __x, const _Up& __v) {
+#    endif
+_LIBCPP_HIDE_FROM_ABI constexpr bool operator>=(const optional<_Tp>& __x, const _Up& __v)
+#    if _LIBCPP_STD_VER >= 26
+  requires(!__is_std_optional<_Up>::value) && requires {
+    { *__x >= __v } -> __core_convertible_to<bool>;
+  }
+#    endif
+{
   return static_cast<bool>(__x) ? *__x >= __v : false;
 }
 
+#    if _LIBCPP_STD_VER >= 26
+template < class _Tp, class _Up>
+#    else
 template <
     class _Tp,
     class _Up,
     enable_if_t<is_convertible_v<decltype(std::declval<const _Tp&>() >= std::declval<const _Up&>()), bool>, int> = 0>
-_LIBCPP_HIDE_FROM_ABI constexpr bool operator>=(const _Tp& __v, const optional<_Up>& __x) {
+#    endif
+_LIBCPP_HIDE_FROM_ABI constexpr bool operator>=(const _Tp& __v, const optional<_Up>& __x)
+#    if _LIBCPP_STD_VER >= 26
+  requires(!__is_std_optional<_Tp>::value) && requires {
+    { __v >= *__x } -> __core_convertible_to<bool>;
+  }
+#    endif
+{
   return static_cast<bool>(__x) ? __v >= *__x : true;
 }
 
diff --git a/libcxx/test/std/utilities/optional/optional.comp_with_t/equal.pass.cpp b/libcxx/test/std/utilities/optional/optional.comp_with_t/equal.pass.cpp
index 0636da7bcdf23..54965b270dcce 100644
--- a/libcxx/test/std/utilities/optional/optional.comp_with_t/equal.pass.cpp
+++ b/libcxx/test/std/utilities/optional/optional.comp_with_t/equal.pass.cpp
@@ -14,8 +14,29 @@
 
 #include <optional>
 
+#include "test_comparisons.h"
 #include "test_macros.h"
 
+#if TEST_STD_VER >= 26
+
+// Test SFINAE.
+
+static_assert(HasOperatorEqual<int, std::optional<int>>);
+static_assert(HasOperatorEqual<int, std::optional<EqualityComparable>>);
+static_assert(HasOperatorEqual<EqualityComparable, std::optional<EqualityComparable>>);
+
+static_assert(!HasOperatorEqual<NonComparable, std::optional<NonComparable>>);
+static_assert(!HasOperatorEqual<NonComparable, std::optional<EqualityComparable>>);
+
+static_assert(HasOperatorEqual<std::optional<int>, int>);
+static_assert(HasOperatorEqual<std::optional<EqualityComparable>, int>);
+static_assert(HasOperatorEqual<std::optional<EqualityComparable>, EqualityComparable>);
+
+static_assert(!HasOperatorEqual<std::optional<NonComparable>, NonComparable>);
+static_assert(!HasOperatorEqual<std::optional<EqualityComparable>, NonComparable>);
+
+#endif
+
 using std::optional;
 
 struct X {
diff --git a/libcxx/test/std/utilities/optional/optional.comp_with_t/greater.pass.cpp b/libcxx/test/std/utilities/optional/optional.comp_with_t/greater.pass.cpp
index 2010157a8d011..b63b85b000930 100644
--- a/libcxx/test/std/utilities/optional/optional.comp_with_t/greater.pass.cpp
+++ b/libcxx/test/std/utilities/optional/optional.comp_with_t/greater.pass.cpp
@@ -14,8 +14,28 @@
 
 #include <optional>
 
+#include "test_comparisons.h"
 #include "test_macros.h"
 
+#if TEST_STD_VER >= 26
+
+// Test SFINAE.
+static_assert(HasOperatorGreaterThan<std::optional<ThreeWayComparable>, int>);
+static_assert(HasOperatorGreaterThan<std::optional<ThreeWayComparable>, ThreeWayComparable>);
+
+static_assert(!HasOperatorGreaterThan<std::optional<NonComparable>, NonComparable>);
+static_assert(!HasOperatorGreaterThan<std::optional<ThreeWayComparable>, NonComparable>);
+static_assert(!HasOperatorGreaterThan<std::optional<NonComparable>, ThreeWayComparable>);
+
+static_assert(HasOperatorGreaterThan<int, std::optional<ThreeWayComparable>>);
+static_assert(HasOperatorGreaterThan<ThreeWayComparable, std::optional<ThreeWayComparable>>);
+
+static_assert(!HasOperatorGreaterThan<NonComparable, std::optional<NonComparable>>);
+static_assert(!HasOperatorGreaterThan<NonComparable, std::optional<ThreeWayComparable>>);
+static_assert(!HasOperatorGreaterThan<ThreeWayComparable, std::optional<NonComparable>>);
+
+#endif
+
 using std::optional;
 
 struct X {
diff --git a/libcxx/test/std/utilities/optional/optional.comp_with_t/greater_equal.pass.cpp b/libcxx/test/std/utilities/optional/optional.comp_with_t/greater_equal.pass.cpp
index 2e1e9b9316111..bb1a8c551d3d5 100644
--- a/libcxx/test/std/utilities/optional/optional.comp_with_t/greater_equal.pass.cpp
+++ b/libcxx/test/std/utilities/optional/optional.comp_with_t/greater_equal.pass.cpp
@@ -14,8 +14,28 @@
 
 #include <optional>
 
+#include "test_comparisons.h"
 #include "test_macros.h"
 
+#if TEST_STD_VER >= 26
+
+// Test SFINAE.
+static_assert(HasOperatorGreaterThanEqual<std::optional<ThreeWayComparable>, int>);
+static_assert(HasOperatorGreaterThanEqual<std::optional<ThreeWayComparable>, ThreeWayComparable>);
+
+static_assert(!HasOperatorGreaterThanEqual<std::optional<NonComparable>, NonComparable>);
+static_assert(!HasOperatorGreaterThanEqual<std::optional<ThreeWayComparable>, NonComparable>);
+static_assert(!HasOperatorGreaterThanEqual<std::optional<NonComparable>, ThreeWayComparable>);
+
+static_assert(HasOperatorGreaterThanEqual<int, std::optional<ThreeWayComparable>>);
+static_assert(HasOperatorGreaterThanEqual<ThreeWayComparable, std::optional<ThreeWayComparable>>);
+
+static_assert(!HasOperatorGreaterThanEqual<NonComparable, std::optional<NonComparable>>);
+static_assert(!HasOperatorGreaterThanEqual<NonComparable, std::optional<ThreeWayComparable>>);
+static_assert(!HasOperatorGreaterThanEqual<ThreeWayComparable, std::optional<NonComparable>>);
+
+#endif
+
 using std::optional;
 
 struct X {
diff --git a/libcxx/test/std/utilities/optional/optional.comp_with_t/less_equal.pass.cpp b/libcxx/test/std/utilities/optional/optional.comp_with_t/less_equal.pass.cpp
index 7026c24de5dbf..8b08ec3deea5a 100644
--- a/libcxx/test/std/utilities/optional/optional.comp_with_t/less_equal.pass.cpp
+++ b/libcxx/test/std/utilities/optional/optional.comp_with_t/less_equal.pass.cpp
@@ -14,8 +14,28 @@
 
 #include <optional>
 
+#include "test_comparisons.h"
 #include "test_macros.h"
 
+#if TEST_STD_VER >= 26
+
+// Test SFINAE.
+static_assert(HasOperatorLessThanEqual<std::optional<ThreeWayComparable>, int>);
+static_assert(HasOperatorLessThanEqual<std::optional<ThreeWayComparable>, ThreeWayComparable>);
+
+static_assert(!HasOperatorLessThanEqual<std::optional<NonComparable>, NonComparable>);
+static_assert(!HasOperatorLessThanEqual<std::optional<ThreeWayComparable>, NonComparable>);
+static_assert(!HasOperatorLessThanEqual<std::optional<NonComparable>, ThreeWayComparable>);
+
+static_assert(HasOperatorLessThanEqual<int, std::optional<ThreeWayComparable>>);
+static_assert(HasOperatorLessThanEqual<ThreeWayComparable, std::optional<ThreeWayComparable>...
[truncated]

_LIBCPP_HIDE_FROM_ABI constexpr bool operator==(const optional<_Tp>& __x, const optional<_Up>& __y)
# if _LIBCPP_STD_VER >= 26
requires requires {
{ *__x == *__y } -> __core_convertible_to<bool>;
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 either have a bug in the old constraint or the new. The paper simply changes Mandates to Constraints. That shouldn't change how the constraint looks. I'd also strongly encourage to only have a single constraint style and not different ones. That should significantly reduce the diff here. I think in the end we should simply have a single constraint for all language modes, since the wording didn't fundamentally change.

Copy link
Contributor

@frederick-vs-ja frederick-vs-ja Jun 16, 2025

Choose a reason for hiding this comment

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

Another question: Do we want to keep the constraints in old modes? They don't seem conforming in old modes, unless we add some deleted overloads as fallback.

Copy link
Contributor

Choose a reason for hiding this comment

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

Why would they be non-conforming?

Copy link
Contributor

Choose a reason for hiding this comment

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

Because when the types failed to satisfy the constraints, another (probably user-provided) overloads might be selected. Previously, the standard overloads were possibly required to be selected and cause error.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd really like to see an example of that. Even then, given [intro.compliance.general]/p8 I don't see how it would be non-conforming.

Copy link
Contributor

Choose a reason for hiding this comment

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

Here's an example. Godbolt link (the 2nd example has the constraints for operator== commented out):

#include <optional>
#include <type_traits>

namespace my {
  struct foo {};
  template<class T, class U>
    requires (!std::is_same_v<std::remove_cv_t<T>, foo>)
          && (!std::is_same_v<std::remove_cv_t<U>, foo>)
  bool operator==(const T&, const U&) { return true; }
}

int main() {
  using ofoo = std::optional<my::foo>;
  (void)(ofoo{} == ofoo{});
  // C++20/23: should select the standard overload and be ill-formed
  // C++26: should select the user-provided overload
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like that the controversy about constraints before C++26 won't be resolved soon, and other implementations also constrain these operators since C++17. Perhaps at this moment we can use __is_core_convertible and traditional SFINAE in all modes. @H-G-Hristov @Zingam

Copy link
Contributor

Choose a reason for hiding this comment

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

@frederick-vs-ja OK. Thanks.

Copy link
Member

Choose a reason for hiding this comment

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

That's a fantastic example @frederick-vs-ja , thank you.

@github-actions
Copy link

github-actions bot commented Jun 25, 2025

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


#if _LIBCPP_STD_VER >= 17

template <class _Tp>
Copy link
Member

Choose a reason for hiding this comment

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

Why not just use __is_core_convertible directly? The additional instantiations might not be needed.

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 it's better to introduce __is_core_convertible_v first and implement __is_core_convertible with it, then use __is_core_convertible_v<decltype(...), bool> in the constraints.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@frederick-vs-ja I implemented __is_core_convertible_v for C++17 onward similar to other traits. I don't understand why you suggest to do it in reverse? Everything seems fine as it is right now. Also we have the concept __core_convertible_to, do we want to keep it or replace it with __is_core_convertible_v for insignificant code reduction (of redundancy)?

Copy link
Contributor

Choose a reason for hiding this comment

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

Variable templates are better on compiler throughput than class template, and we're even relying on using variable template in C++03 mode as an extension. So I guess it would be better to have __is_core_convertible_v in old modes (and perform SFINAE in it) and use __is_core_convertible only with _And and its friends.

@H-G-Hristov H-G-Hristov closed this Jul 6, 2025
@H-G-Hristov H-G-Hristov deleted the hgh/libcxx/P2944R3-optional-constrained-equality-part_2 branch July 6, 2025 06:24
@H-G-Hristov H-G-Hristov restored the hgh/libcxx/P2944R3-optional-constrained-equality-part_2 branch July 6, 2025 06:26
@H-G-Hristov H-G-Hristov reopened this Jul 6, 2025
@ldionne ldionne changed the title [libc++] P2944R3: Constrained comparisions - optional [libc++] P2944R3: Constrained comparisons - optional Jul 7, 2025
Comment on lines +33 to +34
template <class _Tp, class _Up>
using __is_core_convertible _LIBCPP_NODEBUG = integral_constant<bool, __is_core_convertible_v<_Tp, _Up> >;
Copy link
Contributor

Choose a reason for hiding this comment

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

It's a bit unusual to just use an alias template instead of inheritance from the bool_constant. But this should be fine since no compilation failures happens, and is possibly better due to reduction of instantiation.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, the difference between inheritance and aliasing seems to be laziness of instantiation. If it's confirmed that shortcut instantiation is not so that desired, we can even remove __is_core_convertible and consistently use __is_core_convertible_v. (The changes should belong to another PR, though.)

@Zingam Zingam requested a review from philnik777 July 11, 2025 03:43
@Zingam
Copy link
Contributor

Zingam commented Jul 14, 2025

@philnik777 If you are OK, can we get this in 21? I don't mind leaving it for later too.
This basically completes the constrained comparisons but the tuples overload introduced in P2165R4. Should we add a release note with a remark now?

@frederick-vs-ja
Copy link
Contributor

This basically completes the constrained comparisons but the tuples overload introduced in P2165R4.

I'm implementing the new overloads now. I want P2944R3 to be completed in LLVM 21.

@philnik777 philnik777 merged commit 5fc844a into llvm:main Jul 15, 2025
78 checks passed
@Zingam
Copy link
Contributor

Zingam commented Jul 15, 2025

Thanks!

@H-G-Hristov H-G-Hristov deleted the hgh/libcxx/P2944R3-optional-constrained-equality-part_2 branch July 22, 2025 15:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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::optional

6 participants