Implements part of P1614R2 "The Mothership has Landed"
Details
- Reviewers
Mordante mumbleskates huixie90 avogelsgesang philnik - Group Reviewers
Restricted Project - Commits
- rG254986d2df8d: [libc++][spaceship] Implement `operator<=>` for `array`
Diff Detail
- Repository
- rG LLVM Github Monorepo
Unit Tests
Event Timeline
I have some issues regarding the difference of the paper and the Standard, can you look for an LWG-issue. I couldn't find one quickly. I glossed over the rest of the patch but didn't do a real review since it's uncertain what should be implemented.
libcxx/include/array | ||
---|---|---|
91 | Did you look for any LWG issues in this header? P1614 states [ Editor's note: array’s comparisons move to be hidden friends to allow for use as non-type template parameters. All the other containers drop != and, if they have relational operators, those get replaced with a <=>. ] Change 22.3.2 [array.syn]: Change 22.3.7.1 [array.overview]: + friend constexpr bool operator==(const array&, const array&) = default; + friend constexpr synth-three-way-result<value_type> + operator<=>(const array&, const array&); But this doesn't match the WP. No friends and no defaulted operator==. | |
449 | ||
453 | ||
libcxx/test/std/containers/sequences/array/compare.pass.cpp | ||
76 | nice! |
libcxx/include/array | ||
---|---|---|
91 | https://cplusplus.github.io/LWG/issue3347 removed the friends again and reintroduced the free-standing comparisons again |
needs to be adjusted to use the still-to-be-introduced test utilities from https://reviews.llvm.org/D132312 is done
The CI failure is not related to your changes.
Is this patch ready for review or do you have more changes planned?
libcxx/include/array | ||
---|---|---|
449 | This change is not applied. |
libcxx/docs/Status/Cxx20.rst | ||
---|---|---|
65 | Would you be interested in implementing the rest of this issue? It doesn't look too complicated. | |
libcxx/include/array | ||
410–415 | Let's try to not reintroduce wrong formatting. | |
libcxx/test/std/containers/sequences/array/compare.fail.cpp | ||
1 | Could you make this a .verify.cpp while you're at it? | |
29–31 | ||
libcxx/test/std/containers/sequences/array/compare.pass.cpp | ||
1 | Thanks for improving this test! | |
libcxx/test/std/containers/sequences/array/compare.three_way.compile.fail.cpp | ||
2 | Please use .verify.cpp instead. | |
libcxx/test/std/containers/sequences/array/compare.three_way.pass.cpp | ||
63 | ||
72 | ||
75–77 | Let's move the SFINAE test to the top of the file. |
@philnik Thank you for the review!
libcxx/docs/Status/Cxx20.rst | ||
---|---|---|
65 | Yes, let me do it in a follow up patch. | |
libcxx/include/array | ||
410–415 | I replaced _LIBCPP_INLINE_VISIBILITY and applied format to the whole section. | |
libcxx/test/std/containers/sequences/array/compare.three_way.pass.cpp | ||
75–77 | Moved it. My original motivation was to keep the same stricture of the test as tests in: |
- Marked LWG3347 as complete
It was even marked as Nothing to Do: https://libcxx.llvm.org/Status/Spaceship.html
libcxx/docs/Status/Cxx20.rst | ||
---|---|---|
65 | Let me correct myself, the std::pair's part is already implemented. Marked the issue as completed. |
Would you be interested in implementing the rest of this issue? It doesn't look too complicated.