[libc++] [P1614] Implement std::compare_three_way_result[_t].

[Mordante]

We recently started implementing libc++ is more granular headers. Can you adopt that style for these changes too?
You an use D103734 as an example.
I see a lot of build failure. I did a quick look and I expect none of them are due to this patch, but to main not working properly. Can you rebase your patch to see whether it passes the build?

libcxx/include/compare

485

Please update the synopsis.

libcxx/test/std/language.support/cmp/cmp.result/compare_three_way_result.pass.cpp

46

Please make these line fit in our maximum column width of 120 characters.

76

I'd like to see some more tests with fundamental and library types.

[• Quuxplusone]

libcxx/test/std/language.support/cmp/cmp.result/compare_three_way_result.pass.cpp

30

friend std::strong_ordering operator<=>(const OtherOperand&, const OtherOperand&) = delete;

but also consider whether it would break anything to make this simply

friend std::strong_ordering operator<=>(OtherOperand, OtherOperand) = delete;

because empty structs are typically passed by value. (Or even omit it entirely, although I assume that would break something you're trying to test here, is that right?)

46

Throughout the file, make these substitutions:
SameOperandsOrdering -> SameOrder
DifferentOperandsOrdering -> DifferentOrder
OperandType -> OT
do_compare_three_way_result_positive_test -> test
do_compare_three_way_result_negative_test -> negative_test
Remove redundant usages of type traits such as add_lvalue_reference.
Use the ASSERT_SAME_TYPE test helper.
So that makes this line

ASSERT_SAME_TYPE(std::compare_three_way_result_t<OT&, OT&>, SameOrder>);

which comfortably fits within our informally enforced column limit.
Please make these changes throughout.

[• Quuxplusone]

Commandeer and replace with the relevant piece of D107036.

[• Quuxplusone]

Mark the test file as requiring Concepts support. (The Mac OSX buildkite runs were failing.)

[rarutyun]

Should I do something else with this review?

[• Quuxplusone]

@rarutyun wrote:

Should I do something else with this review?

IMO no, I hope to take it from here.
Ping @Mordante @ldionne; buildkite is happy with this; are you also happy with it?

[cjdb]

Otherwise LGTM

libcxx/include/__compare/compare_three_way_result.h

28 ↗

(On Diff #365794)

We can probably use that __add_lvalue_reference_t<_Tp> alias here.

[• Quuxplusone]

Use __make_const_lvalue_ref<X> as suggested by @cjdb.

[• Quuxplusone]

libcxx/include/__compare/compare_three_way_result.h

28 ↗

(On Diff #365794)

Good point. Done.

[ldionne]

LGTM, but can you please add a test that std::is_same_v<compare_three_way_result<T>, compare_three_way_result<T, T>>?

[• Quuxplusone]

Add a couple new tests, and poke buildkite one more time before I land this.

libcxx/include/compare

 using common_comparison_category_t = typename common_comparison_category<_Ts...>::type;
 
 // [cmp.alg], comparison algorithms
 // TODO: unimplemented
 template<class _Tp> constexpr strong_ordering strong_order(const _Tp& __lhs, const _Tp& __rhs);
 template<class _Tp> constexpr weak_ordering weak_order(const _Tp& __lhs, const _Tp& __rhs);
 template<class _Tp> constexpr partial_ordering partial_order(const _Tp& __lhs, const _Tp& __rhs);
 
+// [cmp.result], result of three-way comparison
+template<typename _Tp, typename _Up>
+using __compare_three_way_result_type = decltype(declval<__make_const_lvalue_ref<_Tp>>() <=>
+                                                 declval<__make_const_lvalue_ref<_Up>>());
+
+template<class _Tp, class _Up = _Tp>
+struct compare_three_way_result {};
+
+template<class _Tp, class _Up>
+  requires requires { typename __compare_three_way_result_type<_Tp, _Up>; }
+struct compare_three_way_result<_Tp, _Up> {
+  using type = __compare_three_way_result_type<_Tp, _Up>;
+};
+
+template<class _Tp, class _Up = _Tp>
+using compare_three_way_result_t = typename compare_three_way_result<_Tp, _Up>::type;
+
 #endif // _LIBCPP_STD_VER > 17 && !defined(_LIBCPP_HAS_NO_SPACESHIP_OPERATOR)
 
 _LIBCPP_END_NAMESPACE_STD
 
 #endif // _LIBCPP_COMPARE

libcxx/test/std/language.support/cmp/cmp.result/compare_three_way_result.pass.cpp

+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// UNSUPPORTED: c++98, c++03, c++11, c++14, c++17
+// UNSUPPORTED: libcpp-no-concepts
+
+// <compare>
+
+// compare_three_way_result, compare_three_way_result_t
+
+#include <compare>
+#include <type_traits>
+
+template <class T, class U>
+concept compare_three_way_result_type_defined = requires {
+    typename std::compare_three_way_result<T, U>::type;
+};
+
+template <class T, class U>
+concept compare_three_way_result_t_defined = requires {
+    typename std::compare_three_way_result_t<T, U>;
+};
+
+struct OtherOperand {
+    friend std::strong_ordering operator<=>( const OtherOperand&, const OtherOperand& ) = delete;
+};
+
+template <class SameOperandsOrdering, class DifferentOperandsOrdering>
+struct Operand {
+    SameOperandsOrdering operator<=>( const Operand& ) const = default;
+    friend DifferentOperandsOrdering operator<=>( const Operand&, const OtherOperand& ) {
+        return DifferentOperandsOrdering::equivalent;
+    }
+};
+
+template <class SameOperandsOrdering, class DifferentOperandsOrdering>
+constexpr void do_compare_three_way_result_positive_test() {
+    using OperandType = Operand<SameOperandsOrdering, DifferentOperandsOrdering>;
+    static_assert(std::is_same_v<std::compare_three_way_result_t<OperandType>, SameOperandsOrdering>);
+    static_assert(std::is_same_v<std::compare_three_way_result_t<OperandType, OperandType>, SameOperandsOrdering>);
+    static_assert(std::is_same_v<std::compare_three_way_result_t<std::add_lvalue_reference_t<OperandType>, std::add_lvalue_reference_t<OperandType>>,
+                                SameOperandsOrdering>);
+    static_assert(std::is_same_v<std::compare_three_way_result_t<OperandType, OtherOperand>, DifferentOperandsOrdering>);
+    static_assert(std::is_same_v<std::compare_three_way_result_t<OtherOperand, OperandType>, DifferentOperandsOrdering>);
+    static_assert(std::is_same_v<std::compare_three_way_result_t<std::add_lvalue_reference_t<OtherOperand>, std::add_lvalue_reference_t<OperandType>>,
+                                DifferentOperandsOrdering>);
+
+    static_assert(std::is_same_v<std::compare_three_way_result_t<OperandType>,
+                                typename std::compare_three_way_result<OperandType>::type>);
+    static_assert(std::is_same_v<std::compare_three_way_result_t<OperandType, OperandType>,
+                                typename std::compare_three_way_result<OperandType, OperandType>::type>);
+    static_assert(std::is_same_v<std::compare_three_way_result_t<OperandType, OtherOperand>,
+                                typename std::compare_three_way_result<OperandType, OtherOperand>::type>);
+    static_assert(std::is_same_v<std::compare_three_way_result_t<OtherOperand, OperandType>,
+                                typename std::compare_three_way_result<OtherOperand, OperandType>::type>);
+    static_assert(std::is_same_v<std::compare_three_way_result_t<std::add_lvalue_reference_t<OperandType>,
+                                                                std::add_lvalue_reference_t<OperandType>>,
+                                typename std::compare_three_way_result<OperandType, OperandType>::type>);
+}
+
+constexpr void do_compare_three_way_result_negative_test() {
+    static_assert(!compare_three_way_result_type_defined<OtherOperand, OtherOperand>);
+    static_assert(!compare_three_way_result_type_defined<OtherOperand, int>);
+    static_assert(!compare_three_way_result_type_defined<int, OtherOperand>);
+
+    static_assert(!compare_three_way_result_t_defined<OtherOperand, OtherOperand>);
+    static_assert(!compare_three_way_result_t_defined<OtherOperand, int>);
+    static_assert(!compare_three_way_result_t_defined<int, OtherOperand>);
+}
+
+int main(int, char**) {
+    do_compare_three_way_result_positive_test<std::strong_ordering, std::weak_ordering>();
+    do_compare_three_way_result_positive_test<std::weak_ordering, std::partial_ordering>();
+    do_compare_three_way_result_positive_test<std::partial_ordering, std::strong_ordering>();
+    do_compare_three_way_result_negative_test();
+}