General shared_ptr cleanup

[zoecarver]

Summary

This patch applies some general cleanup to shared_ptr and its implementation structures:

• stop using compressed_pair to store objects.
• implement __allocate_shared to reduce duplicate code.
• remove _LIBCPP_HAS_NO_VARIADICS. I don't think it is necessary, but I can add it back if it is.

[EricWF]

include/memory

3665

Changing the __compressed_pair to these two separate members changes the layout of __shared_ptr_emplace and in turn breaks the ABI.

4008

You're deleting a nullptr here. IDK if that makes sense.

[mclow.lists]

stop using compressed_pair to store objects.

Besides breaking the ABI, what's the motivation to stop using compressed_pair?
Many allocators are "state-free", and there's no need to actually *store* one of them.
That's the purpose of compressed_pair, to not store objects that have no state.

given two types T and A, where A has no non-static data members (and no virtual functions), we have

sizeof(std::pair<T, A>) == sizeof(T) + sizeof(A) + possible padding.
sizeof(compressed_pair<T,A>) == sizeof(T)

[zoecarver]

Thanks for the explanation @mclow.lists I didn't fully understand the importance of using compressed_pair (or that removing it would cause an ABI break). I don't have a "good" reason for removing compressed_pair other than, I got tired of typing out .second().second() and remembering which child class uses which order of elements (which is not a good reason). I will remove that change.

[zoecarver]

• remove changes to control block around compressed_pair

[mclow.lists]

remove _LIBCPP_HAS_NO_VARIADICS. I don't think it is necessary, but I can add it back if it is.

Are you running the tests with -std=c++03?, because that's where _LIBCPP_HAS_NO_VARIADICS gets set.

[zoecarver]

I thought shared_ptr was not introduced until C++11?

[mclow.lists]

In D62233#1512217, @zoecarver wrote:

I thought shared_ptr was not introduced until C++11?

Libc++ is a c++11 library; it provides many c++11 features when compiling for C++03.

[zoecarver]

• re-add functions for _LIBCPP_HAS_NO_VARIADICS

[zoecarver]

• remove static member functions shared_ptr::make_shared and shared_ptr::allocate_shared (and move their contents to the correct place).

[zoecarver]

• fix typedef order (sorry for repetitive updates).

[zoecarver]

• fix, so it works in c++03

[zoecarver]

include/memory

4008

If it matters, I can change it, but I don't think it should. If, for example, a shared_ptr to a nullptr went out of scope, the same thing would happen, so people's deleters should be able to handle this (especially if they are ever passing it a nullptr).

[zoecarver]

Friendly ping @mclow.lists @EricWF and @ldionne. I would very much like to get this committed so that I can update my following patches. I know @EricWF has some patches that may conflict with this, I am happy to update based on those or help update those based on this. Regardless, getting this committed is a very high priority for me.

[ldionne]

Could you please split this patch into one that removes the variadic stuff, and one that does the rest (so we can look at them separately). I think we can get rid of the variadic stuff because of Clang extensions, but it's not clear to me the rest is worth doing.

[zoecarver]

In D62233#1627092, @ldionne wrote:

Could you please split this patch into one that removes the variadic stuff, and one that does the rest (so we can look at them separately). I think we can get rid of the variadic stuff because of Clang extensions, but it's not clear to me the rest is worth doing.

@EricWF is going to remove the variadics here. The code I removed here is removing shared_ptr::make_shared and shared_ptr::allocate_shared. Those two static member functions should not exist and are not part of the standard. Would it be helpful for me to break this patch down further into smaller patches?

[ldionne]

This patch touches a lot of code, and the code it touches is very sensitive (std::shared_ptr is used literally everywhere). That's why I like being able to easily verify its correctness, which reducing the diff or just moving code around (without modifying it -- as much as possible) will make easier in this case.

include/memory

3666

I don't see the point in moving those functions around. I would leave them where they were to reduce the diff unless there's a reason to move them around (in which case please just let me know).

3794

Since you're deducing _Dp& now, this needs to be is_convertible<typename unique_ptr<_Yp, _Dp&>::pointer.

I *think* the deduction like what you wrote works, however the fact that we've been using is_lvalue_reference<_Dp> previously makes me suspicious that I'm missing something subtle. If the tests pass, though, I would say this is OK.

4322

I'd be more comfortable if you inlined the std::shared_ptr::make_shareds verbatim into their respective callers (std::make_shared). But that would require friendship, and that's the purpose of std::shared_ptr::__create_with_cntrl_block being a static member function, right?

4358

There's a lot of stuff going on in this patch. The removal of variadics below could be split off in a separate patch, that's what I meant in my previous comment.

[zoecarver]

That is a completely valid concern. I will try to focus this patch more.

include/memory

3666

Just for readability. We used to have:

// code
public:
//code
private:
//code
public:
//code

Now it is:

private:
//code
public:
//code

I will revert.

3794

Good catch. I will test with _LIBCPP_HAS_NO_RVALUE_REFERENCES.

4322

