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.

Sign up for GitHub

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
merged 18 commits into from
Jun 25, 2025
+128 −6
Merged

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

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
a7517fd
[libc++] P2944R3: Constrained comparisions - optional and reference_w…
H-G-Hristov May 10, 2025
ab710f4
Implement tuple
H-G-Hristov May 13, 2025
dd32d6c
Cleanup
H-G-Hristov May 13, 2025
23ebfd7
`variant` more tests
H-G-Hristov May 13, 2025
f862580
Added missing include
H-G-Hristov Jun 5, 2025
ba27f7f
Merge branch 'main' into hgh/libcxx/P2944R3-constrained-equality-variant
H-G-Hristov Jun 5, 2025
8483790
Formatting
H-G-Hristov Jun 5, 2025
98b05c4
Fix
H-G-Hristov Jun 5, 2025
c64a4ec
Merge branch 'main' into hgh/libcxx/P2944R3-constrained-equality-variant
H-G-Hristov Jun 5, 2025
6e11bf0
Merge branch 'main' into hgh/libcxx/P2944R3-constrained-equality-variant
H-G-Hristov Jun 15, 2025
2b671b5
Cleanup
H-G-Hristov Jun 15, 2025
3cb270b
Fix CI
H-G-Hristov Jun 15, 2025
475d1f3
Merge branch 'main' into hgh/libcxx/P2944R3-constrained-equality-variant
H-G-Hristov Jun 15, 2025
d1cfb12
Update libcxx/test/std/utilities/variant/variant.relops/relops_bool_c…
H-G-Hristov Jun 15, 2025
3c9e9c3
Fix formatting
H-G-Hristov Jun 15, 2025
7d5ff27
Merge branch 'main' into hgh/libcxx/P2944R3-constrained-equality-variant
H-G-Hristov Jun 15, 2025
df79b72
Removed `tuple`
H-G-Hristov Jun 24, 2025
e08025e
Updated status pages
H-G-Hristov Jun 24, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions libcxx/include/tuple
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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))
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.

# 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
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.

return __tuple_equal<sizeof...(_Tp)>()(__x, __y);
}

Expand Down
31 changes: 31 additions & 0 deletions libcxx/include/variant
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -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())
Expand All @@ -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())
Expand All @@ -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())
Expand All @@ -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())
Expand All @@ -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())
Expand Down
23 changes: 21 additions & 2 deletions libcxx/test/std/utilities/tuple/tuple.tuple/tuple.rel/eq.pass.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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**)
{
{
Expand Down
6 changes: 6 additions & 0 deletions ...xx/test/std/utilities/tuple/tuple.tuple/tuple.rel/size_incompatible_comparison.verify.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
49 changes: 49 additions & 0 deletions libcxx/test/std/utilities/variant/variant.relops/relops.pass.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
66 changes: 65 additions & 1 deletion libcxx/test/support/test_comparisons.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Loading