[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

149

Please update the synopsis.

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

46 ↗

(On Diff #349433)

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

76 ↗

(On Diff #349433)

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 ↗

(On Diff #349433)

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 ↗

(On Diff #349433)

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

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

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/CMakeLists.txt

set(files
   __algorithm/unwrap_iter.h
   __algorithm/upper_bound.h
   __availability
   __bit_reference
   __bits
   __bsd_locale_defaults.h
   __bsd_locale_fallbacks.h
   __compare/common_comparison_category.h
+  __compare/compare_three_way_result.h
   __compare/ordering.h
   __concepts/arithmetic.h
   __concepts/assignable.h
   __concepts/boolean_testable.h
   __concepts/class_or_enum.h
   __concepts/common_reference_with.h
   __concepts/common_with.h
   __concepts/constructible.h

libcxx/include/__compare/compare_three_way_result.h

+//===----------------------------------------------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef _LIBCPP___COMPARE_COMPARE_THREE_WAY_RESULT_H
+#define _LIBCPP___COMPARE_COMPARE_THREE_WAY_RESULT_H
+
+#include <__config>
+#include <type_traits>
+
+#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
+#pragma GCC system_header
+#endif
+
+_LIBCPP_BEGIN_NAMESPACE_STD
+
+#if _LIBCPP_STD_VER > 17
+
+template<class, class, class>
+struct _LIBCPP_HIDE_FROM_ABI __compare_three_way_result { };
+
+template<class _Tp, class _Up>
+struct _LIBCPP_HIDE_FROM_ABI __compare_three_way_result<_Tp, _Up, decltype(
+  declval<const remove_reference_t<_Tp>&>() <=> declval<const remove_reference_t<_Up>&>(), void()
+)> {
+    using type = decltype(declval<const remove_reference_t<_Tp>&>() <=> declval<const remove_reference_t<_Up>&>());
+};
+
+template<class _Tp, class _Up = _Tp>
+struct _LIBCPP_TEMPLATE_VIS compare_three_way_result : __compare_three_way_result<_Tp, _Up, void> { };
+
+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
+
+_LIBCPP_END_NAMESPACE_STD
+
+#endif // _LIBCPP___COMPARE_COMPARE_THREE_WAY_RESULT_H

libcxx/include/compare

namespace std {
   // [cmp.common], common comparison category type
   template<class... Ts>
   struct common_comparison_category {
     using type = see below;
   };
   template<class... Ts>
     using common_comparison_category_t = typename common_comparison_category<Ts...>::type;
 
+  // [cmp.result], result of three-way comparison
+  template<class T, class U = T> struct compare_three_way_result;
+
+  template<class T, class U = T>
+    using compare_three_way_result_t = typename compare_three_way_result<T, U>::type;
+
   // [cmp.alg], comparison algorithms
   template<class T> constexpr strong_ordering strong_order(const T& a, const T& b);
   template<class T> constexpr weak_ordering weak_order(const T& a, const T& b);
   template<class T> constexpr partial_ordering partial_order(const T& a, const T& b);
 
   // [cmp.partialord], Class partial_ordering
   class partial_ordering {
   public:
public:
     friend constexpr bool operator>=(unspecified, strong_ordering v) noexcept;
     friend constexpr strong_ordering operator<=>(strong_ordering v, unspecified) noexcept;
     friend constexpr strong_ordering operator<=>(unspecified, strong_ordering v) noexcept;
   };
 }
 */
 
 #include <__compare/common_comparison_category.h>
+#include <__compare/compare_three_way_result.h>
 #include <__compare/ordering.h>
 #include <__config>
 #include <type_traits>
 
 #ifndef _LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER
 #pragma GCC system_header
 #endif
 
 _LIBCPP_BEGIN_NAMESPACE_STD
 
 #if _LIBCPP_STD_VER > 17 && !defined(_LIBCPP_HAS_NO_SPACESHIP_OPERATOR)
 
 // [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);
 
 #endif // _LIBCPP_STD_VER > 17 && !defined(_LIBCPP_HAS_NO_SPACESHIP_OPERATOR)
 
 _LIBCPP_END_NAMESPACE_STD
 
 #endif // _LIBCPP_COMPARE

libcxx/include/module.modulemap

module codecvt {
     export *
   }
   module compare {
     header "compare"
     export *
 
     module __compare {
       module common_comparison_category { private header "__compare/common_comparison_category.h" }
+      module compare_three_way_result   { private header "__compare/compare_three_way_result.h"   }
       module ordering                   { private header "__compare/ordering.h"                   }
     }
   }
   module complex {
     header "complex"
     export *
   }
   module concepts {

libcxx/test/libcxx/diagnostics/detail.headers/compare/compare_three_way_result.module.verify.cpp

+// -*- C++ -*-
+//===----------------------------------------------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+// REQUIRES: modules-build
+
+// WARNING: This test was generated by 'generate_private_header_tests.py'
+// and should not be edited manually.
+
+// expected-error@*:* {{use of private header from outside its module: '__compare/compare_three_way_result.h'}}
+#include <__compare/compare_three_way_result.h>

libcxx/test/std/language.support/cmp/cmp.result/compare_three_way_result.compile.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++03, c++11, c++14, c++17
+// UNSUPPORTED: libcpp-no-concepts
+
+// <compare>
+
+// template<class T, class U = T> struct compare_three_way_result;
+// template<class T, class U = T>
+//   using compare_three_way_result_t = typename compare_three_way_result<T, U>::type;
+
+#include <compare>
+
+#include "test_macros.h"
+
+template<class T>
+concept has_no_nested_type = !requires { typename T::type; };
+
+ASSERT_SAME_TYPE(std::compare_three_way_result_t<int>, std::strong_ordering);
+ASSERT_SAME_TYPE(std::compare_three_way_result_t<float>, std::partial_ordering);
+
+ASSERT_SAME_TYPE(std::compare_three_way_result_t<int, int>, std::strong_ordering);
+ASSERT_SAME_TYPE(std::compare_three_way_result_t<int, float>, std::partial_ordering);
+ASSERT_SAME_TYPE(std::compare_three_way_result_t<float, float>, std::partial_ordering);
+
+ASSERT_SAME_TYPE(std::compare_three_way_result_t<const int&>, std::strong_ordering);
+ASSERT_SAME_TYPE(std::compare_three_way_result_t<const int&, int>, std::strong_ordering);
+ASSERT_SAME_TYPE(std::compare_three_way_result_t<const int*>, std::strong_ordering);
+ASSERT_SAME_TYPE(std::compare_three_way_result_t<const int*, int*>, std::strong_ordering);
+
+static_assert(has_no_nested_type<std::compare_three_way_result<void>>);
+static_assert(has_no_nested_type<std::compare_three_way_result<void, void>>);
+static_assert(has_no_nested_type<std::compare_three_way_result<int, void>>);
+static_assert(has_no_nested_type<std::compare_three_way_result<int, int*>>);
+
+struct A {
+    float operator<=>(const A&) const;  // a non-comparison-category type is OK
+};
+ASSERT_SAME_TYPE(std::compare_three_way_result_t<A, A>, float);
+
+struct B {
+    using T = int(&)();
+    T operator<=>(const B&) const;  // no decay takes place either
+};
+ASSERT_SAME_TYPE(std::compare_three_way_result_t<B, B>, int(&)());
+
+struct C {
+    std::strong_ordering operator<=>(C&);  // C isn't const-comparable
+};
+static_assert(has_no_nested_type<std::compare_three_way_result<C, C>>);
+static_assert(has_no_nested_type<std::compare_three_way_result<C&, C&>>);
+static_assert(has_no_nested_type<std::compare_three_way_result<C&&, C&&>>);
+
+struct D {
+    std::strong_ordering operator<=>(D&) &;
+    std::strong_ordering operator<=>(D&&) &&;
+    std::weak_ordering operator<=>(const D&) const&;  // comparison is always done by const&
+    std::strong_ordering operator<=>(const D&&) const&&;
+};
+ASSERT_SAME_TYPE(std::compare_three_way_result_t<D, D>, std::weak_ordering);
+ASSERT_SAME_TYPE(std::compare_three_way_result_t<D&, D&>, std::weak_ordering);
+ASSERT_SAME_TYPE(std::compare_three_way_result_t<D&&, D&&>, std::weak_ordering);
+ASSERT_SAME_TYPE(std::compare_three_way_result_t<const D&, const D&>, std::weak_ordering);
+ASSERT_SAME_TYPE(std::compare_three_way_result_t<const D&&, const D&&>, std::weak_ordering);