Yes. While having these functions be private would be better than what we currently have, I would much prefer not to use friendship and keep all this duplicate code. The main purpose of __create_with_cntrl_block is to remove duplicate code. Going forward it will be much easier to update all these functions at once (though, hopefully, we will remove these variadics soon).

4358

I am happy to move __create_with_cntrl_block and all changes to these functions into another patch. Just say the word. I want to confirm we are the same page first.

[zoecarver]

I am closing this patch in favor of D66177 and D66178. I may submit another patch for the remaining changes, but that is a much lower priority. I am leaving this patch intact for posterity.

include/memory

 private:
     virtual void __on_zero_shared_weak() _NOEXCEPT = 0;
 };
 
 template <class _Tp, class _Dp, class _Alloc>
 class __shared_ptr_pointer
     : public __shared_weak_count
 {
-    __compressed_pair<__compressed_pair<_Tp, _Dp>, _Alloc> __data_;
+    _Tp __pointee;
+    _Dp __deleter;
+    _Alloc __alloc;
+
 public:
     _LIBCPP_INLINE_VISIBILITY
     __shared_ptr_pointer(_Tp __p, _Dp __d, _Alloc __a)
-        :  __data_(__compressed_pair<_Tp, _Dp>(__p, _VSTD::move(__d)), _VSTD::move(__a)) {}
+        :  __pointee(__p),
+           __deleter(_VSTD::move(__d)),
+           __alloc(_VSTD::move(__a)) {}
 
 #ifndef _LIBCPP_NO_RTTI
     virtual const void* __get_deleter(const type_info&) const _NOEXCEPT;
 #endif
 
 private:
     virtual void __on_zero_shared() _NOEXCEPT;
     virtual void __on_zero_shared_weak() _NOEXCEPT;
 };
 
 #ifndef _LIBCPP_NO_RTTI
 
 template <class _Tp, class _Dp, class _Alloc>
 const void*
 __shared_ptr_pointer<_Tp, _Dp, _Alloc>::__get_deleter(const type_info& __t) const _NOEXCEPT
 {
-    return __t == typeid(_Dp) ? _VSTD::addressof(__data_.first().second()) : nullptr;
+    return __t == typeid(_Dp) ? _VSTD::addressof(__deleter) : nullptr;
 }
 
 #endif  // _LIBCPP_NO_RTTI
 
 template <class _Tp, class _Dp, class _Alloc>
 void
 __shared_ptr_pointer<_Tp, _Dp, _Alloc>::__on_zero_shared() _NOEXCEPT
 {
-    __data_.first().second()(__data_.first().first());
-    __data_.first().second().~_Dp();
+    __deleter(__pointee);
+    __deleter.~_Dp();
 }
 
 template <class _Tp, class _Dp, class _Alloc>
 void
 __shared_ptr_pointer<_Tp, _Dp, _Alloc>::__on_zero_shared_weak() _NOEXCEPT
 {
     typedef typename __allocator_traits_rebind<_Alloc, __shared_ptr_pointer>::type _Al;
     typedef allocator_traits<_Al> _ATraits;
     typedef pointer_traits<typename _ATraits::pointer> _PTraits;
 
-    _Al __a(__data_.second());
-    __data_.second().~_Alloc();
+    _Al __a(__alloc);
+    __alloc.~_Alloc();
     __a.deallocate(_PTraits::pointer_to(*this), 1);
 }
 
 template <class _Tp, class _Alloc>
 class __shared_ptr_emplace
     : public __shared_weak_count
 {
-    __compressed_pair<_Alloc, _Tp> __data_;
-public:
-#ifndef _LIBCPP_HAS_NO_VARIADICS
+    _Tp __pointee;
+    _Alloc __alloc;
+
+    virtual void __on_zero_shared() _NOEXCEPT;
+    virtual void __on_zero_shared_weak() _NOEXCEPT;
 
+public:
     _LIBCPP_INLINE_VISIBILITY
     __shared_ptr_emplace(_Alloc __a)
-        :  __data_(_VSTD::move(__a)) {}
+        :  __alloc(_VSTD::move(__a)) {}
 
     template <class ..._Args>
         _LIBCPP_INLINE_VISIBILITY
         __shared_ptr_emplace(_Alloc __a, _Args&& ...__args)
-            :  __data_(piecewise_construct, _VSTD::forward_as_tuple(__a),
-                   _VSTD::forward_as_tuple(_VSTD::forward<_Args>(__args)...)) {}
-
-#else  // _LIBCPP_HAS_NO_VARIADICS
-
-    _LIBCPP_INLINE_VISIBILITY
-    __shared_ptr_emplace(_Alloc __a)
-        :  __data_(__a) {}
-
-    template <class _A0>
-        _LIBCPP_INLINE_VISIBILITY
-        __shared_ptr_emplace(_Alloc __a, _A0& __a0)
-            :  __data_(__a, _Tp(__a0)) {}
-
-    template <class _A0, class _A1>
-        _LIBCPP_INLINE_VISIBILITY
-        __shared_ptr_emplace(_Alloc __a, _A0& __a0, _A1& __a1)
-            :  __data_(__a, _Tp(__a0, __a1)) {}
-
-    template <class _A0, class _A1, class _A2>
-        _LIBCPP_INLINE_VISIBILITY
-        __shared_ptr_emplace(_Alloc __a, _A0& __a0, _A1& __a1, _A2& __a2)
-            :  __data_(__a, _Tp(__a0, __a1, __a2)) {}
-
-#endif  // _LIBCPP_HAS_NO_VARIADICS
+            :   __pointee(_Tp(_VSTD::forward<_Args>(__args)...)),
+                __alloc(_VSTD::move(__a)) {}
 
-private:
-    virtual void __on_zero_shared() _NOEXCEPT;
-    virtual void __on_zero_shared_weak() _NOEXCEPT;
-public:
     _LIBCPP_INLINE_VISIBILITY
-    _Tp* get() _NOEXCEPT {return _VSTD::addressof(__data_.second());}
+    _Tp* get() _NOEXCEPT {return _VSTD::addressof(__pointee);}
 };
 
 template <class _Tp, class _Alloc>
 void
 __shared_ptr_emplace<_Tp, _Alloc>::__on_zero_shared() _NOEXCEPT
 {
-    __data_.second().~_Tp();
+    __pointee.~_Tp();
 }
 
 template <class _Tp, class _Alloc>
 void
 __shared_ptr_emplace<_Tp, _Alloc>::__on_zero_shared_weak() _NOEXCEPT
 {
     typedef typename __allocator_traits_rebind<_Alloc, __shared_ptr_emplace>::type _Al;
     typedef allocator_traits<_Al> _ATraits;
     typedef pointer_traits<typename _ATraits::pointer> _PTraits;
-    _Al __a(__data_.first());
-    __data_.first().~_Alloc();
+    _Al __a(__alloc);
+    __alloc.~_Alloc();
     __a.deallocate(_PTraits::pointer_to(*this), 1);
 }
 
 struct __shared_ptr_dummy_rebind_allocator_type;
 template <>
 class _LIBCPP_TEMPLATE_VIS allocator<__shared_ptr_dummy_rebind_allocator_type>
 {
 public:
     template <class _Other>
     struct rebind
     {
         typedef allocator<_Other> other;
     };
 };
 
 template<class _Tp> class _LIBCPP_TEMPLATE_VIS enable_shared_from_this;
 
 template<class _Tp>
 class _LIBCPP_TEMPLATE_VIS shared_ptr
 {
-public:
-    typedef _Tp element_type;
-
-#if _LIBCPP_STD_VER > 14
-    typedef weak_ptr<_Tp> weak_type;
-#endif
 private:
-    element_type*      __ptr_;
+    _Tp*                 __ptr_;
     __shared_weak_count* __cntrl_;
 
     struct __nat {int __for_bool_;};
 public:
+#if _LIBCPP_STD_VER > 14
+    typedef weak_ptr<_Tp> weak_type;
+#endif
+
+    typedef _Tp element_type;
+
     _LIBCPP_INLINE_VISIBILITY
     _LIBCPP_CONSTEXPR shared_ptr() _NOEXCEPT;
     _LIBCPP_INLINE_VISIBILITY
     _LIBCPP_CONSTEXPR shared_ptr(nullptr_t) _NOEXCEPT;
     template<class _Yp>
         explicit shared_ptr(_Yp* __p,
                             typename enable_if<is_convertible<_Yp*, element_type*>::value, __nat>::type = __nat());
     template<class _Yp, class _Dp>
template<class _Yp>
         shared_ptr(auto_ptr<_Yp>&& __r,
                    typename enable_if<is_convertible<_Yp*, element_type*>::value, __nat>::type = __nat());
 #else
     template<class _Yp>
         shared_ptr(auto_ptr<_Yp> __r,
                    typename enable_if<is_convertible<_Yp*, element_type*>::value, __nat>::type = __nat());
 #endif
 #endif
-#ifndef _LIBCPP_HAS_NO_RVALUE_REFERENCES
-    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());
-#else  // _LIBCPP_HAS_NO_RVALUE_REFERENCES
-    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(unique_ptr<_Yp, _Dp>&& __r,
+            is_convertible<typename unique_ptr<_Yp, _Dp>::pointer,
+                                element_type*>::value,
     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());
