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

3668

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

4072

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

4072

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

3663

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).

3808

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

3663

Just for readability. We used to have:

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

Now it is:

private:
//code
public:
//code

I will revert.

3808

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

__shared_ptr_pointer<_Tp, _Dp, _Alloc>::__on_zero_shared_weak() _NOEXCEPT
     __data_.second().~_Alloc();
     __a.deallocate(_PTraits::pointer_to(*this), 1);
 }
 
 template <class _Tp, class _Alloc>
 class __shared_ptr_emplace
     : public __shared_weak_count
 {
+private:
     __compressed_pair<_Alloc, _Tp> __data_;
+
+    virtual void __on_zero_shared() _NOEXCEPT;
+    virtual void __on_zero_shared_weak() _NOEXCEPT;
+
 public:
+
 #ifndef _LIBCPP_HAS_NO_VARIADICS
 
     _LIBCPP_INLINE_VISIBILITY
     __shared_ptr_emplace(_Alloc __a)
         :  __data_(_VSTD::move(__a)) {}
 
     template <class ..._Args>
         _LIBCPP_INLINE_VISIBILITY
         __shared_ptr_emplace(_Alloc __a, _Args&& ...__args)
#else  // _LIBCPP_HAS_NO_VARIADICS
 
     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
 
-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());}
 };
 
 template <class _Tp, class _Alloc>
 void
 __shared_ptr_emplace<_Tp, _Alloc>::__on_zero_shared() _NOEXCEPT
 {
public:
     };
 };
 
 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());
-    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
+
+    template<class _Yp, class _Dp>
+#ifndef _LIBCPP_HAS_NO_RVALUE_REFERENCES
+        shared_ptr(unique_ptr<_Yp, _Dp&>&& __r,
+#else
+        shared_ptr(unique_ptr<_Yp, _Dp&> __r,
+#endif
+                is_convertible<typename unique_ptr<_Yp, _Dp>::pointer,
+                                    element_type*>::value,
+
+    template<class _Yp, class _Dp>
+#ifndef _LIBCPP_HAS_NO_RVALUE_REFERENCES
+        shared_ptr(unique_ptr<_Yp, _Dp>&& __r,
+#else
+        shared_ptr(unique_ptr<_Yp, _Dp> __r,
+#endif
+                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
         <
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
     __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,
+shared_ptr<_Tp>::shared_ptr(unique_ptr<_Yp, _Dp&>&& __r,
 #else
-shared_ptr<_Tp>::shared_ptr(unique_ptr<_Yp, _Dp> __r,
+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
                             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;