[libc++] Implement `operator<=>` for `unique_ptr`

[avogelsgesang]

Summary

Implements part of:

• P1614R2 The Mothership has Landed

Fixes LWG3426

[avogelsgesang]

Fix tests on older C++ versions

[avogelsgesang]

need to update based on comments in https://reviews.llvm.org/D130852

[avogelsgesang]

Update based on feedback from D130852

[Mordante]

Thanks! Looks quite good overall, but a few remarks.

libcxx/include/__memory/unique_ptr.h

601

711

libcxx/include/memory

509

Interesting these aren't removed, including in the WP.

libcxx/test/std/utilities/smartptr/unique.ptr/unique.ptr.special/cmp.pass.cpp

72 ↗

(On Diff #449377)

Can you also test with the other two comparison categories? Not 100% sure it's even possible.

libcxx/test/std/utilities/smartptr/unique.ptr/unique.ptr.special/cmp_nullptr.pass.cpp

22

Nice catch! It seems we've added _NOEXCEPT in the code. @ldionne should we remove these noexcepts in a separate commit?

66

[avogelsgesang]

address review comments

libcxx/include/memory

509

I agree. The revision history of P1614R2 states

The comparisons between unique_ptr<T, D> and nullptr were originally removed and replaced with a <=>, but this was reverted.

But I don't know where to find the history on why that change was made

libcxx/test/std/utilities/smartptr/unique.ptr/unique.ptr.special/cmp.pass.cpp

72 ↗

(On Diff #449377)

I don't understand.

unique_ptr <=> unique_ptr is specified to return std::strong_ordering. If I added, e.g.,

AssertOrderReturn<std::weak_ordering, std::unique_ptr<int>>();

this test would fail because the result of unique_ptr <=> unique_ptr is strong_ordering, not weak_ordering

[Mordante]

LGTM, thanks!
I'll commit this on your behalf.

libcxx/test/std/utilities/smartptr/unique.ptr/unique.ptr.special/cmp.pass.cpp

72 ↗

(On Diff #449377)

After I wrote the question I wondered whether it would be possible at all. The odd part is that std::shared_ptr uses strong_ordering, but unique_ptr deduces the ordering. But it seems it indeed should always be strong_ordering.

libcxx/docs/ReleaseNotes.rst

 ============================
 
 Implemented Papers
 ------------------
 
 Improvements and New Features
 -----------------------------
 
+* Implemented ``operator<=>`` for ``std::unique_ptr``
+
 Deprecations and Removals
 -------------------------
 
 Upcoming Deprecations and Removals
 ----------------------------------
 
 ABI Affecting Changes
 ---------------------
 
 Build System Changes
 --------------------

libcxx/docs/Status/SpaceshipProjects.csv

 | `[tuple.rel] <https://wg21.link/tuple.rel>`_,| `tuple <https://reviews.llvm.org/D108250>`_,[expos.only.func],Kent Ross,|Complete|
 "| `[optional.relops] <https://wg21.link/optional.relops>`_
 | `[optional.nullops] <https://wg21.link/optional.nullops>`_
 | `[optional.comp.with.t] <https://wg21.link/optional.comp.with.t>`_","| optional
 | nullopt",None,Kent Ross,|In Progress|
 "| `[variant.relops] <https://wg21.link/variant.relops>`_
 | `[variant.monostate.relops] <https://wg21.link/variant.monostate.relops>`_","| monostate
 | variant",None,Kent Ross,|In Progress|
-| `[unique.ptr.special] <https://wg21.link/unique.ptr.special>`_,| unique_ptr,[comparisons.three.way],Unassigned,|Not Started|
+| `[unique.ptr.special] <https://wg21.link/unique.ptr.special>`_,| unique_ptr,[comparisons.three.way],Adrian Vogelsgesang,|Complete|
 | `[util.smartptr.shared.cmp] <https://wg21.link/util.smartptr.shared.cmp>`_,| shared_ptr,[comparisons.three.way],Unassigned,|Not Started|
 | `[type.index.members] <https://wg21.link/type.index.members>`_,| type_index,None,Unassigned,|Not Started|
 | `[charconv.syn] <https://wg21.link/charconv.syn>`_,| to_chars_result,None,Mark de Wever,|Complete|
 | `[charconv.syn] <https://wg21.link/charconv.syn>`_,| from_chars_result,None,Mark de Wever,|Complete|
 | `[stacktrace.entry.cmp] <https://wg21.link/stacktrace.entry.cmp>`_,| stacktrace_entry,None,Unassigned,|Not Started|
 | `[stacktrace.basic.cmp] <https://wg21.link/stacktrace.basic.cmp>`_,| basic_stacktrace,[alg.three.way],Unassigned,|Not Started|
 | `[string.cmp] <https://wg21.link/string.cmp>`_,| `basic_string <https://reviews.llvm.org/D80895>`_,None,Christopher Di Bella,|In Progress|
 | `[string.view.comparison] <https://wg21.link/string.view.comparison>`_,| `basic_string_view <https://reviews.llvm.org/D80891>`_,None,Christopher Di Bella,|In Progress|

libcxx/include/__memory/unique_ptr.h

 // -*- C++ -*-
 //===----------------------------------------------------------------------===//
 //
 // 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
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef _LIBCPP___MEMORY_UNIQUE_PTR_H
 #define _LIBCPP___MEMORY_UNIQUE_PTR_H
 
+#include <__compare/compare_three_way.h>
+#include <__compare/compare_three_way_result.h>
+#include <__compare/three_way_comparable.h>
 #include <__config>
 #include <__functional/hash.h>
 #include <__functional/operations.h>
 #include <__memory/allocator_traits.h> // __pointer
 #include <__memory/auto_ptr.h>
 #include <__memory/compressed_pair.h>
 #include <__utility/forward.h>
 #include <__utility/move.h>

 >::type
 swap(unique_ptr<_Tp, _Dp>& __x, unique_ptr<_Tp, _Dp>& __y) _NOEXCEPT {__x.swap(__y);}
 
 template <class _T1, class _D1, class _T2, class _D2>
 inline _LIBCPP_INLINE_VISIBILITY
 bool
 operator==(const unique_ptr<_T1, _D1>& __x, const unique_ptr<_T2, _D2>& __y) {return __x.get() == __y.get();}
 
+#if _LIBCPP_STD_VER <= 17
 template <class _T1, class _D1, class _T2, class _D2>
 inline _LIBCPP_INLINE_VISIBILITY
 bool
 operator!=(const unique_ptr<_T1, _D1>& __x, const unique_ptr<_T2, _D2>& __y) {return !(__x == __y);}
+#endif
 
 template <class _T1, class _D1, class _T2, class _D2>
 inline _LIBCPP_INLINE_VISIBILITY
 bool
 operator< (const unique_ptr<_T1, _D1>& __x, const unique_ptr<_T2, _D2>& __y)
 {
     typedef typename unique_ptr<_T1, _D1>::pointer _P1;
     typedef typename unique_ptr<_T2, _D2>::pointer _P2;

 bool
 operator<=(const unique_ptr<_T1, _D1>& __x, const unique_ptr<_T2, _D2>& __y) {return !(__y < __x);}
 
 template <class _T1, class _D1, class _T2, class _D2>
 inline _LIBCPP_INLINE_VISIBILITY
 bool
 operator>=(const unique_ptr<_T1, _D1>& __x, const unique_ptr<_T2, _D2>& __y) {return !(__x < __y);}
 
+
+#if _LIBCPP_STD_VER > 17
+template <class _T1, class _D1, class _T2, class _D2>
+requires three_way_comparable_with<typename unique_ptr<_T1, _D1>::pointer,
+                                   typename unique_ptr<_T2, _D2>::pointer>
+compare_three_way_result_t<typename unique_ptr<_T1, _D1>::pointer,
+                           typename unique_ptr<_T2, _D2>::pointer>
+operator<=>(const unique_ptr<_T1, _D1>& __x, const unique_ptr<_T2, _D2>& __y) {
+   return compare_three_way()(__x.get(), __y.get());
+}
+#endif
+
 template <class _T1, class _D1>
 inline _LIBCPP_INLINE_VISIBILITY
 bool
 operator==(const unique_ptr<_T1, _D1>& __x, nullptr_t) _NOEXCEPT
 {
     return !__x;
 }
 
+#if _LIBCPP_STD_VER <= 17
 template <class _T1, class _D1>
 inline _LIBCPP_INLINE_VISIBILITY
 bool
 operator==(nullptr_t, const unique_ptr<_T1, _D1>& __x) _NOEXCEPT
 {
     return !__x;
 }
 
 template <class _T1, class _D1>
 inline _LIBCPP_INLINE_VISIBILITY
 bool
 operator!=(const unique_ptr<_T1, _D1>& __x, nullptr_t) _NOEXCEPT
 {
     return static_cast<bool>(__x);
 }
 
 template <class _T1, class _D1>
 inline _LIBCPP_INLINE_VISIBILITY
 bool
 operator!=(nullptr_t, const unique_ptr<_T1, _D1>& __x) _NOEXCEPT
 {
     return static_cast<bool>(__x);
 }
+#endif // _LIBCPP_STD_VER <= 17
 
 template <class _T1, class _D1>
 inline _LIBCPP_INLINE_VISIBILITY
 bool
 operator<(const unique_ptr<_T1, _D1>& __x, nullptr_t)
 {
     typedef typename unique_ptr<_T1, _D1>::pointer _P1;
     return less<_P1>()(__x.get(), nullptr);

 template <class _T1, class _D1>
 inline _LIBCPP_INLINE_VISIBILITY
 bool
 operator>=(nullptr_t, const unique_ptr<_T1, _D1>& __x)
 {
     return !(nullptr < __x);
 }
 
+#if _LIBCPP_STD_VER > 17
+template <class _T1, class _D1>
+requires three_way_comparable<typename unique_ptr<_T1, _D1>::pointer>
+compare_three_way_result_t<typename unique_ptr<_T1, _D1>::pointer>
+operator<=>(const unique_ptr<_T1, _D1>& __x, nullptr_t) {
+   return compare_three_way()(__x.get(), static_cast<typename unique_ptr<_T1, _D1>::pointer>(nullptr));
+}
+#endif
+
 #if _LIBCPP_STD_VER > 11
 
 template<class _Tp>
 struct __unique_if
 {
     typedef unique_ptr<_Tp> __unique_single;
 };
 

libcxx/include/memory

 };
 
 template <class T, class D>
     void swap(unique_ptr<T, D>& x, unique_ptr<T, D>& y) noexcept;
 
 template <class T1, class D1, class T2, class D2>
     bool operator==(const unique_ptr<T1, D1>& x, const unique_ptr<T2, D2>& y);
 template <class T1, class D1, class T2, class D2>
-    bool operator!=(const unique_ptr<T1, D1>& x, const unique_ptr<T2, D2>& y);
+    bool operator!=(const unique_ptr<T1, D1>& x, const unique_ptr<T2, D2>& y);    // removed in C++20
 template <class T1, class D1, class T2, class D2>
     bool operator<(const unique_ptr<T1, D1>& x, const unique_ptr<T2, D2>& y);
 template <class T1, class D1, class T2, class D2>
     bool operator<=(const unique_ptr<T1, D1>& x, const unique_ptr<T2, D2>& y);
 template <class T1, class D1, class T2, class D2>
     bool operator>(const unique_ptr<T1, D1>& x, const unique_ptr<T2, D2>& y);
 template <class T1, class D1, class T2, class D2>
     bool operator>=(const unique_ptr<T1, D1>& x, const unique_ptr<T2, D2>& y);
+template<class T1, class D1, class T2, class D2>
+  requires three_way_comparable_with<typename unique_ptr<T1, D1>::pointer,
+                                     typename unique_ptr<T2, D2>::pointer>
+  compare_three_way_result_t<typename unique_ptr<T1, D1>::pointer,
+                             typename unique_ptr<T2, D2>::pointer>
+    operator<=>(const unique_ptr<T1, D1>& x, const unique_ptr<T2, D2>& y);      // C++20
 
 template <class T, class D>
     bool operator==(const unique_ptr<T, D>& x, nullptr_t) noexcept;
 template <class T, class D>
-    bool operator==(nullptr_t, const unique_ptr<T, D>& y) noexcept;
+    bool operator==(nullptr_t, const unique_ptr<T, D>& y) noexcept;     // removed in C++20
 template <class T, class D>
-    bool operator!=(const unique_ptr<T, D>& x, nullptr_t) noexcept;
+    bool operator!=(const unique_ptr<T, D>& x, nullptr_t) noexcept;     // removed in C++20
 template <class T, class D>
-    bool operator!=(nullptr_t, const unique_ptr<T, D>& y) noexcept;
+    bool operator!=(nullptr_t, const unique_ptr<T, D>& y) noexcept;     // removed in C++20
 
 template <class T, class D>
     bool operator<(const unique_ptr<T, D>& x, nullptr_t);
 template <class T, class D>
     bool operator<(nullptr_t, const unique_ptr<T, D>& y);
 template <class T, class D>
     bool operator<=(const unique_ptr<T, D>& x, nullptr_t);
 template <class T, class D>
     bool operator<=(nullptr_t, const unique_ptr<T, D>& y);
 template <class T, class D>
     bool operator>(const unique_ptr<T, D>& x, nullptr_t);
 template <class T, class D>
     bool operator>(nullptr_t, const unique_ptr<T, D>& y);
 template <class T, class D>
     bool operator>=(const unique_ptr<T, D>& x, nullptr_t);
 template <class T, class D>
     bool operator>=(nullptr_t, const unique_ptr<T, D>& y);
+template<class T, class D>
+  requires three_way_comparable<typename unique_ptr<T, D>::pointer>
+  compare_three_way_result_t<typename unique_ptr<T, D>::pointer>
+    operator<=>(const unique_ptr<T, D>& x, nullptr_t);                          // C++20
 
 class bad_weak_ptr
     : public std::exception
 {
     bad_weak_ptr() noexcept;
 };
 
 template<class T, class... Args> unique_ptr<T> make_unique(Args&&... args);     // C++14

libcxx/test/std/utilities/smartptr/unique.ptr/unique.ptr.special/cmp_nullptr.pass.cpp

 //     bool operator==(const unique_ptr<T, D>& x, nullptr_t) noexcept;
 // template <class T, class D>
 //     bool operator==(nullptr_t, const unique_ptr<T, D>& y) noexcept;
 // template <class T, class D>
 //     bool operator!=(const unique_ptr<T, D>& x, nullptr_t) noexcept;
 // template <class T, class D>
 //     bool operator!=(nullptr_t, const unique_ptr<T, D>& y) noexcept;
 // template <class T, class D>
 //     bool operator<(const unique_ptr<T, D>& x, nullptr_t) noexcept;
 // template <class T, class D>
 //     bool operator<(nullptr_t, const unique_ptr<T, D>& y) noexcept;
 // template <class T, class D>
 //     bool operator<=(const unique_ptr<T, D>& x, nullptr_t) noexcept;
 // template <class T, class D>
 //     bool operator<=(nullptr_t, const unique_ptr<T, D>& y) noexcept;
 // template <class T, class D>
 //     bool operator>(const unique_ptr<T, D>& x, nullptr_t) noexcept;
 // template <class T, class D>
 //     bool operator>(nullptr_t, const unique_ptr<T, D>& y) noexcept;
 // template <class T, class D>
 //     bool operator>=(const unique_ptr<T, D>& x, nullptr_t) noexcept;
 // template <class T, class D>
 //     bool operator>=(nullptr_t, const unique_ptr<T, D>& y) noexcept;
+// template<class T, class D>
+//   requires three_­way_­comparable<typename unique_ptr<T, D>::pointer>
+//   constexpr compare_three_way_result_t<typename unique_ptr<T, D>::pointer>
+//     operator<=>(const unique_ptr<T, D>& x, nullptr_t);                            // C++20
 
 #include <memory>
 #include <cassert>
 
 #include "test_macros.h"
 
 void do_nothing(int*) {}
 
 int main(int, char**)
 {
     const std::unique_ptr<int> p1(new int(1));
     assert(!(p1 == nullptr));
     assert(!(nullptr == p1));
     assert(!(p1 < nullptr));
     assert( (nullptr < p1));
     assert(!(p1 <= nullptr));
     assert( (nullptr <= p1));
     assert( (p1 > nullptr));
     assert(!(nullptr > p1));
     assert( (p1 >= nullptr));
     assert(!(nullptr >= p1));
+#if TEST_STD_VER > 17
+    assert((nullptr <=> p1) == std::strong_ordering::less);
+    assert((p1 <=> nullptr) == std::strong_ordering::greater);
+#endif
 
     const std::unique_ptr<int> p2;
     assert( (p2 == nullptr));
     assert( (nullptr == p2));
     assert(!(p2 < nullptr));
     assert(!(nullptr < p2));
     assert( (p2 <= nullptr));
     assert( (nullptr <= p2));
     assert(!(p2 > nullptr));
     assert(!(nullptr > p2));
     assert( (p2 >= nullptr));
     assert( (nullptr >= p2));
+#if TEST_STD_VER > 17
+    assert((nullptr <=> p2) == std::strong_ordering::equivalent);
+#endif
 
     return 0;
 }

libcxx/test/std/utilities/smartptr/unique.ptr/unique.ptr.special/rel.pass.cpp

 // template <class T1, class D1, class T2, class D2>
 //   bool
 //   operator<=(const unique_ptr<T1, D1>& x, const unique_ptr<T2, D2>& y);
 
 // template <class T1, class D1, class T2, class D2>
 //   bool
 //   operator>=(const unique_ptr<T1, D1>& x, const unique_ptr<T2, D2>& y);
 
+// template<class T1, class D1, class T2, class D2>
+//   requires three_way_comparable_with<typename unique_ptr<T1, D1>::pointer,
+//                                      typename unique_ptr<T2, D2>::pointer>
+//   compare_three_way_result_t<typename unique_ptr<T1, D1>::pointer,
+//                              typename unique_ptr<T2, D2>::pointer>
+//     operator<=>(const unique_ptr<T1, D1>& x, const unique_ptr<T2, D2>& y);
+
 #include <memory>
 #include <cassert>
 
 #include "test_macros.h"
 #include "deleter_types.h"
 
 struct A
 {
struct B
     static int count;
     B() {++count;}
     B(const B& other) : A(other) {++count;}
     virtual ~B() {--count;}
 };
 
 int B::count = 0;
 
+static std::strong_ordering reverse(std::strong_ordering v) { return 0 <=> v; }
+
 int main(int, char**)
 {
     {
     const std::unique_ptr<A, Deleter<A> > p1(new A);
     const std::unique_ptr<A, Deleter<A> > p2(new A);
     assert((p1 < p2) == !(p1 > p2));
     assert((p1 < p2) == (p1 <= p2));
     assert((p1 < p2) == !(p1 >= p2));
+#if TEST_STD_VER > 17
+    assert((p1 <=> p1) == std::strong_ordering::equivalent);
+    assert((p1 <=> p2) == reverse(p2 <=> p1));
+#endif
     }
     {
     const std::unique_ptr<A, Deleter<A> > p1(new A);
     const std::unique_ptr<B, Deleter<B> > p2(new B);
     assert((p1 < p2) == !(p1 > p2));
     assert((p1 < p2) == (p1 <= p2));
     assert((p1 < p2) == !(p1 >= p2));
+#if TEST_STD_VER > 17
+    assert((p1 <=> p2) == reverse(p2 <=> p1));
+#endif
     }
     {
     const std::unique_ptr<A[], Deleter<A[]> > p1(new A[3]);
     const std::unique_ptr<A[], Deleter<A[]> > p2(new A[3]);
     assert((p1 < p2) == !(p1 > p2));
     assert((p1 < p2) == (p1 <= p2));
     assert((p1 < p2) == !(p1 >= p2));
+#if TEST_STD_VER > 17
+    assert((p1 <=> p2) == reverse(p2 <=> p1));
+#endif
     }
     {
     const std::unique_ptr<A[], Deleter<A[]> > p1(new A[3]);
     const std::unique_ptr<B[], Deleter<B[]> > p2(new B[3]);
     assert((p1 < p2) == !(p1 > p2));
     assert((p1 < p2) == (p1 <= p2));
     assert((p1 < p2) == !(p1 >= p2));
+#if TEST_STD_VER > 17
+    assert((p1 <=> p2) == reverse(p2 <=> p1));
+#endif
     }
     {
     const std::unique_ptr<A, Deleter<A> > p1;
     const std::unique_ptr<A, Deleter<A> > p2;
     assert((p1 < p2) == (p1 > p2));
     assert((p1 < p2) == !(p1 <= p2));
     assert((p1 < p2) == !(p1 >= p2));
+#if TEST_STD_VER > 17
+    assert((p1 <=> p2) == reverse(p2 <=> p1));
+#endif
     }
     {
     const std::unique_ptr<A, Deleter<A> > p1;
     const std::unique_ptr<B, Deleter<B> > p2;
     assert((p1 < p2) == (p1 > p2));
     assert((p1 < p2) == !(p1 <= p2));
     assert((p1 < p2) == !(p1 >= p2));
+#if TEST_STD_VER > 17
+    assert((p1 <=> p2) == reverse(p2 <=> p1));
+#endif
     }
 
   return 0;
 }