[libcxx] Allow shared_ptr's unique_ptr converting constructor to support array types.

[zoecarver]

Summary

Refs: https://bugs.llvm.org/show_bug.cgi?id=32147

[zoecarver]

Friendly ping.

[zoecarver]

• Add test for lvalue deleter.
• Format with clang-format.

[ldionne]

Sorry this fell through the cracks. Let's fix this!

Do we have the same problem for operator=? We should rebase and make sure CI passes - LMK if you'd like me to commandeer this.

[zoecarver]

• Remove constraint not is_array from overloads.

[zoecarver]

libcxx/include/memory

3258

Interestingly enough, updating the enable_if here wasn't required to get the tests to pass, I think because of implicit conversion to shared_ptr before the assignment happens.

[ldionne]

libcxx/test/std/utilities/memory/util.smartptr/util.smartptr.shared/util.smartptr.shared.assign/unique_ptr_Y.pass.cpp

131

This is not testing operator=, this is testing initialization!

libcxx/test/std/utilities/memory/util.smartptr/util.smartptr.shared/util.smartptr.shared.const/unique_ptr.pass.cpp

13

Hmm, this shouldn't be explicit. It looks like the implementation is OK, only this comment is wrong. Can you fix as a fly-by?

[zoecarver]

• Test the right thing!
• Remove "explicit" in ctor comment.

[ldionne]

Thanks! Ship it once CI passes. And you can then close https://llvm.org/PR32147. Thanks a lot!

[zoecarver]

For sure, will do, thanks for reviewing!

[zoecarver]

libcxx/test/std/utilities/memory/util.smartptr/util.smartptr.shared/util.smartptr.shared.const/unique_ptr.pass.cpp

181

shared_ptr<array[]> wasn't supported until C++17, so these tests shouldn't be enabled until C++17. That's why the C++14 build is failing.

[zoecarver]

• Enable array->array tests after C++14 only

[ldionne]

Still LGTM if CI passes. Thanks!

libcxx/include/memory

template<class _Yp>
         shared_ptr(auto_ptr<_Yp>&& __r,
                    typename enable_if<is_convertible<_Yp*, element_type*>::value, __nat>::type = __nat());
 #endif
     template <class _Yp, class _Dp>
         shared_ptr(unique_ptr<_Yp, _Dp>&&,
                    typename enable_if
                    <
                        !is_lvalue_reference<_Dp>::value &&
-                       !is_array<_Yp>::value &&
                        is_convertible<typename unique_ptr<_Yp, _Dp>::pointer, element_type*>::value,
                        __nat
                    >::type = __nat());
     template <class _Yp, class _Dp>
         shared_ptr(unique_ptr<_Yp, _Dp>&&,
                    typename enable_if
                    <
                        is_lvalue_reference<_Dp>::value &&
-                       !is_array<_Yp>::value &&
                        is_convertible<typename unique_ptr<_Yp, _Dp>::pointer, element_type*>::value,
                        __nat
                    >::type = __nat());
 
     ~shared_ptr();
 
     _LIBCPP_INLINE_VISIBILITY
     shared_ptr& operator=(const shared_ptr& __r) _NOEXCEPT;
template<class _Yp>
             is_convertible<_Yp*, element_type*>::value,
             shared_ptr
         >::type&
         operator=(auto_ptr<_Yp>&& __r);
 #endif
     template <class _Yp, class _Dp>
         typename enable_if
         <
