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

[Arthur O'Dwyer via Phabricator via libcxx-commits]

Quuxplusone added inline comments.


================
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); }
----------------
Mordante wrote:
> The Standard doesn't specify the `noexcept`, is this added intentionally?
Yes, for consistency with `std::less<void>` etc.
https://eel.is/c++draft/function.objects#comparisons doesn't put the conditional-noexcept on those either, but libc++ does. (That is, libc++ puts conditional-noexcept only on the transparent `less<void>`; not on `less<T>`.)


================
Comment at: libcxx/test/std/utilities/function.objects/comparisons/compare_three_way.pass.cpp:33
+};
+ASSERT_SAME_TYPE(std::compare_three_way_result_t<NotThreeWayComparable>, std::strong_ordering);
+static_assert(!std::three_way_comparable<NotThreeWayComparable>);  // it lacks operator==
----------------
rarutyun wrote:
> Why do we check `std::compare_three_way_result_t` in this test?
Just sanity-checking. `NotThreeWayComparable` is an example of a type where it //is// legal to ask for its `compare_three_way_result_t`, but in fact if you try to call `compare_three_way` on it, it doesn't compile. (But the actual relevant test is down on line 64; I should move them closer together, but I have no good idea how to.)


================
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
----------------
Mordante wrote:
> Nit: the empty string isn't required in C++20.
Yeah, but consistency with the earlier lines (including the wonky spacing).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110735