-#endif  // _LIBCPP_HAS_NO_RVALUE_REFERENCES
+    shared_ptr(unique_ptr<_Yp, _Dp&>&& __r,
+            is_convertible<typename unique_ptr<_Yp, _Dp>::pointer,
+                                element_type*>::value,
 
     ~shared_ptr();
 
     _LIBCPP_INLINE_VISIBILITY
     shared_ptr& operator=(const shared_ptr& __r) _NOEXCEPT;
     template<class _Yp>
         typename enable_if
         <
#ifndef _LIBCPP_NO_RTTI
     template <class _Dp>
         _LIBCPP_INLINE_VISIBILITY
         _Dp* __get_deleter() const _NOEXCEPT
             {return static_cast<_Dp*>(__cntrl_
                     ? const_cast<void *>(__cntrl_->__get_deleter(typeid(_Dp)))
                       : nullptr);}
 #endif  // _LIBCPP_NO_RTTI
 
-#ifndef _LIBCPP_HAS_NO_VARIADICS
-
     template<class ..._Args>
         static
         shared_ptr<_Tp>
         make_shared(_Args&& ...__args);
 
     template<class _Alloc, class ..._Args>
         static
         shared_ptr<_Tp>
         allocate_shared(const _Alloc& __a, _Args&& ...__args);
 
-#else  // _LIBCPP_HAS_NO_VARIADICS
-
-    static shared_ptr<_Tp> make_shared();
-
-    template<class _A0>
-        static shared_ptr<_Tp> make_shared(_A0&);
-
-    template<class _A0, class _A1>
-        static shared_ptr<_Tp> make_shared(_A0&, _A1&);
-
-    template<class _A0, class _A1, class _A2>
-        static shared_ptr<_Tp> make_shared(_A0&, _A1&, _A2&);
-
-    template<class _Alloc>
-        static shared_ptr<_Tp>
-        allocate_shared(const _Alloc& __a);
-
-    template<class _Alloc, class _A0>
-        static shared_ptr<_Tp>
-        allocate_shared(const _Alloc& __a, _A0& __a0);
-
-    template<class _Alloc, class _A0, class _A1>
-        static shared_ptr<_Tp>
-        allocate_shared(const _Alloc& __a, _A0& __a0, _A1& __a1);
-
-    template<class _Alloc, class _A0, class _A1, class _A2>
-        static shared_ptr<_Tp>
-        allocate_shared(const _Alloc& __a, _A0& __a0, _A1& __a1, _A2& __a2);
-
-#endif  // _LIBCPP_HAS_NO_VARIADICS
-
 private:
     template <class _Yp, bool = is_function<_Yp>::value>
         struct __shared_ptr_default_allocator
         {
             typedef allocator<_Yp> type;
         };
 
     template <class _Yp>
template <class _Yp, class _OrigPtr>
                     const_cast<_RawYp*>(static_cast<const _Yp*>(__ptr)));
             }
         }
 
     _LIBCPP_INLINE_VISIBILITY void __enable_weak_this(...) _NOEXCEPT {}
 
     template <class _Up> friend class _LIBCPP_TEMPLATE_VIS shared_ptr;
     template <class _Up> friend class _LIBCPP_TEMPLATE_VIS weak_ptr;
