From f3070dab7a2ded2c2ee612d6e48ca01b77327379 Mon Sep 17 00:00:00 2001 From: Jonathan Wakely Date: Wed, 19 Oct 2016 21:13:14 +0100 Subject: [PATCH] Make std::enable_shared_from_this cope with ambiguity * include/backward/auto_ptr.h (__shared_ptr(auto_ptr&&)): Call _M_enable_shared_from_this_with instead of __enable_shared_from_this_helper. * include/bits/shared_ptr.h (__enable_shared_from_this_helper): Remove overload for std::enable_shared_from_this.. (__enable_shared_from_this_base): Define friend function to select a std::enable_shared_from_this base class. * include/bits/shared_ptr_base.h (__enable_shared_from_this_helper): Remove all overloads. (__shared_ptr): Change all relevant constructors to call _M_enable_shared_from_this_with instead of __enable_shared_from_this_helper. (__shared_ptr::__efst_base_t, __shared_ptr::__has_efst_base): Helpers to detect accessible and unambiguous enable_shared_from_this bases. (__shared_ptr::_M_enable_shared_from_this_with): New function to replace __enable_shared_from_this_helper overloads. (__enable_shared_from_this_helper): Remove overload for std::__enable_shared_from_this. (__enable_shared_from_this_base): Define friend function to select a std::__enable_shared_from_this base class. * include/experimental/bits/shared_ptr.h (experimental::shared_ptr): Change relevant constructors to call _M_enable_shared_from_this_with. (experimental::shared_ptr::__efst_base_t) (experimental::shared_ptr::__has_efst_base): Helpers to detect accessible and unambiguous enable_shared_from_this bases. (experimental::shared_ptr::_M_enable_shared_from_this_with): Define. (experimental::__enable_shared_from_this_helper): Remove overload for std::experimental::enable_shared_from_this. (experimental::__expt_enable_shared_from_this_base): Define friend function to select a std::experimental::enable_shared_from_this base. * testsuite/experimental/memory/shared_ptr/cons/ enable_shared_from_this.cc: New test. * testsuite/experimental/memory/shared_ptr/cons/unique_ptr_ctor.cc: Adjust expected behaviour for shared_ptr. From-SVN: r241353 --- libstdc++-v3/ChangeLog | 35 ++++++++ libstdc++-v3/include/backward/auto_ptr.h | 2 +- libstdc++-v3/include/bits/shared_ptr.h | 23 ++---- libstdc++-v3/include/bits/shared_ptr_base.h | 82 +++++++++---------- .../include/experimental/bits/shared_ptr.h | 69 ++++++++++++---- .../cons/enable_shared_from_this.cc | 47 +++++++++++ .../memory/shared_ptr/cons/unique_ptr_ctor.cc | 11 ++- 7 files changed, 191 insertions(+), 78 deletions(-) create mode 100644 libstdc++-v3/testsuite/experimental/memory/shared_ptr/cons/enable_shared_from_this.cc diff --git a/libstdc++-v3/ChangeLog b/libstdc++-v3/ChangeLog index 1caa17a7016..93a8f3ec4ca 100644 --- a/libstdc++-v3/ChangeLog +++ b/libstdc++-v3/ChangeLog @@ -1,5 +1,40 @@ 2016-10-19 Jonathan Wakely + * include/backward/auto_ptr.h (__shared_ptr(auto_ptr&&)): Call + _M_enable_shared_from_this_with instead of + __enable_shared_from_this_helper. + * include/bits/shared_ptr.h (__enable_shared_from_this_helper): + Remove overload for std::enable_shared_from_this.. + (__enable_shared_from_this_base): Define friend function to select a + std::enable_shared_from_this base class. + * include/bits/shared_ptr_base.h (__enable_shared_from_this_helper): + Remove all overloads. + (__shared_ptr): Change all relevant constructors to call + _M_enable_shared_from_this_with instead of + __enable_shared_from_this_helper. + (__shared_ptr::__efst_base_t, __shared_ptr::__has_efst_base): Helpers + to detect accessible and unambiguous enable_shared_from_this bases. + (__shared_ptr::_M_enable_shared_from_this_with): New function to + replace __enable_shared_from_this_helper overloads. + (__enable_shared_from_this_helper): Remove overload for + std::__enable_shared_from_this. + (__enable_shared_from_this_base): Define friend function to select a + std::__enable_shared_from_this base class. + * include/experimental/bits/shared_ptr.h (experimental::shared_ptr): + Change relevant constructors to call _M_enable_shared_from_this_with. + (experimental::shared_ptr::__efst_base_t) + (experimental::shared_ptr::__has_efst_base): Helpers to detect + accessible and unambiguous enable_shared_from_this bases. + (experimental::shared_ptr::_M_enable_shared_from_this_with): Define. + (experimental::__enable_shared_from_this_helper): Remove overload for + std::experimental::enable_shared_from_this. + (experimental::__expt_enable_shared_from_this_base): Define friend + function to select a std::experimental::enable_shared_from_this base. + * testsuite/experimental/memory/shared_ptr/cons/ + enable_shared_from_this.cc: New test. + * testsuite/experimental/memory/shared_ptr/cons/unique_ptr_ctor.cc: + Adjust expected behaviour for shared_ptr. + * include/debug/vector (__gnu_debug::vector::emplace_back): Fix return type. diff --git a/libstdc++-v3/include/backward/auto_ptr.h b/libstdc++-v3/include/backward/auto_ptr.h index 4dfc8cc9134..94911c87981 100644 --- a/libstdc++-v3/include/backward/auto_ptr.h +++ b/libstdc++-v3/include/backward/auto_ptr.h @@ -311,7 +311,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION static_assert( sizeof(_Tp1) > 0, "incomplete type" ); _Tp1* __tmp = __r.get(); _M_refcount = __shared_count<_Lp>(std::move(__r)); - __enable_shared_from_this_helper(_M_refcount, __tmp, __tmp); + _M_enable_shared_from_this_with(__tmp); } template diff --git a/libstdc++-v3/include/bits/shared_ptr.h b/libstdc++-v3/include/bits/shared_ptr.h index cbcb3b3f3ca..9b9261c04e4 100644 --- a/libstdc++-v3/include/bits/shared_ptr.h +++ b/libstdc++-v3/include/bits/shared_ptr.h @@ -607,25 +607,18 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION _M_weak_assign(_Tp1* __p, const __shared_count<>& __n) const noexcept { _M_weak_this._M_assign(__p, __n); } - template - friend void - __enable_shared_from_this_helper(const __shared_count<>&, - const enable_shared_from_this<_Tp1>*, - const _Tp2*) noexcept; + // Found by ADL when this is an associated class. + friend const enable_shared_from_this* + __enable_shared_from_this_base(const __shared_count<>&, + const enable_shared_from_this* __p) + { return __p; } + + template + friend class __shared_ptr; mutable weak_ptr<_Tp> _M_weak_this; }; - template - inline void - __enable_shared_from_this_helper(const __shared_count<>& __pn, - const enable_shared_from_this<_Tp1>* - __pe, const _Tp2* __px) noexcept - { - if (__pe != nullptr) - __pe->_M_weak_assign(const_cast<_Tp2*>(__px), __pn); - } - /** * @brief Create an object that is owned by a shared_ptr. * @param __a An allocator. diff --git a/libstdc++-v3/include/bits/shared_ptr_base.h b/libstdc++-v3/include/bits/shared_ptr_base.h index 422e3b5b959..c0686ad95da 100644 --- a/libstdc++-v3/include/bits/shared_ptr_base.h +++ b/libstdc++-v3/include/bits/shared_ptr_base.h @@ -847,28 +847,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION _M_pi = nullptr; } - // Support for enable_shared_from_this. - - // Friend of __enable_shared_from_this. - template<_Lock_policy _Lp, typename _Tp1, typename _Tp2> - void - __enable_shared_from_this_helper(const __shared_count<_Lp>&, - const __enable_shared_from_this<_Tp1, - _Lp>*, const _Tp2*) noexcept; - - // Friend of enable_shared_from_this. - template - void - __enable_shared_from_this_helper(const __shared_count<>&, - const enable_shared_from_this<_Tp1>*, - const _Tp2*) noexcept; - - template<_Lock_policy _Lp> - inline void - __enable_shared_from_this_helper(const __shared_count<_Lp>&, ...) noexcept - { } - - template class __shared_ptr { @@ -898,7 +876,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION __glibcxx_function_requires(_ConvertibleConcept<_Tp1*, _Tp*>) static_assert( !is_void<_Tp1>::value, "incomplete type" ); static_assert( sizeof(_Tp1) > 0, "incomplete type" ); - __enable_shared_from_this_helper(_M_refcount, __p, __p); + _M_enable_shared_from_this_with(__p); } template @@ -907,7 +885,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION { __glibcxx_function_requires(_ConvertibleConcept<_Tp1*, _Tp*>) // TODO requires _Deleter CopyConstructible and __d(__p) well-formed - __enable_shared_from_this_helper(_M_refcount, __p, __p); + _M_enable_shared_from_this_with(__p); } template @@ -916,7 +894,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION { __glibcxx_function_requires(_ConvertibleConcept<_Tp1*, _Tp*>) // TODO requires _Deleter CopyConstructible and __d(__p) well-formed - __enable_shared_from_this_helper(_M_refcount, __p, __p); + _M_enable_shared_from_this_with(__p); } template @@ -978,7 +956,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION __glibcxx_function_requires(_ConvertibleConcept<_Tp1*, _Tp*>) auto __raw = _S_raw_ptr(__r.get()); _M_refcount = __shared_count<_Lp>(std::move(__r)); - __enable_shared_from_this_helper(_M_refcount, __raw, __raw); + _M_enable_shared_from_this_with(__raw); } #if _GLIBCXX_USE_DEPRECATED @@ -1114,7 +1092,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION // This relies on _Sp_counted_ptr_inplace::_M_get_deleter. void* __p = _M_refcount._M_get_deleter(typeid(__tag)); _M_ptr = static_cast<_Tp*>(__p); - __enable_shared_from_this_helper(_M_refcount, _M_ptr, _M_ptr); + _M_enable_shared_from_this_with(_M_ptr); } #else template @@ -1146,7 +1124,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION __shared_count<_Lp> __count(__ptr, __del, __del._M_alloc); _M_refcount._M_swap(__count); _M_ptr = __ptr; - __enable_shared_from_this_helper(_M_refcount, _M_ptr, _M_ptr); + _M_enable_shared_from_this_with(_M_ptr); } #endif @@ -1166,6 +1144,34 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION friend class __weak_ptr<_Tp, _Lp>; private: + + template + using __esft_base_t = decltype(__enable_shared_from_this_base( + std::declval&>(), + std::declval<_Yp*>())); + + // Detect an accessible and unambiguous enable_shared_from_this base. + template + struct __has_esft_base + : false_type { }; + + template + struct __has_esft_base<_Yp, __void_t<__esft_base_t<_Yp>>> + : true_type { }; + + template + typename enable_if<__has_esft_base<_Yp>::value>::type + _M_enable_shared_from_this_with(const _Yp* __p) noexcept + { + if (auto __base = __enable_shared_from_this_base(_M_refcount, __p)) + __base->_M_weak_assign(const_cast<_Yp*>(__p), _M_refcount); + } + + template + typename enable_if::value>::type + _M_enable_shared_from_this_with(const _Yp*) noexcept + { } + void* _M_get_deleter(const std::type_info& __ti) const noexcept { return _M_refcount._M_get_deleter(__ti); } @@ -1579,26 +1585,14 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION _M_weak_assign(_Tp1* __p, const __shared_count<_Lp>& __n) const noexcept { _M_weak_this._M_assign(__p, __n); } - template<_Lock_policy _Lp1, typename _Tp1, typename _Tp2> - friend void - __enable_shared_from_this_helper(const __shared_count<_Lp1>&, - const __enable_shared_from_this<_Tp1, - _Lp1>*, const _Tp2*) noexcept; + friend void + __enable_shared_from_this_base(const __shared_count<_Lp>&, + const __enable_shared_from_this* __p) + { return __p; } mutable __weak_ptr<_Tp, _Lp> _M_weak_this; }; - template<_Lock_policy _Lp1, typename _Tp1, typename _Tp2> - inline void - __enable_shared_from_this_helper(const __shared_count<_Lp1>& __pn, - const __enable_shared_from_this<_Tp1, - _Lp1>* __pe, - const _Tp2* __px) noexcept - { - if (__pe != nullptr) - __pe->_M_weak_assign(const_cast<_Tp2*>(__px), __pn); - } - template inline __shared_ptr<_Tp, _Lp> __allocate_shared(const _Alloc& __a, _Args&&... __args) diff --git a/libstdc++-v3/include/experimental/bits/shared_ptr.h b/libstdc++-v3/include/experimental/bits/shared_ptr.h index 2e3da62c599..e8c533e158c 100644 --- a/libstdc++-v3/include/experimental/bits/shared_ptr.h +++ b/libstdc++-v3/include/experimental/bits/shared_ptr.h @@ -259,7 +259,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION // { // void* __p = _M_refcount._M_get_deleter(typeid(__tag)); // _M_ptr = static_cast<_Tp*>(__p); - // __enable_shared_from_this_helper(_M_refcount, _M_ptr, _M_ptr); // } // __weak_ptr::lock() @@ -557,7 +556,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION // { // void* __p = _M_refcount._M_get_deleter(typeid(__tag)); // _M_ptr = static_cast<_Tp*>(__p); - // __enable_shared_from_this_helper(_M_refcount, _M_ptr, _M_ptr); // } // __weak_ptr::lock() @@ -740,16 +738,19 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION template> explicit - shared_ptr(_Tp1* __p) : _Base_type(__p) { } + shared_ptr(_Tp1* __p) : _Base_type(__p) + { _M_enable_shared_from_this_with(__p); } template> shared_ptr(_Tp1* __p, _Deleter __d) - : _Base_type(__p, __d) { } + : _Base_type(__p, __d) + { _M_enable_shared_from_this_with(__p); } template> shared_ptr(_Tp1* __p, _Deleter __d, _Alloc __a) - : _Base_type(__p, __d, __a) { } + : _Base_type(__p, __d, __a) + { _M_enable_shared_from_this_with(__p); } template shared_ptr(nullptr_t __p, _Deleter __d) @@ -785,13 +786,20 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION #if _GLIBCXX_USE_DEPRECATED template> shared_ptr(std::auto_ptr<_Tp1>&& __r) - : _Base_type(std::move(__r)) { } + : _Base_type(std::move(__r)) + { _M_enable_shared_from_this_with(static_cast<_Tp1*>(this->get())); } #endif template> shared_ptr(unique_ptr<_Tp1, _Del>&& __r) - : _Base_type(std::move(__r)) { } + : _Base_type(std::move(__r)) + { + // XXX assume conversion from __r.get() to this->get() to __elem_t* + // is a round trip, which might not be true in all cases. + using __elem_t = typename unique_ptr<_Tp1, _Del>::element_type; + _M_enable_shared_from_this_with(static_cast<__elem_t*>(this->get())); + } constexpr shared_ptr(nullptr_t __p) : _Base_type(__p) { } @@ -853,7 +861,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION shared_ptr(_Sp_make_shared_tag __tag, const _Alloc& __a, _Args&&... __args) : _Base_type(__tag, __a, std::forward<_Args>(__args)...) - { } + { _M_enable_shared_from_this_with(this->get()); } template friend shared_ptr<_Tp1> @@ -863,6 +871,35 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION : _Base_type(__r, std::nothrow) { } friend class weak_ptr<_Tp>; + + template + using __esft_base_t = + decltype(__expt_enable_shared_from_this_base(std::declval<_Yp*>())); + + // Detect an accessible and unambiguous enable_shared_from_this base. + template + struct __has_esft_base + : false_type { }; + + template + struct __has_esft_base<_Yp, __void_t<__esft_base_t<_Yp>>> + : __bool_constant> { }; // ignore base for arrays + + template + typename enable_if<__has_esft_base<_Yp>::value>::type + _M_enable_shared_from_this_with(const _Yp* __p) noexcept + { + if (auto __base = __expt_enable_shared_from_this_base(__p)) + { + __base->_M_weak_this + = shared_ptr<_Yp>(*this, const_cast<_Yp*>(__p)); + } + } + + template + typename enable_if::value>::type + _M_enable_shared_from_this_with(const _Yp*) noexcept + { } }; // C++14 §20.8.2.2.7 //DOING @@ -1258,15 +1295,13 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION _M_weak_assign(_Tp1* __p, const __shared_count<>& __n) const noexcept { _M_weak_this._M_assign(__p, __n); } - template - friend void - __enable_shared_from_this_helper(const __shared_count<>& __pn, - const enable_shared_from_this* __pe, - const _Tp1* __px) noexcept - { - if(__pe != 0) - __pe->_M_weak_assign(const_cast<_Tp1*>(__px), __pn); - } + // Found by ADL when this is an associated class. + friend const enable_shared_from_this* + __expt_enable_shared_from_this_base(const enable_shared_from_this* __p) + { return __p; } + + template + friend class shared_ptr; mutable weak_ptr<_Tp> _M_weak_this; }; diff --git a/libstdc++-v3/testsuite/experimental/memory/shared_ptr/cons/enable_shared_from_this.cc b/libstdc++-v3/testsuite/experimental/memory/shared_ptr/cons/enable_shared_from_this.cc new file mode 100644 index 00000000000..5374f754558 --- /dev/null +++ b/libstdc++-v3/testsuite/experimental/memory/shared_ptr/cons/enable_shared_from_this.cc @@ -0,0 +1,47 @@ +// Copyright (C) 2016 Free Software Foundation, Inc. +// +// This file is part of the GNU ISO C++ Library. This library is free +// software; you can redistribute it and/or modify it under the +// terms of the GNU General Public License as published by the +// Free Software Foundation; either version 3, or (at your option) +// any later version. + +// This library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License along +// with this library; see the file COPYING3. If not see +// . + +// { dg-do run { target c++14 } } + +#include +#include + +struct A : std::enable_shared_from_this { }; +struct B : std::experimental::enable_shared_from_this { }; +struct C : A, B { }; + +void +test01() +{ + // This should not fail to compile due to ambiguous base classes: + std::experimental::shared_ptr p(new C); + + // And both base classes should have been enabled: + std::shared_ptr pa = p->A::shared_from_this(); + VERIFY( pa != nullptr ); + // Can't compare pa and p because they're different types + + std::experimental::shared_ptr pb = p->B::shared_from_this(); + VERIFY( pb != nullptr ); + VERIFY( pb == p ); +} + +int +main() +{ + test01(); +} diff --git a/libstdc++-v3/testsuite/experimental/memory/shared_ptr/cons/unique_ptr_ctor.cc b/libstdc++-v3/testsuite/experimental/memory/shared_ptr/cons/unique_ptr_ctor.cc index 0e61a3c662a..eb241765589 100644 --- a/libstdc++-v3/testsuite/experimental/memory/shared_ptr/cons/unique_ptr_ctor.cc +++ b/libstdc++-v3/testsuite/experimental/memory/shared_ptr/cons/unique_ptr_ctor.cc @@ -83,7 +83,16 @@ test02() VERIFY( sp.get() != 0 ); VERIFY( sp.use_count() == 1 ); - VERIFY( sp[0].shared_from_this() != nullptr ); + bool caught = false; + try + { + sp[0].shared_from_this(); // should not be set for arrays + } + catch (const std::bad_weak_ptr&) + { + caught = true; + } + VERIFY( caught ); sp.reset(); VERIFY( destroyed == 5 ); -- 2.30.2