[libc++] LWG2899: Constrain move special functions of tuple and unique_ptr

libc++'s `tuple`'s move constructor is well-constrained when initially
implemented. So this patch only adds test cases.

For `unique_ptr`, its move constructor and move assignment operator were
previously unconstrained and thus this patch changes them.
- In C++20 and later, I think it's better to use obviously correct
`requires`-clauses.
- In C++17 and earlier, there doesn't seem "obviously correct" approach
for constraining. I'm attempting to use `_nat` trick to avoid turning
the functions into templates (which are not move special functions).

Some tests case are adjusted because false positive of
move-assignability of `unique_ptr` is reduced. Comments are updated to
reflect that move-constructibility is not actually required for
`unique_ptr`'s deleter.

This patch also explicitly deletes copy functions of `unique_ptr` in all
modes. Previously, they are implicitly deleted since C++11 mode,
although the standard wording always explicitly deletes them. Clang
seems to be somehow buggy on propagating deleted-ness of special member
functions from unnamed structs, while the explicit deletion can act as a
workaround.

The title of this patch is adjusted to reflect the final resolution of
LWG2899.

Author: frederick-vs-ja

libcxx/docs/Status/Cxx20Issues.csv

@@ -143,7 +143,7 @@
 "`LWG3180 <https://wg21.link/LWG3180>`__","Inconsistently named return type for ``ranges::minmax_element``\ ","2019-02 (Kona)","|Complete|","15","`#103844 <https://github.com/llvm/llvm-project/issues/103844>`__",""
 "`LWG3182 <https://wg21.link/LWG3182>`__","Specification of ``Same``\  could be clearer","2019-02 (Kona)","|Complete|","15","`#103845 <https://github.com/llvm/llvm-project/issues/103845>`__",""
 "","","","","","",""
-"`LWG2899 <https://wg21.link/LWG2899>`__","``is_(nothrow_)move_constructible``\  and ``tuple``\ , ``optional``\  and ``unique_ptr``\ ","2019-07 (Cologne)","","","`#100255 <https://github.com/llvm/llvm-project/issues/100255>`__",""
+"`LWG2899 <https://wg21.link/LWG2899>`__","``is_(nothrow_)move_constructible``\  and ``tuple``\ , ``optional``\  and ``unique_ptr``\ ","2019-07 (Cologne)","|Complete|","22","`#100255 <https://github.com/llvm/llvm-project/issues/100255>`__",""
 "`LWG3055 <https://wg21.link/LWG3055>`__","``path::operator+=(*single-character*)``\  misspecified","2019-07 (Cologne)","|Complete|","7","`#103846 <https://github.com/llvm/llvm-project/issues/103846>`__",""
 "`LWG3158 <https://wg21.link/LWG3158>`__","``tuple(allocator_arg_t, const Alloc&)``\  should be conditionally explicit","2019-07 (Cologne)","|Complete|","10","`#103847 <https://github.com/llvm/llvm-project/issues/103847>`__",""
 "`LWG3169 <https://wg21.link/LWG3169>`__","``ranges``\  permutation generators discard useful information","2019-07 (Cologne)","|Complete|","15","`#103848 <https://github.com/llvm/llvm-project/issues/103848>`__",""

libcxx/include/__memory/unique_ptr.h

@@ -45,6 +45,7 @@
 #include <__type_traits/is_trivially_relocatable.h>
 #include <__type_traits/is_unbounded_array.h>
 #include <__type_traits/is_void.h>
+#include <__type_traits/nat.h>
 #include <__type_traits/remove_extent.h>
 #include <__type_traits/type_identity.h>
 #include <__utility/declval.h>