+
+    template<class _Yp, class _Deleter = default_delete<_Yp>,
+                        class _Alloc = typename __shared_ptr_default_allocator<_Yp>::type>
+    static __shared_weak_count*
+    __allocate_shared(_Yp* __p, _Deleter __d = _Deleter {}, _Alloc __a = _Alloc {})
+    {
+        try {
+            typedef __shared_ptr_pointer<_Yp*, _Deleter, _Alloc> _CT;
+            typedef typename __allocator_traits_rebind<_Alloc, _CT>::type _A2;
+            typedef __allocator_destructor<_A2> _D2;
+
+            _A2 __a2(__a);
+            unique_ptr<_CT, _D2> __hold2(__a2.allocate(1), _D2(__a2, 1));
+
+            ::new(static_cast<void*>(_VSTD::addressof(*__hold2.get())))
+                _CT(__p, __d, __a);
+
+            return _VSTD::addressof(*__hold2.release());
+        } catch (...) {
+            __d(__p);
+            throw;
+        }
+    }
+
+    template<class _Deleter = default_delete<_Tp>,
+             class _Alloc = typename __shared_ptr_default_allocator<_Tp>::type>
+    static __shared_weak_count*
+    __allocate_shared(nullptr_t __p, _Deleter __d = _Deleter {}, _Alloc __a = _Alloc {})
+    {
+        try {
+            typedef __shared_ptr_pointer<_Tp*, _Deleter, _Alloc> _CT;
+            typedef typename __allocator_traits_rebind<_Alloc, _CT>::type _A2;
+            typedef __allocator_destructor<_A2> _D2;
+
+            _A2 __a2(__a);
+            unique_ptr<_CT, _D2> __hold2(__a2.allocate(1), _D2(__a2, 1));
+
+            ::new(static_cast<void*>(_VSTD::addressof(*__hold2.get())))
+                _CT(__p, __d, __a);
+
+            return _VSTD::addressof(*__hold2.release());
+        } catch (...) {
+            __d(__p);
+            throw;
+        }
+    }
 };
 
 
 template<class _Tp>
 inline
 _LIBCPP_CONSTEXPR
 shared_ptr<_Tp>::shared_ptr() _NOEXCEPT
     : __ptr_(0),
-      __cntrl_(0)
-{
-}
+      __cntrl_(0) {}
 
 template<class _Tp>
 inline
 _LIBCPP_CONSTEXPR
 shared_ptr<_Tp>::shared_ptr(nullptr_t) _NOEXCEPT
     : __ptr_(0),
-      __cntrl_(0)
-{
-}
+      __cntrl_(0) {}
 
 template<class _Tp>
 template<class _Yp>
 shared_ptr<_Tp>::shared_ptr(_Yp* __p,
                             typename enable_if<is_convertible<_Yp*, element_type*>::value, __nat>::type)
