[libcxx-commits] [PATCH] D144821: [libc++][spaceship] Implement `operator<=>` for `deque`

Nikolas Klauser via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Wed Mar 1 07:46:13 PST 2023


philnik accepted this revision.
philnik added a comment.
This revision is now accepted and ready to land.

In D144821#4161020 <https://reviews.llvm.org/D144821#4161020>, @H-G-Hristov wrote:

> In D144821#4156977 <https://reviews.llvm.org/D144821#4156977>, @avogelsgesang wrote:
>
>> Thanks for this change!
>>
>> In general, I won't be able to approve your commit for the `libc++` group. In the end, you will need the review from one of the "core libc++ contributors" (e.g., @Mordante) to ship this fix.
>>
>> Also, for "stacked commits", i.e. commits which depend on other changes which didn't land on `main`, yet, you need to add a parent revision here on Phabricator. I did this for you already and also restarted the CI job. So this is just for future reference :)
>>
>> Please also update `libcxx/docs/Status/SpaceshipProjects.csv`
>
> Thank you very much for the review and the notes and of course for completing the real work previously. I wanted to try out the process of submitting a patch.
>
> A have a couple of questions:
>
> 1. Do I need to add anyone specific as a reviewer, message/ask somebody or I should just wait.

For libc++ not, because #libc <https://reviews.llvm.org/tag/libc/> gets automatically added, so every libc++ approver gets notified. For other subprojects that's not necessarily the case, so you might have to add people.

> 2. If this patch passes the review successfully, would you mind if I try completing some of the other containers too?

Feel free to do so, but make sure there isn't a patch already (AFAICT for spaceship there is only for `array` and `vector` a patch). More contributors are always welcome.

LGTM % nits. I guess you don't have commit access? In that case, please provide `"Your Name" <your.email at address>`.



================
Comment at: libcxx/include/deque:146-149
+template<class T, class Allocator>
+    synth-three-way-result<T> operator<=>(const deque<T, Allocator>& x,
+                                          const deque<T, Allocator>& y);
 
----------------



================
Comment at: libcxx/test/libcxx/containers/sequences/deque/compare.three_way.pass.cpp:16-17
+
+#include <deque>
+#include <cassert>
+
----------------



================
Comment at: libcxx/test/libcxx/containers/sequences/deque/compare.three_way.pass.cpp:26
+}
\ No newline at end of file

----------------
H-G-Hristov wrote:
> avogelsgesang wrote:
> > missing newline at end of file
> > 
> > (I assume you are on Windows? In case you are using VS Code, there is an "Insert Final Newline" setting which you might want to change)
> I don't get it. Is this file still missing an empty line?
No. Phab only complains about it when there isn't one.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144821



More information about the libcxx-commits mailing list