std::strong_order() etc. depend on this. Let's get this shipped!
Details
- Reviewers
ldionne Mordante rarutyun - Group Reviewers
Restricted Project - Commits
- rG3df094d31eac: [libc++] [P1614] Implement std::compare_three_way.
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
libcxx/test/std/utilities/function.objects/comparisons/compare_three_way.pass.cpp | ||
---|---|---|
10 | Some steps in CI failed. I believe adding // UNSUPPORTED: libcpp-no-concepts should help | |
34 | Why do we check std::compare_three_way_result_t in this test? | |
47 | I would suggest to also test for std::partial_ordering::unordered and also add some tests for weak_ordering | |
50 | For heterogeneous case does it make sense to put int and double in reverse order? |
Should we add test to check availability from <functional> header or it's already covered by this one libcxx/test/std/utilities/function.objects/comparisons/transparent.pass.cpp?
libcxx/include/__compare/compare_three_way.h | ||
---|---|---|
30 | Nit: s/_LIBCPP_INLINE_VISIBILITY/_LIBCPP_HIDE_FROM_ABI/ | |
32 | The Standard doesn't specify the noexcept, is this added intentionally? | |
libcxx/test/std/utilities/function.objects/comparisons/compare_three_way.pass.cpp | ||
47 | +1 | |
libcxx/test/std/utilities/function.objects/comparisons/transparent.pass.cpp | ||
60 ↗ | (On Diff #375952) | Nit: the empty string isn't required in C++20. |
Address review comments; add test for <functional>.
libcxx/include/__compare/compare_three_way.h | ||
---|---|---|
32 | Yes, for consistency with std::less<void> etc. | |
libcxx/test/std/utilities/function.objects/comparisons/compare_three_way.pass.cpp | ||
34 | 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.) | |
libcxx/test/std/utilities/function.objects/comparisons/transparent.pass.cpp | ||
60 ↗ | (On Diff #375952) | Yeah, but consistency with the earlier lines (including the wonky spacing). |
libcxx/include/functional | ||
---|---|---|
139 |
libcxx/test/std/utilities/function.objects/comparisons/transparent.pass.cpp | ||
---|---|---|
61 ↗ | (On Diff #378337) | Bother. @ldionne, is there anything clever we could do here, to avoid splitting this one line out into a separate test marked UNSUPPORTED: libcpp-no-concepts? I'll split it out if I need to, but I'm hoping that there's a clever approach I'm forgetting about. |
Split out the is_transparent test into its own file; mark it UNSUPPORTED: libcpp-no-concepts.
I'm expecting this to pass buildkite, and then land it tonight or tomorrow.
Nit: s/_LIBCPP_INLINE_VISIBILITY/_LIBCPP_HIDE_FROM_ABI/