-    : __ptr_(__p)
+    : __ptr_(__p),
+      __cntrl_(__allocate_shared(__p))
 {
-    unique_ptr<_Yp> __hold(__p);
-    typedef typename __shared_ptr_default_allocator<_Yp>::type _AllocT;
-    typedef __shared_ptr_pointer<_Yp*, default_delete<_Yp>, _AllocT > _CntrlBlk;
-    __cntrl_ = new _CntrlBlk(__p, default_delete<_Yp>(), _AllocT());
-    __hold.release();
     __enable_weak_this(__p, __p);
 }
 
 template<class _Tp>
 template<class _Yp, class _Dp>
 shared_ptr<_Tp>::shared_ptr(_Yp* __p, _Dp __d,
                             typename enable_if<is_convertible<_Yp*, element_type*>::value, __nat>::type)
-    : __ptr_(__p)
-{
-#ifndef _LIBCPP_NO_EXCEPTIONS
-    try
-    {
-#endif  // _LIBCPP_NO_EXCEPTIONS
-        typedef typename __shared_ptr_default_allocator<_Yp>::type _AllocT;
-        typedef __shared_ptr_pointer<_Yp*, _Dp, _AllocT > _CntrlBlk;
-        __cntrl_ = new _CntrlBlk(__p, __d, _AllocT());
-        __enable_weak_this(__p, __p);
-#ifndef _LIBCPP_NO_EXCEPTIONS
-    }
-    catch (...)
-    {
-        __d(__p);
-        throw;
-    }
-#endif  // _LIBCPP_NO_EXCEPTIONS
+    : __ptr_(__p),
+      __cntrl_(__allocate_shared(__p, __d))
 }
 
 template<class _Tp>
 template<class _Dp>
 shared_ptr<_Tp>::shared_ptr(nullptr_t __p, _Dp __d)
-    : __ptr_(0)
-{
-#ifndef _LIBCPP_NO_EXCEPTIONS
-    try
-    {
-#endif  // _LIBCPP_NO_EXCEPTIONS
-        typedef typename __shared_ptr_default_allocator<_Tp>::type _AllocT;
-        typedef __shared_ptr_pointer<nullptr_t, _Dp, _AllocT > _CntrlBlk;
-        __cntrl_ = new _CntrlBlk(__p, __d, _AllocT());
-#ifndef _LIBCPP_NO_EXCEPTIONS
-    }
-    catch (...)
-    {
-        __d(__p);
-        throw;
-    }
-#endif  // _LIBCPP_NO_EXCEPTIONS
-}
+    : __ptr_(0),
+      __cntrl_(__allocate_shared(__p, __d)) {}
 
 template<class _Tp>
 template<class _Yp, class _Dp, class _Alloc>
 shared_ptr<_Tp>::shared_ptr(_Yp* __p, _Dp __d, _Alloc __a,
                             typename enable_if<is_convertible<_Yp*, element_type*>::value, __nat>::type)
-    : __ptr_(__p)
-{
-#ifndef _LIBCPP_NO_EXCEPTIONS
-    try
-    {
-#endif  // _LIBCPP_NO_EXCEPTIONS
-        typedef __shared_ptr_pointer<_Yp*, _Dp, _Alloc> _CntrlBlk;
-        typedef typename __allocator_traits_rebind<_Alloc, _CntrlBlk>::type _A2;
-        typedef __allocator_destructor<_A2> _D2;
-        _A2 __a2(__a);
-        unique_ptr<_CntrlBlk, _D2> __hold2(__a2.allocate(1), _D2(__a2, 1));
-        ::new(static_cast<void*>(_VSTD::addressof(*__hold2.get())))
-            _CntrlBlk(__p, __d, __a);
-        __cntrl_ = _VSTD::addressof(*__hold2.release());
-        __enable_weak_this(__p, __p);
-#ifndef _LIBCPP_NO_EXCEPTIONS
-    }
-    catch (...)
-    {
-        __d(__p);
-        throw;
-    }
-#endif  // _LIBCPP_NO_EXCEPTIONS
+    : __ptr_(__p),
+      __cntrl_(__allocate_shared(__p, __d, __a))
 }
 
 template<class _Tp>
 template<class _Dp, class _Alloc>
 shared_ptr<_Tp>::shared_ptr(nullptr_t __p, _Dp __d, _Alloc __a)
