From 53d8eec3862e888f908cb24721066ffe28a360ba Mon Sep 17 00:00:00 2001 From: Daniel Winsor Date: Wed, 17 Feb 2021 13:38:47 -0800 Subject: [PATCH 1/5] Spaceship operator for variant --- stl/inc/variant | 56 ++++++++++++++++++---- tests/std/tests/P1614R2_spaceship/test.cpp | 17 +++++++ 2 files changed, 64 insertions(+), 9 deletions(-) diff --git a/stl/inc/variant b/stl/inc/variant index bccf5add82a..0803a4537b9 100644 --- a/stl/inc/variant +++ b/stl/inc/variant @@ -20,6 +20,11 @@ #include #include +#ifdef _HAS_CXX20 +#include +#include +#endif // _HAS_CXX20 + #pragma pack(push, _CRT_PACKING) #pragma warning(push, _STL_WARNING_LEVEL) #pragma warning(disable : _STL_DISABLED_WARNINGS) @@ -1365,13 +1370,13 @@ _NODISCARD constexpr add_pointer_t get_if( } // RELATIONAL OPERATORS [variant.relops] -template +template struct _Variant_relop_visitor { // evaluate _Op with the contained value of two variants that hold the same alternative const _Variant_storage<_Types...>& _Left; template - _NODISCARD constexpr bool operator()(_Tagged _Right) const noexcept( - disjunction_v, is_nothrow_invocable_r>) { + _NODISCARD constexpr _Result operator()(_Tagged _Right) const noexcept( + disjunction_v, is_nothrow_invocable_r<_Result, _Op, const _Ty&, const _Ty&>>) { // determine the relationship between the stored values of _Left and _Right // pre: _Left.index() == _Idx && _Right.index() == _Idx if constexpr (_Idx != variant_npos) { @@ -1387,7 +1392,7 @@ template _NODISCARD constexpr bool operator==(const variant<_Types...>& _Left, const variant<_Types...>& _Right) noexcept( conjunction_v, const _Types&, const _Types&>...>) /* strengthened */ { // determine if the arguments are both valueless or contain equal values - using _Visitor = _Variant_relop_visitor, _Types...>; + using _Visitor = _Variant_relop_visitor, bool, _Types...>; const size_t _Right_index = _Right.index(); return _Left.index() == _Right_index && _Variant_raw_visit(_Right_index, _Right._Storage(), _Visitor{_Left._Storage()}); @@ -1397,7 +1402,7 @@ template _NODISCARD constexpr bool operator!=(const variant<_Types...>& _Left, const variant<_Types...>& _Right) noexcept( conjunction_v, const _Types&, const _Types&>...>) /* strengthened */ { // determine if the arguments have different active alternatives or contain unequal values - using _Visitor = _Variant_relop_visitor, _Types...>; + using _Visitor = _Variant_relop_visitor, bool, _Types...>; const size_t _Right_index = _Right.index(); return _Left.index() != _Right_index || _Variant_raw_visit(_Right_index, _Right._Storage(), _Visitor{_Left._Storage()}); @@ -1408,7 +1413,7 @@ _NODISCARD constexpr bool operator<(const variant<_Types...>& _Left, const varia conjunction_v, const _Types&, const _Types&>...>) /* strengthened */ { // determine if _Left has a lesser index(), or equal index() and lesser // contained value than _Right - using _Visitor = _Variant_relop_visitor, _Types...>; + using _Visitor = _Variant_relop_visitor, bool, _Types...>; const size_t _Left_offset = _Left.index() + 1; const size_t _Right_offset = _Right.index() + 1; return _Left_offset < _Right_offset @@ -1421,7 +1426,7 @@ _NODISCARD constexpr bool operator>(const variant<_Types...>& _Left, const varia conjunction_v, const _Types&, const _Types&>...>) /* strengthened */ { // determine if _Left has a greater index(), or equal index() and // greater contained value than _Right - using _Visitor = _Variant_relop_visitor, _Types...>; + using _Visitor = _Variant_relop_visitor, bool, _Types...>; const size_t _Left_offset = _Left.index() + 1; const size_t _Right_offset = _Right.index() + 1; return _Left_offset > _Right_offset @@ -1434,7 +1439,7 @@ _NODISCARD constexpr bool operator<=(const variant<_Types...>& _Left, const vari conjunction_v, const _Types&, const _Types&>...>) /* strengthened */ { // determine if _Left's index() is less than _Right's, or equal and // _Left contains a value less than or equal to _Right - using _Visitor = _Variant_relop_visitor, _Types...>; + using _Visitor = _Variant_relop_visitor, bool, _Types...>; const size_t _Left_offset = _Left.index() + 1; const size_t _Right_offset = _Right.index() + 1; return _Left_offset < _Right_offset @@ -1447,7 +1452,7 @@ _NODISCARD constexpr bool operator>=(const variant<_Types...>& _Left, const vari conjunction_v, const _Types&, const _Types&>...>) /* strengthened */ { // determine if _Left's index() is greater than _Right's, or equal and // _Left contains a value greater than or equal to _Right - using _Visitor = _Variant_relop_visitor, _Types...>; + using _Visitor = _Variant_relop_visitor, bool, _Types...>; const size_t _Left_offset = _Left.index() + 1; const size_t _Right_offset = _Right.index() + 1; return _Left_offset > _Right_offset @@ -1455,6 +1460,31 @@ _NODISCARD constexpr bool operator>=(const variant<_Types...>& _Left, const vari && _Variant_raw_visit(_Right_offset - 1, _Right._Storage(), _Visitor{_Left._Storage()})); } +#ifdef _HAS_CXX20 +struct _Variant_relop_3way_comparison { + template + compare_three_way_result_t<_Ty> operator()(const _Ty& _Left, const _Ty& _Right) { + return _Left <=> _Right; + } +}; + +template +_NODISCARD constexpr common_comparison_category_t...> + operator<=>(const variant<_Types...>& _Left, const variant<_Types...>& _Right) noexcept( + conjunction_v...>, + _Variant_relop_3way_comparison, const _Types&, const _Types&>...>) /* strengthened */ { + // determine the three-way comparison of _Left's and _Right's index, if equal + // return the three-way comparison of the contained value of _Left and _Right. + using _Visitor = _Variant_relop_visitor<_Variant_relop_3way_comparison, + common_comparison_category_t...>, _Types...>; + const size_t _Left_offset = _Left.index() + 1; + const size_t _Right_offset = _Right.index() + 1; + const auto _Offset_order = _Left_offset <=> _Right_offset; + return _Offset_order != 0 ? _Offset_order + : _Variant_raw_visit(_Right_offset - 1, _Right._Storage(), _Visitor{_Left._Storage()}); +} +#endif // _HAS_CXX20 + // VISITATION [variant.visit] template inline constexpr size_t _Variant_total_states = @@ -1698,6 +1728,8 @@ struct monostate {}; _NODISCARD constexpr bool operator==(monostate, monostate) noexcept { return true; } + +#if !_HAS_CXX20 _NODISCARD constexpr bool operator!=(monostate, monostate) noexcept { return false; } @@ -1713,6 +1745,12 @@ _NODISCARD constexpr bool operator<=(monostate, monostate) noexcept { _NODISCARD constexpr bool operator>=(monostate, monostate) noexcept { return true; } +#else // ^^^ _HAS_CXX20 vvv + +_NODISCARD constexpr strong_ordering operator<=>(monostate, monostate) noexcept { + return strong_ordering::equal; +} +#endif // !_HAS_CXX20 // SPECIALIZED ALGORITHMS [variant.specalg] template #include #include +#include #include template @@ -484,6 +485,22 @@ void ordering_test_cases() { spaceship_test(c_mem[0], c_mem[0], c_mem[1]); } + { // variant + using V = std::variant; + constexpr V v0_0(std::in_place_index<0>, 0); + constexpr V v0_1(std::in_place_index<0>, 1); + constexpr V v1_0(std::in_place_index<1>, 0); + constexpr V v1_1(std::in_place_index<1>, 1); + + spaceship_test(v0_0, v0_0, v0_1); + spaceship_test(v0_1, v0_1, v1_0); + spaceship_test(v1_0, v1_0, v1_1); + + using M = std::monostate; + constexpr M m1{}; + constexpr M m2{}; + static_assert((m1 <=> m2) == 0, ""); + } { // slice std::slice a1(2, 3, 4); std::slice a2(2, 3, 4); From 0bf12f3c286a320defbbb9ba76d1d85faf551de2 Mon Sep 17 00:00:00 2001 From: Daniel Winsor Date: Wed, 17 Feb 2021 13:57:08 -0800 Subject: [PATCH 2/5] formating --- stl/inc/variant | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/stl/inc/variant b/stl/inc/variant index 0803a4537b9..0e6035cf8c9 100644 --- a/stl/inc/variant +++ b/stl/inc/variant @@ -1375,8 +1375,9 @@ struct _Variant_relop_visitor { // evaluate _Op with the contained value of two const _Variant_storage<_Types...>& _Left; template - _NODISCARD constexpr _Result operator()(_Tagged _Right) const noexcept( - disjunction_v, is_nothrow_invocable_r<_Result, _Op, const _Ty&, const _Ty&>>) { + _NODISCARD constexpr _Result operator()(_Tagged _Right) const + noexcept(disjunction_v, + is_nothrow_invocable_r<_Result, _Op, const _Ty&, const _Ty&>>) { // determine the relationship between the stored values of _Left and _Right // pre: _Left.index() == _Idx && _Right.index() == _Idx if constexpr (_Idx != variant_npos) { From 13c01a196c5e953fd0cbfee0b6c3bddda199703d Mon Sep 17 00:00:00 2001 From: Daniel Winsor Date: Wed, 17 Feb 2021 17:44:14 -0800 Subject: [PATCH 3/5] Feedback and EDG test failures --- stl/inc/variant | 49 ++++++++-------------- tests/std/tests/P1614R2_spaceship/test.cpp | 2 +- 2 files changed, 19 insertions(+), 32 deletions(-) diff --git a/stl/inc/variant b/stl/inc/variant index 0e6035cf8c9..8a84d222d6d 100644 --- a/stl/inc/variant +++ b/stl/inc/variant @@ -20,11 +20,6 @@ #include #include -#ifdef _HAS_CXX20 -#include -#include -#endif // _HAS_CXX20 - #pragma pack(push, _CRT_PACKING) #pragma warning(push, _STL_WARNING_LEVEL) #pragma warning(disable : _STL_DISABLED_WARNINGS) @@ -1371,7 +1366,7 @@ _NODISCARD constexpr add_pointer_t get_if( // RELATIONAL OPERATORS [variant.relops] template -struct _Variant_relop_visitor { // evaluate _Op with the contained value of two variants that hold the same alternative +struct _Variant_relop_visitor2 { // evaluate _Op with the contained value of two variants that hold the same alternative const _Variant_storage<_Types...>& _Left; template @@ -1393,7 +1388,7 @@ template _NODISCARD constexpr bool operator==(const variant<_Types...>& _Left, const variant<_Types...>& _Right) noexcept( conjunction_v, const _Types&, const _Types&>...>) /* strengthened */ { // determine if the arguments are both valueless or contain equal values - using _Visitor = _Variant_relop_visitor, bool, _Types...>; + using _Visitor = _Variant_relop_visitor2, bool, _Types...>; const size_t _Right_index = _Right.index(); return _Left.index() == _Right_index && _Variant_raw_visit(_Right_index, _Right._Storage(), _Visitor{_Left._Storage()}); @@ -1403,7 +1398,7 @@ template _NODISCARD constexpr bool operator!=(const variant<_Types...>& _Left, const variant<_Types...>& _Right) noexcept( conjunction_v, const _Types&, const _Types&>...>) /* strengthened */ { // determine if the arguments have different active alternatives or contain unequal values - using _Visitor = _Variant_relop_visitor, bool, _Types...>; + using _Visitor = _Variant_relop_visitor2, bool, _Types...>; const size_t _Right_index = _Right.index(); return _Left.index() != _Right_index || _Variant_raw_visit(_Right_index, _Right._Storage(), _Visitor{_Left._Storage()}); @@ -1414,7 +1409,7 @@ _NODISCARD constexpr bool operator<(const variant<_Types...>& _Left, const varia conjunction_v, const _Types&, const _Types&>...>) /* strengthened */ { // determine if _Left has a lesser index(), or equal index() and lesser // contained value than _Right - using _Visitor = _Variant_relop_visitor, bool, _Types...>; + using _Visitor = _Variant_relop_visitor2, bool, _Types...>; const size_t _Left_offset = _Left.index() + 1; const size_t _Right_offset = _Right.index() + 1; return _Left_offset < _Right_offset @@ -1427,7 +1422,7 @@ _NODISCARD constexpr bool operator>(const variant<_Types...>& _Left, const varia conjunction_v, const _Types&, const _Types&>...>) /* strengthened */ { // determine if _Left has a greater index(), or equal index() and // greater contained value than _Right - using _Visitor = _Variant_relop_visitor, bool, _Types...>; + using _Visitor = _Variant_relop_visitor2, bool, _Types...>; const size_t _Left_offset = _Left.index() + 1; const size_t _Right_offset = _Right.index() + 1; return _Left_offset > _Right_offset @@ -1440,7 +1435,7 @@ _NODISCARD constexpr bool operator<=(const variant<_Types...>& _Left, const vari conjunction_v, const _Types&, const _Types&>...>) /* strengthened */ { // determine if _Left's index() is less than _Right's, or equal and // _Left contains a value less than or equal to _Right - using _Visitor = _Variant_relop_visitor, bool, _Types...>; + using _Visitor = _Variant_relop_visitor2, bool, _Types...>; const size_t _Left_offset = _Left.index() + 1; const size_t _Right_offset = _Right.index() + 1; return _Left_offset < _Right_offset @@ -1453,7 +1448,7 @@ _NODISCARD constexpr bool operator>=(const variant<_Types...>& _Left, const vari conjunction_v, const _Types&, const _Types&>...>) /* strengthened */ { // determine if _Left's index() is greater than _Right's, or equal and // _Left contains a value greater than or equal to _Right - using _Visitor = _Variant_relop_visitor, bool, _Types...>; + using _Visitor = _Variant_relop_visitor2, bool, _Types...>; const size_t _Left_offset = _Left.index() + 1; const size_t _Right_offset = _Right.index() + 1; return _Left_offset > _Right_offset @@ -1461,22 +1456,15 @@ _NODISCARD constexpr bool operator>=(const variant<_Types...>& _Left, const vari && _Variant_raw_visit(_Right_offset - 1, _Right._Storage(), _Visitor{_Left._Storage()})); } -#ifdef _HAS_CXX20 -struct _Variant_relop_3way_comparison { - template - compare_three_way_result_t<_Ty> operator()(const _Ty& _Left, const _Ty& _Right) { - return _Left <=> _Right; - } -}; - +#ifdef __cpp_lib_concepts template _NODISCARD constexpr common_comparison_category_t...> operator<=>(const variant<_Types...>& _Left, const variant<_Types...>& _Right) noexcept( conjunction_v...>, - _Variant_relop_3way_comparison, const _Types&, const _Types&>...>) /* strengthened */ { - // determine the three-way comparison of _Left's and _Right's index, if equal - // return the three-way comparison of the contained value of _Left and _Right. - using _Visitor = _Variant_relop_visitor<_Variant_relop_3way_comparison, + compare_three_way, const _Types&, const _Types&>...>) /* strengthened */ { + // Determine the three-way comparison of _Left's and _Right's index, if equal + // return the three-way comparison of the contained values of _Left and _Right + using _Visitor = _Variant_relop_visitor2...>, _Types...>; const size_t _Left_offset = _Left.index() + 1; const size_t _Right_offset = _Right.index() + 1; @@ -1484,7 +1472,7 @@ _NODISCARD constexpr common_comparison_category_t @@ -1730,7 +1718,11 @@ _NODISCARD constexpr bool operator==(monostate, monostate) noexcept { return true; } -#if !_HAS_CXX20 +#if _HAS_CXX20 +_NODISCARD constexpr strong_ordering operator<=>(monostate, monostate) noexcept { + return strong_ordering::equal; +} +#else // ^^^ _HAS_CXX20 / !_HAS_CXX20 vvv _NODISCARD constexpr bool operator!=(monostate, monostate) noexcept { return false; } @@ -1746,11 +1738,6 @@ _NODISCARD constexpr bool operator<=(monostate, monostate) noexcept { _NODISCARD constexpr bool operator>=(monostate, monostate) noexcept { return true; } -#else // ^^^ _HAS_CXX20 vvv - -_NODISCARD constexpr strong_ordering operator<=>(monostate, monostate) noexcept { - return strong_ordering::equal; -} #endif // !_HAS_CXX20 // SPECIALIZED ALGORITHMS [variant.specalg] diff --git a/tests/std/tests/P1614R2_spaceship/test.cpp b/tests/std/tests/P1614R2_spaceship/test.cpp index 1d10d05ca99..632ed54781d 100644 --- a/tests/std/tests/P1614R2_spaceship/test.cpp +++ b/tests/std/tests/P1614R2_spaceship/test.cpp @@ -499,7 +499,7 @@ void ordering_test_cases() { using M = std::monostate; constexpr M m1{}; constexpr M m2{}; - static_assert((m1 <=> m2) == 0, ""); + static_assert((m1 <=> m2) == 0); } { // slice std::slice a1(2, 3, 4); From ab06d6887f107732035e11f54ad20fa2b4de8b8e Mon Sep 17 00:00:00 2001 From: Daniel Winsor Date: Thu, 18 Feb 2021 14:01:16 -0800 Subject: [PATCH 4/5] Added requires clause --- stl/inc/variant | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/stl/inc/variant b/stl/inc/variant index 8a84d222d6d..7389fbee200 100644 --- a/stl/inc/variant +++ b/stl/inc/variant @@ -1457,12 +1457,15 @@ _NODISCARD constexpr bool operator>=(const variant<_Types...>& _Left, const vari } #ifdef __cpp_lib_concepts +// clang-format off template + requires (three_way_comparable<_Types> && ...) _NODISCARD constexpr common_comparison_category_t...> operator<=>(const variant<_Types...>& _Left, const variant<_Types...>& _Right) noexcept( conjunction_v...>, compare_three_way, const _Types&, const _Types&>...>) /* strengthened */ { - // Determine the three-way comparison of _Left's and _Right's index, if equal + // clang-format on + // determine the three-way comparison of _Left's and _Right's index, if equal // return the three-way comparison of the contained values of _Left and _Right using _Visitor = _Variant_relop_visitor2...>, _Types...>; From 68a8cff833304d2c7363d1a8ed5efde8044afd6d Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Thu, 18 Feb 2021 15:55:53 -0800 Subject: [PATCH 5/5] Code review feedback. --- tests/std/tests/P1614R2_spaceship/test.cpp | 24 +++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/tests/std/tests/P1614R2_spaceship/test.cpp b/tests/std/tests/P1614R2_spaceship/test.cpp index 56a8e7abf54..01b773ad200 100644 --- a/tests/std/tests/P1614R2_spaceship/test.cpp +++ b/tests/std/tests/P1614R2_spaceship/test.cpp @@ -134,7 +134,7 @@ struct dummy_diagnostic : std::error_category { }; template -void spaceship_test(const SmallType& smaller, const EqualType& smaller_equal, const LargeType& larger) { +constexpr bool spaceship_test(const SmallType& smaller, const EqualType& smaller_equal, const LargeType& larger) { assert(smaller == smaller_equal); assert(smaller_equal == smaller); assert(smaller != larger); @@ -152,6 +152,8 @@ void spaceship_test(const SmallType& smaller, const EqualType& smaller_equal, co assert((smaller <=> smaller_equal) == 0); static_assert(std::is_same_v larger), ReturnType>); + + return true; } template @@ -615,9 +617,29 @@ void ordering_test_cases() { spaceship_test(v0_1, v0_1, v1_0); spaceship_test(v1_0, v1_0, v1_1); + static_assert(spaceship_test(v0_0, v0_0, v0_1)); + static_assert(spaceship_test(v0_1, v0_1, v1_0)); + static_assert(spaceship_test(v1_0, v1_0, v1_1)); + + struct ThrowException { + operator int() { + throw "woof"; + } + }; + V valueless(std::in_place_index<1>, 1729L); + try { + valueless.emplace<0>(ThrowException{}); + } catch (...) { + // ignore exception + } + assert(valueless.valueless_by_exception()); + spaceship_test(valueless, valueless, v0_0); + spaceship_test(valueless, valueless, v1_1); + using M = std::monostate; constexpr M m1{}; constexpr M m2{}; + assert((m1 <=> m2) == 0); static_assert((m1 <=> m2) == 0); } { // slice