-            !is_array<_Yp>::value &&
             is_convertible<typename unique_ptr<_Yp, _Dp>::pointer, element_type*>::value,
             shared_ptr&
         >::type
         _LIBCPP_INLINE_VISIBILITY
         operator=(unique_ptr<_Yp, _Dp>&& __r);
 
     _LIBCPP_INLINE_VISIBILITY
     void swap(shared_ptr& __r) _NOEXCEPT;

 #endif
 
 template<class _Tp>
 template <class _Yp, class _Dp>
 shared_ptr<_Tp>::shared_ptr(unique_ptr<_Yp, _Dp>&& __r,
                             typename enable_if
                             <
                                 !is_lvalue_reference<_Dp>::value &&
-                                !is_array<_Yp>::value &&
                                 is_convertible<typename unique_ptr<_Yp, _Dp>::pointer, element_type*>::value,
                                 __nat
                             >::type)
     : __ptr_(__r.get())
 {
 #if _LIBCPP_STD_VER > 11
     if (__ptr_ == nullptr)
         __cntrl_ = nullptr;
     else
 #endif
     {
         typedef typename __shared_ptr_default_allocator<_Yp>::type _AllocT;
-        typedef __shared_ptr_pointer<_Yp*, _Dp, _AllocT > _CntrlBlk;
+        typedef __shared_ptr_pointer<typename unique_ptr<_Yp, _Dp>::pointer, _Dp, _AllocT > _CntrlBlk;
         __cntrl_ = new _CntrlBlk(__r.get(), __r.get_deleter(), _AllocT());
         __enable_weak_this(__r.get(), __r.get());
     }
     __r.release();
 }
 
 template<class _Tp>
 template <class _Yp, class _Dp>
 shared_ptr<_Tp>::shared_ptr(unique_ptr<_Yp, _Dp>&& __r,
                             typename enable_if
                             <
                                 is_lvalue_reference<_Dp>::value &&
-                                !is_array<_Yp>::value &&
                                 is_convertible<typename unique_ptr<_Yp, _Dp>::pointer, element_type*>::value,
                                 __nat
                             >::type)
     : __ptr_(__r.get())
 {
 #if _LIBCPP_STD_VER > 11
     if (__ptr_ == nullptr)
         __cntrl_ = nullptr;
     else
 #endif
     {
         typedef typename __shared_ptr_default_allocator<_Yp>::type _AllocT;
-        typedef __shared_ptr_pointer<_Yp*,
+        typedef __shared_ptr_pointer<typename unique_ptr<_Yp, _Dp>::pointer,
                                      reference_wrapper<typename remove_reference<_Dp>::type>,
                                      _AllocT > _CntrlBlk;
         __cntrl_ = new _CntrlBlk(__r.get(), _VSTD::ref(__r.get_deleter()), _AllocT());
         __enable_weak_this(__r.get(), __r.get());
     }
     __r.release();
 }
 

 }
 #endif
 
 template<class _Tp>
 template <class _Yp, class _Dp>
 inline
 typename enable_if
 <
-    !is_array<_Yp>::value &&
     is_convertible<typename unique_ptr<_Yp, _Dp>::pointer,
                    typename shared_ptr<_Tp>::element_type*>::value,
     shared_ptr<_Tp>&
 >::type
 shared_ptr<_Tp>::operator=(unique_ptr<_Yp, _Dp>&& __r)
 {
     shared_ptr(_VSTD::move(__r)).swap(*this);
     return *this;

libcxx/test/std/utilities/memory/util.smartptr/util.smartptr.shared/util.smartptr.shared.assign/unique_ptr_Y.pass.cpp

struct A
 
     A() {++count;}
     A(const A& other) : B(other) {++count;}
     ~A() {--count;}
 };
 
 int A::count = 0;
 
+template <class T>
+struct StatefulArrayDeleter {
+  int state = 0;
+
+  StatefulArrayDeleter(int val = 0) : state(val) {}
+  StatefulArrayDeleter(StatefulArrayDeleter const&) { assert(false); }
+
+  void operator()(T* ptr) {
+    assert(state == 42);
+    delete []ptr;
+  }
+};
+
 int main(int, char**)
 {
     {
         std::unique_ptr<A> pA(new A);
         A* ptrA = pA.get();
         {
             std::shared_ptr<B> pB(new B);
             pB = std::move(pA);
//          assert(pB.use_count() == 1); // no longer true due to LWG 2415
             assert(pB.get() == ptrA);
         }
         assert(B::count == 0);
         assert(A::count == 0);
     }
     assert(B::count == 0);
     assert(A::count == 0);
 
+    {
+      std::unique_ptr<A[]> ptr(new A[8]);
+      A* raw_ptr = ptr.get();
+      std::shared_ptr<B> p;
+      p = std::move(ptr);
+      assert(A::count == 8);
+      assert(B::count == 8);
+      assert(p.use_count() == 1);
+      assert(p.get() == raw_ptr);
+      assert(ptr.get() == 0);
+    }
+    assert(A::count == 0);
+    assert(B::count == 0);
+
+    {
+      std::unique_ptr<A[]> ptr(new A[8]);
+      A* raw_ptr = ptr.get();
+      std::shared_ptr<A> p;
+      p = std::move(ptr);
+      assert(A::count == 8);
+      assert(p.use_count() == 1);
+      assert(p.get() == raw_ptr);
+      assert(ptr.get() == 0);
+    }
+    assert(A::count == 0);
+
+    {
+      std::unique_ptr<int[]> ptr(new int[8]);
+      std::shared_ptr<int> p;
+      p = std::move(ptr);
+    }
+
+#if TEST_STD_VER > 14
+    {
+      StatefulArrayDeleter<A> d;
+      std::unique_ptr<A[], StatefulArrayDeleter<A>&> u(new A[4], d);
+      std::shared_ptr<A[]> p;
+      p = std::move(u);
+      d.state = 42;
+      assert(A::count == 4);
+    }
+    assert(A::count == 0);
+    assert(B::count == 0);
+
+    {
+      std::unique_ptr<A[]> ptr(new A[8]);
+      A* raw_ptr = ptr.get();
+      std::shared_ptr<B[]> p;
+      p = std::move(ptr);
+      assert(A::count == 8);
+      assert(B::count == 8);
+      assert(p.use_count() == 1);
+      assert(p.get() == raw_ptr);
+      assert(ptr.get() == 0);
+    }
+    assert(A::count == 0);
+    assert(B::count == 0);
+
+    {
+      std::unique_ptr<A[]> ptr(new A[8]);
+      A* raw_ptr = ptr.get();
+      std::shared_ptr<A[]> p;
+      p = std::move(ptr);
+      assert(A::count == 8);
+      assert(p.use_count() == 1);
+      assert(p.get() == raw_ptr);
+      assert(ptr.get() == 0);
+    }
+    assert(A::count == 0);
+
+    {
+      std::unique_ptr<int[]> ptr(new int[8]);
+      std::shared_ptr<int[]> p;
+      p = std::move(ptr);
+    }
+#endif // TEST_STD_VER >= 14
+
   return 0;
 }