-    : __ptr_(0)
-{
-#ifndef _LIBCPP_NO_EXCEPTIONS
-    try
-    {
-#endif  // _LIBCPP_NO_EXCEPTIONS
-        typedef __shared_ptr_pointer<nullptr_t, _Dp, _Alloc> _CntrlBlk;
-        typedef typename __allocator_traits_rebind<_Alloc, _CntrlBlk>::type _A2;
-        typedef __allocator_destructor<_A2> _D2;
-        _A2 __a2(__a);
-        unique_ptr<_CntrlBlk, _D2> __hold2(__a2.allocate(1), _D2(__a2, 1));
-        ::new(static_cast<void*>(_VSTD::addressof(*__hold2.get())))
-            _CntrlBlk(__p, __d, __a);
-        __cntrl_ = _VSTD::addressof(*__hold2.release());
-#ifndef _LIBCPP_NO_EXCEPTIONS
-    }
-    catch (...)
-    {
-        __d(__p);
-        throw;
-    }
-#endif  // _LIBCPP_NO_EXCEPTIONS
-}
+    : __ptr_(0),
+      __cntrl_(__allocate_shared(__p, __d, __a)) {}
 
 template<class _Tp>
 template<class _Yp>
 inline
 shared_ptr<_Tp>::shared_ptr(const shared_ptr<_Yp>& __r, element_type *__p) _NOEXCEPT
     : __ptr_(__p),
       __cntrl_(__r.__cntrl_)
 {
#endif
     __cntrl_ = new _CntrlBlk(__r.get(), default_delete<_Yp>(), allocator<_Yp>());
     __enable_weak_this(__r.get(), __r.get());
     __r.release();
 }
 #endif
 
 template<class _Tp>
 template <class _Yp, class _Dp>
-#ifndef _LIBCPP_HAS_NO_RVALUE_REFERENCES
 shared_ptr<_Tp>::shared_ptr(unique_ptr<_Yp, _Dp>&& __r,
-#else
-shared_ptr<_Tp>::shared_ptr(unique_ptr<_Yp, _Dp> __r,
-#endif
                             typename enable_if
                             <
-                                !is_lvalue_reference<_Dp>::value &&
                                 !is_array<_Yp>::value &&
-                                is_convertible<typename unique_ptr<_Yp, _Dp>::pointer, element_type*>::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;
-        __cntrl_ = new _CntrlBlk(__r.get(), __r.get_deleter(), _AllocT());
+        typedef __shared_ptr_pointer<_Yp*,
+                                     reference_wrapper<typename remove_reference<_Dp>::type>,
+                                     _AllocT > _CntrlBlk;
+        __cntrl_ = new _CntrlBlk(__r.get(), ref(__r.get_deleter()), _AllocT());
         __enable_weak_this(__r.get(), __r.get());
     }
     __r.release();
 }
 
 template<class _Tp>
 template <class _Yp, class _Dp>
-#ifndef _LIBCPP_HAS_NO_RVALUE_REFERENCES
-shared_ptr<_Tp>::shared_ptr(unique_ptr<_Yp, _Dp>&& __r,
-#else
-shared_ptr<_Tp>::shared_ptr(unique_ptr<_Yp, _Dp> __r,
-#endif
+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,
+                                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*,
-                                     reference_wrapper<typename remove_reference<_Dp>::type>,
-                                     _AllocT > _CntrlBlk;
-        __cntrl_ = new _CntrlBlk(__r.get(), ref(__r.get_deleter()), _AllocT());
+        typedef __shared_ptr_pointer<_Yp*, _Dp, _AllocT > _CntrlBlk;
+        __cntrl_ = new _CntrlBlk(__r.get(), __r.get_deleter(), _AllocT());
         __enable_weak_this(__r.get(), __r.get());
     }
     __r.release();
 }
 
-#ifndef _LIBCPP_HAS_NO_VARIADICS
-
 template<class _Tp>
 template<class ..._Args>
 shared_ptr<_Tp>
 shared_ptr<_Tp>::make_shared(_Args&& ...__args)
 {
     static_assert( is_constructible<_Tp, _Args...>::value, "Can't construct object in make_shared" );
     typedef __shared_ptr_emplace<_Tp, allocator<_Tp> > _CntrlBlk;
     typedef allocator<_CntrlBlk> _A2;
     typedef __allocator_destructor<_A2> _D2;
+
     _A2 __a2;
     unique_ptr<_CntrlBlk, _D2> __hold2(__a2.allocate(1), _D2(__a2, 1));
     ::new(__hold2.get()) _CntrlBlk(__a2, _VSTD::forward<_Args>(__args)...);
+
     shared_ptr<_Tp> __r;
     __r.__ptr_ = __hold2.get()->get();
     __r.__cntrl_ = __hold2.release();
     __r.__enable_weak_this(__r.__ptr_, __r.__ptr_);
     return __r;
 }
 
 template<class _Tp>
 template<class _Alloc, class ..._Args>
 shared_ptr<_Tp>
 shared_ptr<_Tp>::allocate_shared(const _Alloc& __a, _Args&& ...__args)
 {
-    static_assert( is_constructible<_Tp, _Args...>::value, "Can't construct object in allocate_shared" );
+    static_assert(is_constructible<_Tp, _Args...>::value, "Can't construct object in allocate_shared");
+
     typedef __shared_ptr_emplace<_Tp, _Alloc> _CntrlBlk;
     typedef typename __allocator_traits_rebind<_Alloc, _CntrlBlk>::type _A2;
     typedef __allocator_destructor<_A2> _D2;
+
     _A2 __a2(__a);
     unique_ptr<_CntrlBlk, _D2> __hold2(__a2.allocate(1), _D2(__a2, 1));
     ::new(static_cast<void*>(_VSTD::addressof(*__hold2.get())))
         _CntrlBlk(__a, _VSTD::forward<_Args>(__args)...);
-    shared_ptr<_Tp> __r;
-    __r.__ptr_ = __hold2.get()->get();
-    __r.__cntrl_ = _VSTD::addressof(*__hold2.release());
-    __r.__enable_weak_this(__r.__ptr_, __r.__ptr_);
-    return __r;
-}
 
