[libc++][spaceship] Implement `operator<=>` for `array`

[H-G-Hristov]

Summary

Implements part of P1614R2 "The Mothership has Landed"

[avogelsgesang]

update review link

[avogelsgesang]

add AssertComparisonsReturnBool/AssertOrderReturn to test cases

[avogelsgesang]

Add whitespace in templates

[Mordante]

I have some issues regarding the difference of the paper and the Standard, can you look for an LWG-issue. I couldn't find one quickly. I glossed over the rest of the patch but didn't do a real review since it's uncertain what should be implemented.

libcxx/include/array

91

Did you look for any LWG issues in this header?

P1614 states

[ Editor's note: array’s comparisons move to be hidden friends to allow for use as non-type template parameters. All the other containers drop != and, if they have relational operators, those get replaced with a <=>. ]

Change 22.3.2 [array.syn]:
The operator== has been removed from the header and operator<=> isn't added/

Change 22.3.7.1 [array.overview]:

+   friend constexpr bool operator==(const array&, const array&) = default;
+   friend constexpr synth-three-way-result<value_type>
+     operator<=>(const array&, const array&);

But this doesn't match the WP. No friends and no defaulted operator==.

443

447

libcxx/test/std/containers/sequences/array/compare.pass.cpp

123

nice!

[avogelsgesang]

libcxx/include/array

91

https://cplusplus.github.io/LWG/issue3347 removed the friends again and reintroduced the free-standing comparisons again

[avogelsgesang]

needs to be adjusted to use the still-to-be-introduced test utilities from https://reviews.llvm.org/D132312 is done

[H-G-Hristov]

• [libc++][spaceship] Implement operator<=> for array

[H-G-Hristov]

• Try to fix GCC -Werror=maybe-uninitialized

[H-G-Hristov]

• Rebased to rerun CI

[H-G-Hristov]

Rebased

[Mordante]

The CI failure is not related to your changes.

Is this patch ready for review or do you have more changes planned?

libcxx/include/array

443

This change is not applied.

[H-G-Hristov]

• Addressed review comments

[H-G-Hristov]

In D132265#4310036, @Mordante wrote:

The CI failure is not related to your changes.

Is this patch ready for review or do you have more changes planned?

Yes, this patch and also the patches for vector and optionalare ready for a review.

[philnik]

libcxx/docs/Status/Cxx20.rst

65 ↗

(On Diff #519227)

Would you be interested in implementing the rest of this issue? It doesn't look too complicated.

libcxx/include/array

404–409

Let's try to not reintroduce wrong formatting.

libcxx/test/std/containers/sequences/array/compare.fail.cpp

1 ↗

(On Diff #519227)

Could you make this a .verify.cpp while you're at it?

29–30 ↗

(On Diff #519227)

libcxx/test/std/containers/sequences/array/compare.pass.cpp

1

Thanks for improving this test!

libcxx/test/std/containers/sequences/array/compare.three_way.compile.fail.cpp

1 ↗

(On Diff #519227)

Please use .verify.cpp instead.

libcxx/test/std/containers/sequences/array/compare.three_way.pass.cpp

62 ↗

(On Diff #519227)

71 ↗

(On Diff #519227)

74–76 ↗

(On Diff #519227)

Let's move the SFINAE test to the top of the file.

[H-G-Hristov]

• Addressed review comments

[H-G-Hristov]

@philnik Thank you for the review!

NOTE: Changes to compare.pass.cpp were too large so git failed to match it to compare.verify.cpp.

libcxx/docs/Status/Cxx20.rst

65 ↗

(On Diff #519227)

Yes, let me do it in a follow up patch.

libcxx/include/array

404–409

I replaced _LIBCPP_INLINE_VISIBILITY and applied format to the whole section.

libcxx/test/std/containers/sequences/array/compare.three_way.pass.cpp

74–76 ↗

(On Diff #519227)

Moved it. My original motivation was to keep the same stricture of the test as tests in:
libcxx/test/support/test_container_comparisons.h
where they use templates.

[H-G-Hristov]

• Marked LWG3347 as complete

It was even marked as Nothing to Do: https://libcxx.llvm.org/Status/Spaceship.html

[H-G-Hristov]

libcxx/docs/Status/Cxx20.rst

65 ↗

(On Diff #519227)

Let me correct myself, the std::pair's part is already implemented. Marked the issue as completed.

[Mordante]

All my comments have been addressed. I leave the final approval to @philnik.

libcxx/docs/Status/SpaceshipProjects.csv

 | `[util.smartptr.shared.cmp] <https://wg21.link/util.smartptr.shared.cmp>`_,| `shared_ptr <https://reviews.llvm.org/D130852>`_,[comparisons.three.way],Adrian Vogelsgesang,|Complete|
 | `[type.index.members] <https://wg21.link/type.index.members>`_,| `type_index <https://reviews.llvm.org/D131357>`_,None,Adrian Vogelsgesang,|Complete|
 | `[charconv.syn] <https://wg21.link/charconv.syn>`_,| `to_chars_result <https://reviews.llvm.org/D112366>`_,None,Mark de Wever,|Complete|
 | `[charconv.syn] <https://wg21.link/charconv.syn>`_,| `from_chars_result <https://reviews.llvm.org/D112366>`_,None,Mark de Wever,|Complete|
 | `[stacktrace.entry.cmp] <https://wg21.link/stacktrace.entry.cmp>`_,| stacktrace_entry,None,Unassigned,|Not Started|
 | `[stacktrace.basic.cmp] <https://wg21.link/stacktrace.basic.cmp>`_,| basic_stacktrace,[alg.three.way],Unassigned,|Not Started|
 | `[string.cmp] <https://wg21.link/string.cmp>`_,| `basic_string <https://reviews.llvm.org/D131421>`_,None,Mark de Wever,|Complete|
 | `[string.view.comparison] <https://wg21.link/string.view.comparison>`_,| `basic_string_view <https://reviews.llvm.org/D130295>`_,None,Mark de Wever,|Complete|
-| `[array.syn] <https://wg21.link/array.syn>`_ (`general <https://wg21.link/container.opt.reqmts>`_),| array,[expos.only.func],Unassigned,|Not Started|
+| `[array.syn] <https://wg21.link/array.syn>`_ (`general <https://wg21.link/container.opt.reqmts>`_),| `array <https://reviews.llvm.org/D132265>`_,[expos.only.func],Adrian Vogelsgesang,|Complete|
 | `[deque.syn] <https://wg21.link/deque.syn>`_ (`general <https://wg21.link/container.opt.reqmts>`_),| deque,[expos.only.func],Unassigned,|Not Started|
 | `[forward.list.syn] <https://wg21.link/forward.list.syn>`_ (`general <https://wg21.link/container.opt.reqmts>`_),| forward_list,[expos.only.func],Unassigned,|Not Started|
 | `[list.syn] <https://wg21.link/list.syn>`_ (`general <https://wg21.link/container.opt.reqmts>`_),| list,[expos.only.func],Unassigned,|Not Started|
 | `[vector.syn] <https://wg21.link/vector.syn>`_ (`general <https://wg21.link/container.opt.reqmts>`_),| vector,[expos.only.func],Unassigned,|Not Started|
 | `[associative.map.syn] <https://wg21.link/associative.map.syn>`_ (`general <https://wg21.link/container.opt.reqmts>`_),"| map
 | multimap",[expos.only.func],Unassigned,|Not Started|
 | `[associative.set.syn] <https://wg21.link/associative.set.syn>`_ (`general <https://wg21.link/container.opt.reqmts>`_),"| multiset
 | set",[expos.only.func],Unassigned,|Not Started|

libcxx/include/array

 };
 
 template <class T, class... U>
   array(T, U...) -> array<T, 1 + sizeof...(U)>;                 // C++17
 
 template <class T, size_t N>
   bool operator==(const array<T,N>& x, const array<T,N>& y);    // constexpr in C++20
 template <class T, size_t N>
-  bool operator!=(const array<T,N>& x, const array<T,N>& y);    // constexpr in C++20
+  bool operator!=(const array<T,N>& x, const array<T,N>& y);    // removed in C++20
 template <class T, size_t N>
-  bool operator<(const array<T,N>& x, const array<T,N>& y);     // constexpr in C++20
+  bool operator<(const array<T,N>& x, const array<T,N>& y);     // removed in C++20
 template <class T, size_t N>
-  bool operator>(const array<T,N>& x, const array<T,N>& y);     // constexpr in C++20
+  bool operator>(const array<T,N>& x, const array<T,N>& y);     // removed in C++20
 template <class T, size_t N>
-  bool operator<=(const array<T,N>& x, const array<T,N>& y);    // constexpr in C++20
+  bool operator<=(const array<T,N>& x, const array<T,N>& y);    // removed in C++20
 template <class T, size_t N>
-  bool operator>=(const array<T,N>& x, const array<T,N>& y);    // constexpr in C++20
+  bool operator>=(const array<T,N>& x, const array<T,N>& y);    // removed in C++20
+template<class T, size_t N>
+  constexpr synth-three-way-result<T>
+    operator<=>(const array<T, N>& x, const array<T, N>& y);    // since C++23
 
 template <class T, size_t N >
   void swap(array<T,N>& x, array<T,N>& y) noexcept(noexcept(x.swap(y))); // constexpr in C++20
 
 template <class T, size_t N>
   constexpr array<remove_cv_t<T>, N> to_array(T (&a)[N]);  // C++20
 template <class T, size_t N>
   constexpr array<remove_cv_t<T>, N> to_array(T (&&a)[N]); // C++20

 
 }  // std
 
 */
 
 #include <__algorithm/equal.h>
 #include <__algorithm/fill_n.h>
 #include <__algorithm/lexicographical_compare.h>
+#include <__algorithm/lexicographical_compare_three_way.h>
 #include <__algorithm/swap_ranges.h>
 #include <__assert> // all public C++ headers provide the assertion handler
 #include <__config>
 #include <__iterator/reverse_iterator.h>
 #include <__tuple>
 #include <__utility/integer_sequence.h>
 #include <__utility/move.h>
 #include <__utility/unreachable.h>

 template <class _Tp, size_t _Size>
 inline _LIBCPP_INLINE_VISIBILITY
 _LIBCPP_CONSTEXPR_SINCE_CXX20 bool
 operator==(const array<_Tp, _Size>& __x, const array<_Tp, _Size>& __y)
 {
     return _VSTD::equal(__x.begin(), __x.end(), __y.begin());
 }
 
+#if _LIBCPP_STD_VER <= 17
+
 template <class _Tp, size_t _Size>
 inline _LIBCPP_INLINE_VISIBILITY
-_LIBCPP_CONSTEXPR_SINCE_CXX20 bool
-operator!=(const array<_Tp, _Size>& __x, const array<_Tp, _Size>& __y)
+bool operator!=(const array<_Tp, _Size>& __x, const array<_Tp, _Size>& __y)
 {
     return !(__x == __y);
 }
 
 template <class _Tp, size_t _Size>
 inline _LIBCPP_INLINE_VISIBILITY
-_LIBCPP_CONSTEXPR_SINCE_CXX20 bool
-operator<(const array<_Tp, _Size>& __x, const array<_Tp, _Size>& __y)
+bool operator<(const array<_Tp, _Size>& __x, const array<_Tp, _Size>& __y)
 {
     return _VSTD::lexicographical_compare(__x.begin(), __x.end(),
                                           __y.begin(), __y.end());
 }
 
 template <class _Tp, size_t _Size>
 inline _LIBCPP_INLINE_VISIBILITY
-_LIBCPP_CONSTEXPR_SINCE_CXX20 bool
-operator>(const array<_Tp, _Size>& __x, const array<_Tp, _Size>& __y)
+bool operator>(const array<_Tp, _Size>& __x, const array<_Tp, _Size>& __y)
 {
     return __y < __x;
 }
 
 template <class _Tp, size_t _Size>
 inline _LIBCPP_INLINE_VISIBILITY
-_LIBCPP_CONSTEXPR_SINCE_CXX20 bool
-operator<=(const array<_Tp, _Size>& __x, const array<_Tp, _Size>& __y)
+bool operator<=(const array<_Tp, _Size>& __x, const array<_Tp, _Size>& __y)
 {
     return !(__y < __x);
 }
 
 template <class _Tp, size_t _Size>
 inline _LIBCPP_INLINE_VISIBILITY
-_LIBCPP_CONSTEXPR_SINCE_CXX20 bool
-operator>=(const array<_Tp, _Size>& __x, const array<_Tp, _Size>& __y)
+bool operator>=(const array<_Tp, _Size>& __x, const array<_Tp, _Size>& __y)
 {
     return !(__x < __y);
 }
 
+#else // _LIBCPP_STD_VER > 17
+
+template <class _Tp, size_t _Size>
+inline _LIBCPP_INLINE_VISIBILITY
+constexpr __synth_three_way_result<_Tp>
+operator<=>(const array<_Tp, _Size>& __x, const array<_Tp, _Size>& __y)
+{
+    return _VSTD::lexicographical_compare_three_way(
+        __x.begin(), __x.end(), __y.begin(), __y.end(), __synth_three_way);
+}
+
+#endif // _LIBCPP_STD_VER > 17
+
 template <class _Tp, size_t _Size>
 inline _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_SINCE_CXX20
 __enable_if_t<_Size == 0 || __is_swappable<_Tp>::value, void>
 swap(array<_Tp, _Size>& __x, array<_Tp, _Size>& __y)
                                   _NOEXCEPT_(noexcept(__x.swap(__y)))
 {
     __x.swap(__y);
 }

libcxx/test/std/containers/sequences/array/compare.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
 //
 //===----------------------------------------------------------------------===//
 
 // <array>
 
-// bool operator==(array<T, N> const&, array<T, N> const&);   // constexpr in C++20
-// bool operator!=(array<T, N> const&, array<T, N> const&);   // constexpr in C++20
-// bool operator<(array<T, N> const&, array<T, N> const&);    // constexpr in C++20
-// bool operator<=(array<T, N> const&, array<T, N> const&);   // constexpr in C++20
-// bool operator>(array<T, N> const&, array<T, N> const&);    // constexpr in C++20
-// bool operator>=(array<T, N> const&, array<T, N> const&);   // constexpr in C++20
-
+// bool operator==(array<T, N> const&, array<T, N> const&);   // removed in C++20
+// bool operator!=(array<T, N> const&, array<T, N> const&);   // removed in C++20
+// bool operator<(array<T, N> const&, array<T, N> const&);    // removed in C++20
+// bool operator<=(array<T, N> const&, array<T, N> const&);   // removed in C++20
+// bool operator>(array<T, N> const&, array<T, N> const&);    // removed in C++20
+// bool operator>=(array<T, N> const&, array<T, N> const&);   // removed in C++20
+// template<class T, size_t N>
+//   constexpr synth-three-way-result<T>
+//     operator<=>(const array<T, N>& x, const array<T, N>& y);  // since C++23
 
 #include <array>
 #include <cassert>
 
 #include "test_macros.h"
 #include "test_comparisons.h"
 
 TEST_CONSTEXPR_CXX20 bool tests()
 {
-    {
-        typedef std::array<int, 3> C;
-        const C c1 = {1, 2, 3};
-        const C c2 = {1, 2, 3};
-        const C c3 = {3, 2, 1};
-        const C c4 = {1, 2, 1};
-        assert(testComparisons(c1, c2, true, false));
-        assert(testComparisons(c1, c3, false, true));
-        assert(testComparisons(c1, c4, false, false));
-    }
-    {
-        typedef std::array<int, 0> C;
-        const C c1 = {};
-        const C c2 = {};
-        assert(testComparisons(c1, c2, true, false));
-    }
-    {
-        typedef std::array<LessAndEqComp, 3> C;
-        const C c1 = {LessAndEqComp(1), LessAndEqComp(2), LessAndEqComp(3)};
-        const C c2 = {LessAndEqComp(1), LessAndEqComp(2), LessAndEqComp(3)};
-        const C c3 = {LessAndEqComp(3), LessAndEqComp(2), LessAndEqComp(1)};
-        const C c4 = {LessAndEqComp(1), LessAndEqComp(2), LessAndEqComp(1)};
-        assert(testComparisons(c1, c2, true, false));
-        assert(testComparisons(c1, c3, false, true));
-        assert(testComparisons(c1, c4, false, false));
-    }
-    {
-        typedef std::array<LessAndEqComp, 0> C;
-        const C c1 = {};
-        const C c2 = {};
-        assert(testComparisons(c1, c2, true, false));
-    }
+  AssertComparisonsReturnBool<std::array<int, 3>>();
+#if TEST_STD_VER >= 20
+  AssertOrderReturn<std::strong_ordering, std::array<int, 3>>();
+#endif
+  // Arrays where the elements support all comparison operators
+#if TEST_STD_VER >= 20
+    assert(testOrder(c1, c2, std::strong_ordering::equal));
+    assert(testOrder(c1, c3, std::strong_ordering::less));
+    assert(testOrder(c1, c4, std::strong_ordering::greater));
+#endif
+  // Empty array
+#if TEST_STD_VER >= 20
+    assert(testOrder(c1, c2, std::strong_ordering::equal));
+#endif
+  // Arrays where the elements support only less and equality comparisons
+  AssertComparisonsReturnBool<std::array<LessAndEqComp, 3>>();
+#if TEST_STD_VER >= 20
+  AssertOrderReturn<std::weak_ordering, std::array<LessAndEqComp, 3>>();
+#endif
+#if TEST_STD_VER >= 20
+    assert(testOrder(c1, c2, std::strong_ordering::equal));
+    assert(testOrder(c1, c3, std::strong_ordering::less));
+    assert(testOrder(c1, c4, std::strong_ordering::greater));
+#endif
+  // Empty array where the elements support only less and equality comparisons
+#if TEST_STD_VER >= 20
+    assert(testOrder(c1, c2, std::strong_ordering::equal));
+#endif
+#if TEST_STD_VER >= 20
+  // Arrays with weakly comparable elements
+  AssertOrderReturn<std::weak_ordering, std::array<WeakComp, 3>>();
+  {
+    typedef std::array<WeakComp, 3> C;
+    const C c1 = {1, 2, 3};
+    const C c2 = {1, 2, 3};
+    const C c3 = {3, 2, 1};
+    const C c4 = {1, 2, 1};
+    assert(testOrder(c1, c2, std::weak_ordering::equivalent));
+    assert(testOrder(c1, c3, std::weak_ordering::less));
+    assert(testOrder(c1, c4, std::weak_ordering::greater));
+  }
+  // Arrays with partially comparable elements
+  AssertOrderReturn<std::partial_ordering, std::array<PartialComp, 3>>();
+  {
+    typedef std::array<PartialComp, 3> C;
+    const C c1 = {1, 2, 3};
+    const C c2 = {1, 2, 3};
+    const C c3 = {3, 2, 1};
+    const C c4 = {1, 2, 1};
+    assert(testOrder(c1, c2, std::weak_ordering::equivalent));
+    assert(testOrder(c1, c3, std::weak_ordering::less));
+    assert(testOrder(c1, c4, std::weak_ordering::greater));
+  }
+#endif
 
-    return true;
 }
 
 int main(int, char**)
 {
     tests();
 #if TEST_STD_VER >= 20
-    static_assert(tests(), "");
+    static_assert(tests());
 #endif
     return 0;
 }