[libcxx-commits] [PATCH] D110735: [libc++] [P1614] Implement std::compare_three_way.

[Mark de Wever via Phabricator via libcxx-commits]

Mordante added inline comments.


================
Comment at: libcxx/include/__compare/compare_three_way.h:29
+        requires three_way_comparable_with<_T1, _T2>
+    constexpr _LIBCPP_INLINE_VISIBILITY
+    auto operator()(_T1&& __t, _T2&& __u) const
----------------
Nit: s/`_LIBCPP_INLINE_VISIBILITY`/`_LIBCPP_HIDE_FROM_ABI`/


================
Comment at: libcxx/include/__compare/compare_three_way.h:31
+    auto operator()(_T1&& __t, _T2&& __u) const
+        noexcept(noexcept(_VSTD::forward<_T1>(__t) <=> _VSTD::forward<_T2>(__u)))
+        { return          _VSTD::forward<_T1>(__t) <=> _VSTD::forward<_T2>(__u); }
----------------
The Standard doesn't specify the `noexcept`, is this added intentionally?


================
Comment at: libcxx/test/std/utilities/function.objects/comparisons/compare_three_way.pass.cpp:46
+    assert(std::compare_three_way()(1.0, 2.0) == std::partial_ordering::less);
+    assert(std::compare_three_way()(2.0, 1.0) == std::partial_ordering::greater);
+
----------------
rarutyun wrote:
> I would suggest to also test for `std::partial_ordering::unordered` and also add some tests for `weak_ordering`
+1


================
Comment at: libcxx/test/std/utilities/function.objects/comparisons/transparent.pass.cpp:60
+#if TEST_STD_VER > 17
+    static_assert(   is_transparent<std::compare_three_way>::value, "" );
+#endif
----------------
Nit: the empty string isn't required in C++20.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110735