[libc++][spaceship] Implement `operator<=>` for `deque`

[H-G-Hristov]

Summary

Based on https://reviews.llvm.org/D132312
Dependes on https://reviews.llvm.org/D132312

[H-G-Hristov]

Updated papers status

[H-G-Hristov]

• [libc++][spaceship] Implement operator<=> for deque
• Removed test_container_comparisons.h

Based on https://reviews.llvm.org/D132312

[avogelsgesang]

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

libcxx/include/deque

2398

We need to ADL-proof this. My other commit for std::list was actually also missing this. A clang-tidy check in our CI should catch this and report an error for this line...

libcxx/test/libcxx/containers/sequences/deque/compare.three_way.pass.cpp

26

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)

[avogelsgesang]

libcxx/test/libcxx/containers/sequences/deque/compare.three_way.pass.cpp

10

[H-G-Hristov]

• [libc++][spaceship] Implement operator<=> for deque
• Removed test_container_comparisons.h
• Addressed code reviews
• Restored

[H-G-Hristov]

In 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.
2. If this patch passes the review successfully, would you mind if I try completing some of the other containers too?

[H-G-Hristov]

libcxx/test/libcxx/containers/sequences/deque/compare.three_way.pass.cpp

26

I don't get it. Is this file still missing an empty line?

[philnik]

In D144821#4161020, @H-G-Hristov wrote:

In 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 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@address>.

libcxx/include/deque

146–149

libcxx/test/libcxx/containers/sequences/deque/compare.three_way.pass.cpp

17–18

26

No. Phab only complains about it when there isn't one.

[H-G-Hristov]

Addressed review comments

[Mordante]

In D144821#4161664, @philnik wrote:

In D144821#4161020, @H-G-Hristov wrote:

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.

Feel free to pick the items that are unassigned. When you want to pick an assigned item, best discuss on Discord. Some of the assigned items may have a patch already.

Thanks for your contribution, LGTM!

[H-G-Hristov]

• Addressed comment

[H-G-Hristov]

@philnik @Mordante Thank you for your feedback!

Here is my name and email:

Hristo Hristov <[e-mail]>

libcxx/include/deque

 
 template <class InputIterator, class Allocator = allocator<typename iterator_traits<InputIterator>::value_type>>
    deque(InputIterator, InputIterator, Allocator = Allocator())
    -> deque<typename iterator_traits<InputIterator>::value_type, Allocator>; // C++17
 
 template <class T, class Allocator>
     bool operator==(const deque<T,Allocator>& x, const deque<T,Allocator>& y);
 template <class T, class Allocator>
-    bool operator< (const deque<T,Allocator>& x, const deque<T,Allocator>& y);
+    bool operator< (const deque<T,Allocator>& x, const deque<T,Allocator>& y); // removed in C++20
 template <class T, class Allocator>
-    bool operator!=(const deque<T,Allocator>& x, const deque<T,Allocator>& y);
+    bool operator!=(const deque<T,Allocator>& x, const deque<T,Allocator>& y); // removed in C++20
 template <class T, class Allocator>
-    bool operator> (const deque<T,Allocator>& x, const deque<T,Allocator>& y);
+    bool operator> (const deque<T,Allocator>& x, const deque<T,Allocator>& y); // removed in C++20
 template <class T, class Allocator>
-    bool operator>=(const deque<T,Allocator>& x, const deque<T,Allocator>& y);
+    bool operator>=(const deque<T,Allocator>& x, const deque<T,Allocator>& y); // removed in C++20
 template <class T, class Allocator>
-    bool operator<=(const deque<T,Allocator>& x, const deque<T,Allocator>& y);
+    bool operator<=(const deque<T,Allocator>& x, const deque<T,Allocator>& y); // removed in C++20
+template<class T, class Allocator>
+    synth-three-way-result<T> operator<=>(const deque<T, Allocator>& x,
+                                          const deque<T, Allocator>& y);
 
 // specialized algorithms:
 template <class T, class Allocator>
     void swap(deque<T,Allocator>& x, deque<T,Allocator>& y)
          noexcept(noexcept(x.swap(y)));
 
 template <class T, class Allocator, class U>
     typename deque<T, Allocator>::size_type
     erase(deque<T, Allocator>& c, const U& value);       // C++20
 template <class T, class Allocator, class Predicate>
     typename deque<T, Allocator>::size_type
     erase_if(deque<T, Allocator>& c, Predicate pred);    // C++20
 
 }  // std
 
 */
 
 #include <__algorithm/copy.h>
 #include <__algorithm/copy_backward.h>
 #include <__algorithm/equal.h>
 #include <__algorithm/fill_n.h>
 #include <__algorithm/lexicographical_compare.h>
+#include <__algorithm/lexicographical_compare_three_way.h>
 #include <__algorithm/min.h>
 #include <__algorithm/remove.h>
 #include <__algorithm/remove_if.h>
 #include <__algorithm/unwrap_iter.h>
 #include <__assert> // all public C++ headers provide the assertion handler
 #include <__config>
 #include <__format/enable_insertable.h>
 #include <__iterator/iterator_traits.h>

 inline _LIBCPP_HIDE_FROM_ABI
 bool
 operator==(const deque<_Tp, _Allocator>& __x, const deque<_Tp, _Allocator>& __y)
 {
     const typename deque<_Tp, _Allocator>::size_type __sz = __x.size();
     return __sz == __y.size() && _VSTD::equal(__x.begin(), __x.end(), __y.begin());
 }
 
+#if _LIBCPP_STD_VER <= 17
+
 template <class _Tp, class _Allocator>
 inline _LIBCPP_HIDE_FROM_ABI
 bool
 operator!=(const deque<_Tp, _Allocator>& __x, const deque<_Tp, _Allocator>& __y)
 {
     return !(__x == __y);
 }
 

 template <class _Tp, class _Allocator>
 inline _LIBCPP_HIDE_FROM_ABI
 bool
 operator<=(const deque<_Tp, _Allocator>& __x, const deque<_Tp, _Allocator>& __y)
 {
     return !(__y < __x);
 }
 
+#else // _LIBCPP_STD_VER <= 17
+
+template<class _Tp, class _Allocator>
+inline _LIBCPP_HIDE_FROM_ABI
+__synth_three_way_result<_Tp>
+operator<=>(const deque<_Tp, _Allocator>& __x, const deque<_Tp, _Allocator>& __y)
+{
+    return lexicographical_compare_three_way(
+        __x.begin(), __x.end(), __y.begin(), __y.end(), __synth_three_way);
+}
+
+#endif // _LIBCPP_STD_VER <= 17
+
 template <class _Tp, class _Allocator>
 inline _LIBCPP_HIDE_FROM_ABI
 void
 swap(deque<_Tp, _Allocator>& __x, deque<_Tp, _Allocator>& __y)
     _NOEXCEPT_(_NOEXCEPT_(__x.swap(__y)))
 {
     __x.swap(__y);
 }

libcxx/test/libcxx/containers/sequences/deque/compare.three_way.pass.cpp

+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+// UNSUPPORTED: c++03, c++11, c++14, c++17
+
+// <list>
+
+// template<class T, class Allocator>
+//   synth-three-way-result<T> operator<=>(const deque<T, Allocator>& x,
+//                                         const deque<T, Allocator>& y);
+
+#include <deque>
+#include <cassert>
+
+#include "test_container_comparisons.h"
+
+int main(int, char**) {
+  assert(test_ordered_container_spaceship<std::deque>());
+  // `std::deque` is not constexpr, so no `static_assert` test here.
+  return 0;
+}
 No newline at end of file