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

[Ruslan Arutyunyan via Phabricator via libcxx-commits]

rarutyun added inline comments.


================
Comment at: libcxx/test/std/utilities/function.objects/comparisons/compare_three_way.pass.cpp:9
+
+// UNSUPPORTED: c++03, c++11, c++14, c++17
+
----------------
Some steps in CI failed. I believe adding // UNSUPPORTED: libcpp-no-concepts should help


================
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==
----------------
Why do we check `std::compare_three_way_result_t` in this test?


================
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);
+
----------------
I would suggest to also test for `std::partial_ordering::unordered` and also add some tests for `weak_ordering`


================
Comment at: libcxx/test/std/utilities/function.objects/comparisons/compare_three_way.pass.cpp:49
+    // Try heterogeneous comparison.
+    ASSERT_SAME_TYPE(decltype(std::compare_three_way()(42.0, 42)), std::partial_ordering);
+    assert(std::compare_three_way()(42.0, 42) == std::partial_ordering::equivalent);
----------------
For heterogeneous case does it make sense to put `int` and `double` in reverse order?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110735