[libcxx-commits] [PATCH] D132265: [libc++][spaceship] Implement `operator<=>` for `array`

[Nikolas Klauser via Phabricator via libcxx-commits]

philnik added inline comments.


================
Comment at: libcxx/docs/Status/Cxx20.rst:65
+
+   .. [#note-LWG3347] LWG3347: Implemented changes in [array.syn] and [array.overview]
----------------
Would you be interested in implementing the rest of this issue? It doesn't look too complicated.


================
Comment at: libcxx/include/array:410-415
 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);
 }
----------------
Let's try to not reintroduce wrong formatting.


================
Comment at: libcxx/test/std/containers/sequences/array/compare.fail.cpp:1
 //===----------------------------------------------------------------------===//
 //
----------------
Could you make this a `.verify.cpp` while you're at it?


================
Comment at: libcxx/test/std/containers/sequences/array/compare.fail.cpp:29-30
 
-template <class Array>
-void test_compare(const Array& LHS, const Array& RHS) {
-  typedef std::vector<typename Array::value_type> Vector;
-  const Vector LHSV(LHS.begin(), LHS.end());
-  const Vector RHSV(RHS.begin(), RHS.end());
-  assert((LHS == RHS) == (LHSV == RHSV));
-  assert((LHS != RHS) == (LHSV != RHSV));
-  assert((LHS < RHS) == (LHSV < RHSV));
-  assert((LHS <= RHS) == (LHSV <= RHSV));
-  assert((LHS > RHS) == (LHSV > RHSV));
-  assert((LHS >= RHS) == (LHSV >= RHSV));
-}
-
-template <int Dummy> struct NoCompare {};
+template <int Dummy>
+struct NoCompare {};
 
----------------



================
Comment at: libcxx/test/std/containers/sequences/array/compare.pass.cpp:1
 //===----------------------------------------------------------------------===//
 //
----------------
Thanks for improving this test!


================
Comment at: libcxx/test/std/containers/sequences/array/compare.three_way.compile.fail.cpp:1
+//===----------------------------------------------------------------------===//
+//
----------------
Please use `.verify.cpp` instead.


================
Comment at: libcxx/test/std/containers/sequences/array/compare.three_way.pass.cpp:62
+
+  // The container should fulfil `std::three_way_comparable`
+  static_assert(std::three_way_comparable<std::array<int, N>>);
----------------



================
Comment at: libcxx/test/std/containers/sequences/array/compare.three_way.pass.cpp:71
+
+  // `LessAndEqComp` does not have `operator<=>`. ordering is sythesized based on `operator<`
+  test_sequence_container_array_spaceship_with_type<LessAndEqComp, std::weak_ordering>();
----------------



================
Comment at: libcxx/test/std/containers/sequences/array/compare.three_way.pass.cpp:74-76
+  // Thanks to SFINAE, the following is not a compiler error but returns `false`
+  struct NonComparable {};
+  static_assert(!std::three_way_comparable<std::array<NonComparable, N>>);
----------------
Let's move the SFINAE test to the top of the file.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132265/new/

https://reviews.llvm.org/D132265