@@ -208,9 +209,15 @@ class _LIBCPP_UNIQUE_PTR_TRIVIAL_ABI unique_ptr {
   template <bool _Dummy = true, class = _EnableIfDeleterConstructible<_BadRValRefType<_Dummy> > >
   _LIBCPP_HIDE_FROM_ABI unique_ptr(pointer __p, _BadRValRefType<_Dummy> __d) = delete;
 
-  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 unique_ptr(unique_ptr&& __u) _NOEXCEPT
-      : __ptr_(__u.release()),
-        __deleter_(std::forward<deleter_type>(__u.get_deleter())) {}
+  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23
+#if _LIBCPP_STD_VER >= 20
+  unique_ptr(unique_ptr&& __u) noexcept
+    requires is_move_constructible_v<_Dp>
+#else
+  unique_ptr(_If<is_move_constructible<_Dp>::value, unique_ptr&&, __nat> __u) _NOEXCEPT
+#endif
+      : __ptr_(__u.release()), __deleter_(std::forward<deleter_type>(__u.get_deleter())) {
+  }
 
   template <class _Up,
             class _Ep,
@@ -226,7 +233,14 @@ class _LIBCPP_UNIQUE_PTR_TRIVIAL_ABI unique_ptr {
   _LIBCPP_HIDE_FROM_ABI unique_ptr(auto_ptr<_Up>&& __p) _NOEXCEPT : __ptr_(__p.release()), __deleter_() {}
 #endif
 
-  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 unique_ptr& operator=(unique_ptr&& __u) _NOEXCEPT {
+  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 unique_ptr&
+#if _LIBCPP_STD_VER >= 20
+  operator=(unique_ptr&& __u) noexcept
+    requires is_move_assignable_v<_Dp>
+#else
+  operator=(_If<is_move_assignable<_Dp>::value, unique_ptr&&, __nat> __u) _NOEXCEPT
+#endif
+  {
     reset(__u.release());
     __deleter_ = std::forward<deleter_type>(__u.get_deleter());
     return *this;
@@ -251,10 +265,8 @@ class _LIBCPP_UNIQUE_PTR_TRIVIAL_ABI unique_ptr {
   }
 #endif
 
-#ifdef _LIBCPP_CXX03_LANG
   unique_ptr(unique_ptr const&)            = delete;
   unique_ptr& operator=(unique_ptr const&) = delete;
-#endif
 
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 ~unique_ptr() { reset(); }
 
@@ -532,12 +544,26 @@ class _LIBCPP_UNIQUE_PTR_TRIVIAL_ABI unique_ptr<_Tp[], _Dp> {
             class       = _EnableIfPointerConvertible<_Pp> >
   _LIBCPP_HIDE_FROM_ABI unique_ptr(_Pp __ptr, _BadRValRefType<_Dummy> __deleter) = delete;
 
-  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 unique_ptr(unique_ptr&& __u) _NOEXCEPT
+  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23
+#if _LIBCPP_STD_VER >= 20
+  unique_ptr(unique_ptr&& __u) noexcept
+    requires is_move_constructible_v<_Dp>
+#else
+  unique_ptr(_If<is_move_constructible<_Dp>::value, unique_ptr&&, __nat> __u) _NOEXCEPT
+#endif
       : __ptr_(__u.release()),
         __deleter_(std::forward<deleter_type>(__u.get_deleter())),
-        __checker_(std::move(__u.__checker_)) {}
+        __checker_(std::move(__u.__checker_)) {
+  }
 
-  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 unique_ptr& operator=(unique_ptr&& __u) _NOEXCEPT {
+  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 unique_ptr&
+#if _LIBCPP_STD_VER >= 20
+  operator=(unique_ptr&& __u) noexcept
+    requires is_move_assignable_v<_Dp>
+#else
+  operator=(_If<is_move_assignable<_Dp>::value, unique_ptr&&, __nat> __u) _NOEXCEPT
+#endif
+  {
     reset(__u.release());
     __deleter_ = std::forward<deleter_type>(__u.get_deleter());
     __checker_ = std::move(__u.__checker_);
@@ -564,10 +590,8 @@ class _LIBCPP_UNIQUE_PTR_TRIVIAL_ABI unique_ptr<_Tp[], _Dp> {
     return *this;
   }
 
-#ifdef _LIBCPP_CXX03_LANG
   unique_ptr(unique_ptr const&)            = delete;
   unique_ptr& operator=(unique_ptr const&) = delete;
-#endif
 
 public:
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 ~unique_ptr() { reset(); }

libcxx/test/std/utilities/smartptr/unique.ptr/unique.ptr.class/unique.ptr.asgn/move.pass.cpp

@@ -112,19 +112,26 @@ TEST_CONSTEXPR_CXX23 void test_sfinae() {
     static_assert(!std::is_assignable<U, const U&&>::value, "");
     static_assert(std::is_nothrow_assignable<U, U&&>::value, "");
   }
+  {
+    typedef std::unique_ptr<VT, NCDeleter<VT> > U;
+    static_assert(!std::is_assignable<U, U&>::value, "");
+    static_assert(!std::is_assignable<U, const U&>::value, "");
+    static_assert(!std::is_assignable<U, const U&&>::value, "");
+    static_assert(!std::is_assignable<U, U&&>::value, "");
+  }
   {
     typedef std::unique_ptr<VT, NCDeleter<VT>&> U;
     static_assert(!std::is_assignable<U, U&>::value, "");
     static_assert(!std::is_assignable<U, const U&>::value, "");
     static_assert(!std::is_assignable<U, const U&&>::value, "");
-    static_assert(std::is_nothrow_assignable<U, U&&>::value, "");
+    static_assert(!std::is_assignable<U, U&&>::value, "");
   }
   {
     typedef std::unique_ptr<VT, const NCDeleter<VT>&> U;
     static_assert(!std::is_assignable<U, U&>::value, "");
     static_assert(!std::is_assignable<U, const U&>::value, "");
     static_assert(!std::is_assignable<U, const U&&>::value, "");
-    static_assert(std::is_nothrow_assignable<U, U&&>::value, "");
+    static_assert(!std::is_assignable<U, U&&>::value, "");
   }
 }
 

libcxx/test/std/utilities/smartptr/unique.ptr/unique.ptr.class/unique.ptr.asgn/move_convert.pass.cpp

@@ -220,7 +220,7 @@ TEST_CONSTEXPR_CXX23 void test_sfinae() {
     static_assert(!std::is_assignable<U1, U5&&>::value, "");
 
     using U1C = std::unique_ptr<const VT, GenericConvertingDeleter<0> const&>;
-    static_assert(std::is_nothrow_assignable<U1C, U1&&>::value, "");
+    static_assert(!std::is_assignable<U1C, U1&&>::value, "");
   }
   { // Test that if the deleter assignment is not valid the assignment operator
     // SFINAEs.
@@ -296,7 +296,7 @@ TEST_CONSTEXPR_CXX23 void test_noexcept() {
   {
     typedef std::unique_ptr<const VT, NCDeleter<const VT>&> APtr;
     typedef std::unique_ptr<VT, NCDeleter<const VT>&> BPtr;
-    static_assert(std::is_nothrow_assignable<APtr, BPtr>::value, "");
+    static_assert(!std::is_assignable<APtr, BPtr>::value, "");
   }
   {
     typedef std::unique_ptr<const VT, const NCConstDeleter<const VT>&> APtr;

libcxx/test/std/utilities/smartptr/unique.ptr/unique.ptr.class/unique.ptr.asgn/move_convert.runtime.pass.cpp

@@ -89,7 +89,7 @@ void test_sfinae() {
     static_assert(!std::is_assignable<UAC, const UA&>::value, "");
   }
   { // cannot move if the deleter-types cannot convert
-    static_assert(std::is_assignable<UACD, UAD&&>::value, "");
+    static_assert(!std::is_assignable<UACD, UAD&&>::value, "");
     static_assert(!std::is_assignable<UACD, UAC&&>::value, "");
     static_assert(!std::is_assignable<UAC, UACD&&>::value, "");
   }

libcxx/test/std/utilities/smartptr/unique.ptr/unique.ptr.class/unique.ptr.asgn/move_convert.single.pass.cpp

@@ -87,7 +87,7 @@ TEST_CONSTEXPR_CXX23 void test_sfinae() {
     static_assert(!std::is_assignable<UA, const UB&>::value, "");
   }
   { // cannot move if the deleter-types cannot convert
-    static_assert(std::is_assignable<UAD, UBD&&>::value, "");
+    static_assert(!std::is_assignable<UAD, UBD&&>::value, "");
     static_assert(!std::is_assignable<UAD, UB&&>::value, "");
     static_assert(!std::is_assignable<UA, UBD&&>::value, "");
   }

libcxx/test/std/utilities/smartptr/unique.ptr/unique.ptr.class/unique.ptr.ctor/move.pass.cpp

@@ -12,6 +12,8 @@
 
 // Test unique_ptr move ctor
 
+// XFAIL: FROZEN-CXX03-HEADERS-FIXME
+
 #include <memory>
 #include <utility>
 #include <cassert>
@@ -24,10 +26,8 @@
 //
 // Concerns
 //   1 The moved from pointer is empty and the new pointer stores the old value.
-//   2 The only requirement on the deleter is that it is MoveConstructible
-//     or a reference.
-//   3 The constructor works for explicitly moved values (i.e. std::move(x))
-//   4 The constructor works for true temporaries (e.g. a return value)
+//   2 The constructor works for explicitly moved values (i.e. std::move(x))
+//   3 The constructor works for true temporaries (e.g. a return value)
 //
 // Plan
 //  1 Explicitly construct unique_ptr<T, D> for various deleter types 'D'.
@@ -73,10 +73,22 @@ void sink3(std::unique_ptr<VT, NCDeleter<VT>&> p) {
 
 template <class ValueT>
 TEST_CONSTEXPR_CXX23 void test_sfinae() {
-  typedef std::unique_ptr<ValueT> U;
-  { // Ensure unique_ptr is non-copyable
+  // Ensure that
+  // - unique_ptr is non-copyable, and
+  // - unique_ptr's move-constructibility is correctly propagated from its deleter's.
+  {
+    typedef std::unique_ptr<ValueT> U;
     static_assert((!std::is_constructible<U, U const&>::value), "");
     static_assert((!std::is_constructible<U, U&>::value), "");
+    static_assert(std::is_move_constructible<U>::value, "");
+    static_assert(!std::is_constructible<U, const U>::value, "");
+  }
+  {
+    typedef std::unique_ptr<ValueT, NCDeleter<ValueT> > U;
+    static_assert(!std::is_constructible<U, U const&>::value, "");
+    static_assert(!std::is_constructible<U, U&>::value, "");
+    static_assert(!std::is_move_constructible<U>::value, "");
+    static_assert(!std::is_constructible<U, const U>::value, "");
   }
 }
 

libcxx/test/std/utilities/tuple/tuple.tuple/tuple.cnstr/move.pass.cpp

@@ -49,6 +49,19 @@ struct move_only_large final {
   int value;
 };
 
+// a non-movable type
+struct nonmovable {
+  nonmovable(const nonmovable&) = default;
+  nonmovable(nonmovable&&)      = delete;
+};
+
+// a non-movable type with a usable copy constructor
+// verifying that tuple's move constructor is not confused to select that copy constructor
+struct nonmovable_with_copy_ctor {
+  nonmovable_with_copy_ctor(const nonmovable_with_copy_ctor&) = default;
+  nonmovable_with_copy_ctor(nonmovable_with_copy_ctor&&)      = delete;
+};
+
 template <class Elem>
 void test_sfinae() {
     using Tup = std::tuple<Elem>;
@@ -123,6 +136,18 @@ int main(int, char**)
         test_sfinae<move_only_ebo>();
         test_sfinae<move_only_large>();
     }
+    // non-movable types
+    {
+      using Alloc = std::allocator<int>;
+      using Tag   = std::allocator_arg_t;
+
+      static_assert(!std::is_move_constructible<nonmovable>::value, "");
+      static_assert(!std::is_constructible<nonmovable, Tag, Alloc, nonmovable>::value, "");
+
+      static_assert(!std::is_move_constructible<nonmovable_with_copy_ctor>::value, "");
+      static_assert(
+          !std::is_constructible<nonmovable_with_copy_ctor, Tag, Alloc, nonmovable_with_copy_ctor>::value, "");
+    }
 
   return 0;
 }

libcxx/test/std/utilities/tuple/tuple.tuple/tuple.elem/tuple.by.type.verify.cpp

@@ -30,7 +30,7 @@ void test_bad_index() {
 void test_bad_return_type() {
     typedef std::unique_ptr<int> upint;
     std::tuple<upint> t;
-    upint p = std::get<upint>(t); // expected-error{{deleted copy constructor}}
+    upint p = std::get<upint>(t); // expected-error{{deleted constructor}}
 }
 
 void f() {

libcxx/test/std/utilities/utility/pairs/pair.astuple/pairs.by.type3.verify.cpp

@@ -14,5 +14,5 @@
 void f() {
   typedef std::unique_ptr<int> Ptr;
   std::pair<Ptr, int> t(Ptr(new int(4)), 23);
-  Ptr p = std::get<Ptr>(t); // expected-error {{call to implicitly-deleted copy constructor of 'Ptr'}}
+  Ptr p = std::get<Ptr>(t); // expected-error {{call to deleted constructor of 'Ptr'}}
 }