-#else  // _LIBCPP_HAS_NO_VARIADICS
-
-template<class _Tp>
-shared_ptr<_Tp>
-shared_ptr<_Tp>::make_shared()
-{
-    static_assert((is_constructible<_Tp>::value), "Can't construct object in make_shared" );
-    typedef __shared_ptr_emplace<_Tp, allocator<_Tp> > _CntrlBlk;
-    typedef allocator<_CntrlBlk> _Alloc2;
-    typedef __allocator_destructor<_Alloc2> _D2;
-    _Alloc2 __alloc2;
-    unique_ptr<_CntrlBlk, _D2> __hold2(__alloc2.allocate(1), _D2(__alloc2, 1));
-    ::new(__hold2.get()) _CntrlBlk(__alloc2);
-    shared_ptr<_Tp> __r;
-    __r.__ptr_ = __hold2.get()->get();
-    __r.__cntrl_ = __hold2.release();
-    __r.__enable_weak_this(__r.__ptr_, __r.__ptr_);
-    return __r;
-}
-
-template<class _Tp>
-template<class _A0>
-shared_ptr<_Tp>
-shared_ptr<_Tp>::make_shared(_A0& __a0)
-{
-    static_assert((is_constructible<_Tp, _A0>::value), "Can't construct object in make_shared" );
-    typedef __shared_ptr_emplace<_Tp, allocator<_Tp> > _CntrlBlk;
-    typedef allocator<_CntrlBlk> _Alloc2;
-    typedef __allocator_destructor<_Alloc2> _D2;
-    _Alloc2 __alloc2;
-    unique_ptr<_CntrlBlk, _D2> __hold2(__alloc2.allocate(1), _D2(__alloc2, 1));
-    ::new(__hold2.get()) _CntrlBlk(__alloc2, __a0);
-    shared_ptr<_Tp> __r;
-    __r.__ptr_ = __hold2.get()->get();
-    __r.__cntrl_ = __hold2.release();
-    __r.__enable_weak_this(__r.__ptr_, __r.__ptr_);
-    return __r;
-}
-
-template<class _Tp>
-template<class _A0, class _A1>
-shared_ptr<_Tp>
-shared_ptr<_Tp>::make_shared(_A0& __a0, _A1& __a1)
-{
-    static_assert((is_constructible<_Tp, _A0, _A1>::value), "Can't construct object in make_shared" );
-    typedef __shared_ptr_emplace<_Tp, allocator<_Tp> > _CntrlBlk;
-    typedef allocator<_CntrlBlk> _Alloc2;
-    typedef __allocator_destructor<_Alloc2> _D2;
-    _Alloc2 __alloc2;
-    unique_ptr<_CntrlBlk, _D2> __hold2(__alloc2.allocate(1), _D2(__alloc2, 1));
-    ::new(__hold2.get()) _CntrlBlk(__alloc2, __a0, __a1);
-    shared_ptr<_Tp> __r;
-    __r.__ptr_ = __hold2.get()->get();
-    __r.__cntrl_ = __hold2.release();
-    __r.__enable_weak_this(__r.__ptr_, __r.__ptr_);
-    return __r;
-}
-
-template<class _Tp>
-template<class _A0, class _A1, class _A2>
-shared_ptr<_Tp>
-shared_ptr<_Tp>::make_shared(_A0& __a0, _A1& __a1, _A2& __a2)
-{
-    static_assert((is_constructible<_Tp, _A0, _A1, _A2>::value), "Can't construct object in make_shared" );
-    typedef __shared_ptr_emplace<_Tp, allocator<_Tp> > _CntrlBlk;
-    typedef allocator<_CntrlBlk> _Alloc2;
-    typedef __allocator_destructor<_Alloc2> _D2;
-    _Alloc2 __alloc2;
-    unique_ptr<_CntrlBlk, _D2> __hold2(__alloc2.allocate(1), _D2(__alloc2, 1));
-    ::new(__hold2.get()) _CntrlBlk(__alloc2, __a0, __a1, __a2);
-    shared_ptr<_Tp> __r;
-    __r.__ptr_ = __hold2.get()->get();
-    __r.__cntrl_ = __hold2.release();
-    __r.__enable_weak_this(__r.__ptr_, __r.__ptr_);
-    return __r;
-}
-
-template<class _Tp>
-template<class _Alloc>
-shared_ptr<_Tp>
-shared_ptr<_Tp>::allocate_shared(const _Alloc& __a)
-{
-    static_assert((is_constructible<_Tp>::value), "Can't construct object in allocate_shared" );
-    typedef __shared_ptr_emplace<_Tp, _Alloc> _CntrlBlk;
-    typedef typename __allocator_traits_rebind<_Alloc, _CntrlBlk>::type _Alloc2;
-    typedef __allocator_destructor<_Alloc2> _D2;
-    _Alloc2 __alloc2(__a);
-    unique_ptr<_CntrlBlk, _D2> __hold2(__alloc2.allocate(1), _D2(__alloc2, 1));
-    ::new(static_cast<void*>(_VSTD::addressof(*__hold2.get())))
-        _CntrlBlk(__a);
-    shared_ptr<_Tp> __r;
-    __r.__ptr_ = __hold2.get()->get();
-    __r.__cntrl_ = _VSTD::addressof(*__hold2.release());
-    __r.__enable_weak_this(__r.__ptr_, __r.__ptr_);
-    return __r;
-}
-
-template<class _Tp>
-template<class _Alloc, class _A0>
-shared_ptr<_Tp>
-shared_ptr<_Tp>::allocate_shared(const _Alloc& __a, _A0& __a0)
-{
-    static_assert((is_constructible<_Tp, _A0>::value), "Can't construct object in allocate_shared" );
-    typedef __shared_ptr_emplace<_Tp, _Alloc> _CntrlBlk;
-    typedef typename __allocator_traits_rebind<_Alloc, _CntrlBlk>::type _Alloc2;
-    typedef __allocator_destructor<_Alloc2> _D2;
-    _Alloc2 __alloc2(__a);
-    unique_ptr<_CntrlBlk, _D2> __hold2(__alloc2.allocate(1), _D2(__alloc2, 1));
-    ::new(static_cast<void*>(_VSTD::addressof(*__hold2.get())))
-        _CntrlBlk(__a, __a0);
     shared_ptr<_Tp> __r;
     __r.__ptr_ = __hold2.get()->get();
     __r.__cntrl_ = _VSTD::addressof(*__hold2.release());
     __r.__enable_weak_this(__r.__ptr_, __r.__ptr_);
     return __r;
 }
 
 template<class _Tp>