libcxx/test/std/utilities/memory/util.smartptr/util.smartptr.shared/util.smartptr.shared.const/unique_ptr.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: sanitizer-new-delete
 
 // <memory>
 
-// template <class Y, class D> explicit shared_ptr(unique_ptr<Y, D>&&r);
+// template <class Y, class D> shared_ptr(unique_ptr<Y, D>&&r);
 
 #include <memory>
 #include <new>
 #include <cstdlib>
 #include <cassert>
 
 #include "test_macros.h"
 #include "count_new.h"
struct StatefulDeleter {
   StatefulDeleter(StatefulDeleter const&) { assert(false); }
 
   void operator()(T* ptr) {
     assert(state == 42);
     delete ptr;
   }
 };
 
+template <class T>
+struct StatefulArrayDeleter {
+  int state = 0;
+
+  StatefulArrayDeleter(int val = 0) : state(val) {}
+  StatefulArrayDeleter(StatefulArrayDeleter const&) { assert(false); }
+
+  void operator()(T* ptr) {
+    assert(state == 42);
+    delete []ptr;
+  }
+};
+
 int main(int, char**)
 {
     {
         std::unique_ptr<A> ptr(new A);
         A* raw_ptr = ptr.get();
         std::shared_ptr<B> p(std::move(ptr));
         assert(A::count == 1);
         assert(B::count == 1);

 #endif
 
     {
     adl::D d;
     std::unique_ptr<int, adl::D&> u(nullptr, d);
     std::shared_ptr<int> s = std::move(u);
     }
 
+    assert(A::count == 0);
+    {
+      std::unique_ptr<A[]> ptr(new A[8]);
+      A* raw_ptr = ptr.get();
+      std::shared_ptr<B> p(std::move(ptr));
+      assert(A::count == 8);
+      assert(B::count == 8);
+      assert(p.use_count() == 1);
+      assert(p.get() == raw_ptr);
+      assert(ptr.get() == 0);
+    }
+    assert(A::count == 0);
+    assert(B::count == 0);
+
+    {
+      std::unique_ptr<A[]> ptr(new A[8]);
+      A* raw_ptr = ptr.get();
+      std::shared_ptr<A> p(std::move(ptr));
+      assert(A::count == 8);
+      assert(p.use_count() == 1);
+      assert(p.get() == raw_ptr);
+      assert(ptr.get() == 0);
+    }
+    assert(A::count == 0);
+
+    {
+      std::unique_ptr<int[]> ptr(new int[8]);
+      std::shared_ptr<int> p(std::move(ptr));
+    }
+
+#if TEST_STD_VER > 14
+    {
+      StatefulArrayDeleter<A> d;
+      std::unique_ptr<A[], StatefulArrayDeleter<A>&> u(new A[4], d);
+      std::shared_ptr<A[]> p(std::move(u));
+      d.state = 42;
+      assert(A::count == 4);
+    }
+    assert(A::count == 0);
+    assert(B::count == 0);
+
+    {
+      std::unique_ptr<A[]> ptr(new A[8]);
+      A* raw_ptr = ptr.get();
+      std::shared_ptr<B[]> p(std::move(ptr));
+      assert(A::count == 8);
+      assert(B::count == 8);
+      assert(p.use_count() == 1);
+      assert(p.get() == raw_ptr);
+      assert(ptr.get() == 0);
+    }
+    assert(A::count == 0);
+    assert(B::count == 0);
+
+    {
+      std::unique_ptr<A[]> ptr(new A[8]);
+      A* raw_ptr = ptr.get();
+      std::shared_ptr<A[]> p(std::move(ptr));
+      assert(A::count == 8);
+      assert(p.use_count() == 1);
+      assert(p.get() == raw_ptr);
+      assert(ptr.get() == 0);
+    }
+    assert(A::count == 0);
+
+    {
+      std::unique_ptr<int[]> ptr(new int[8]);
+      std::shared_ptr<int[]> p(std::move(ptr));
+    }
+#endif // TEST_STD_VER >= 14
+
     return 0;
 }