-template<class _Alloc, class _A0, class _A1>
-shared_ptr<_Tp>
-shared_ptr<_Tp>::allocate_shared(const _Alloc& __a, _A0& __a0, _A1& __a1)
-{
-    static_assert((is_constructible<_Tp, _A0, _A1>::value), "Can't construct object in allocate_shared" );
-    typedef __shared_ptr_emplace<_Tp, _Alloc> _CntrlBlk;
-    typedef typename __allocator_traits_rebind<_Alloc, _CntrlBlk>::type _Alloc2;
-    typedef __allocator_destructor<_Alloc2> _D2;
-    _Alloc2 __alloc2(__a);
-    unique_ptr<_CntrlBlk, _D2> __hold2(__alloc2.allocate(1), _D2(__alloc2, 1));
-    ::new(static_cast<void*>(_VSTD::addressof(*__hold2.get())))
-        _CntrlBlk(__a, __a0, __a1);
-    shared_ptr<_Tp> __r;
-    __r.__ptr_ = __hold2.get()->get();
-    __r.__cntrl_ = _VSTD::addressof(*__hold2.release());
-    __r.__enable_weak_this(__r.__ptr_, __r.__ptr_);
-    return __r;
-}
-
-template<class _Tp>
-template<class _Alloc, class _A0, class _A1, class _A2>
-shared_ptr<_Tp>
-shared_ptr<_Tp>::allocate_shared(const _Alloc& __a, _A0& __a0, _A1& __a1, _A2& __a2)
-{
-    static_assert((is_constructible<_Tp, _A0, _A1, _A2>::value), "Can't construct object in allocate_shared" );
-    typedef __shared_ptr_emplace<_Tp, _Alloc> _CntrlBlk;
-    typedef typename __allocator_traits_rebind<_Alloc, _CntrlBlk>::type _Alloc2;
-    typedef __allocator_destructor<_Alloc2> _D2;
-    _Alloc2 __alloc2(__a);
-    unique_ptr<_CntrlBlk, _D2> __hold2(__alloc2.allocate(1), _D2(__alloc2, 1));
-    ::new(static_cast<void*>(_VSTD::addressof(*__hold2.get())))
-        _CntrlBlk(__a, __a0, __a1, __a2);
-    shared_ptr<_Tp> __r;
-    __r.__ptr_ = __hold2.get()->get();
-    __r.__cntrl_ = _VSTD::addressof(*__hold2.release());
-    __r.__enable_weak_this(__r.__ptr_, __r.__ptr_);
-    return __r;
-}
-
-#endif  // _LIBCPP_HAS_NO_VARIADICS
-
-template<class _Tp>
 shared_ptr<_Tp>::~shared_ptr()
 {
     if (__cntrl_)
         __cntrl_->__release_shared();
 }
 
 template<class _Tp>
 inline

test/std/utilities/memory/util.smartptr/util.smartptr.shared/util.smartptr.shared.create/make_shared.protected.fail.cpp

 
 struct S {
 protected:
    S () {};  // ctor is protected
 };
 
 int main(int, char**)
 {
+    // expected-error@memory:* {{field of type 'S' has protected default constructor}}
     std::shared_ptr<S> p = std::make_shared<S>();  // expected-error-re@memory:* {{static_assert failed{{.*}} "Can't construct object in make_shared"}}
 
   return 0;
 }