This patch does three things:
1. Refactor std::enable_shared_from_this support code.
Instead of several overloads of __enable_shared_from_this_helper
that contain the same code but operating on slightly different types
I've split it into two parts. Upcasting to an accessible+unambiguous
enable_shared_from_this base class is done by new functions found by
ADL, __enable_shared_from_this_base. That is called by a new
template function __shared_ptr::_M_enable_shared_from_this_with()
which actually does the enabling by calling _M_weak_assign.
Calls to _M_enable_shared_from_this_with(p) closely match the
wording from my https://wg21.link/p0033r1 paper ("enables
shared_from_this with p") and are constrained so they don't give an
error if the __enable_shared_from_this_base() call is ambiguous. We
already gracefully handled ambiguous std::enable_shared_from_this
bases (PR56383) but failed noisily if a type had a combination of
std::enable_shared_from_this, std::__enable_shared_from_this and
std::experimental::enable_shared_from_this bases. Now we treat those
combinations gracefully.
2. Change std::experimental::enable_shared_from_this bases to be
initialized independently of std::enable_shared_from_this bases.
It's now possible to have both, and for both to be enabled. See
enable_shared_from_this.cc which tests this.
The standard says we have to enable shared_from_this for types with
an accessible and unambiguous std::enable_shared_from_this base
class, and we should be able to do that even if the class also has
an experimental::enable_shared_from_this base class. Now we can.
Potentially we could do the same for std::__enable_shared_from_this,
but I'm happy for those bases to be considered ambiguous with
std::enable_shared_from_this.
3. Don't enable shared_from_this for arrays stored in
experimental::shared_ptr. This matches the boost::shared_ptr
semantics, and I intend to propose this as a fix for C++17 too. It
doesn't make sense to treat the first element of an array specially
and enable shared_from_this for the first element only.
* 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<A[]>.
Tested powerpc64le-linux, comitted to trunk.
commit 83405dbeebb5487ae4745b2bdb2edd8f29414a07
Author: Jonathan Wakely <jwak...@redhat.com>
Date: Wed Oct 19 19:36:55 2016 +0100
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<A[]>.
diff --git a/libstdc++-v3/include/backward/auto_ptr.h
b/libstdc++-v3/include/backward/auto_ptr.h
index 4dfc8cc..94911c8 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<typename _Tp>
diff --git a/libstdc++-v3/include/bits/shared_ptr.h
b/libstdc++-v3/include/bits/shared_ptr.h
index cbcb3b3..9b9261c 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<typename _Tp1, typename _Tp2>
- 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<typename, _Lock_policy>
+ friend class __shared_ptr;
mutable weak_ptr<_Tp> _M_weak_this;
};
- template<typename _Tp1, typename _Tp2>
- 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 422e3b5..c0686ad 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<typename _Tp1, typename _Tp2>
- 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<typename _Tp, _Lock_policy _Lp>
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<typename _Tp1, typename _Deleter>
@@ -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<typename _Tp1, typename _Deleter, typename _Alloc>
@@ -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<typename _Deleter>
@@ -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<typename _Alloc>
@@ -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<typename _Yp>
+ using __esft_base_t = decltype(__enable_shared_from_this_base(
+ std::declval<const __shared_count<_Lp>&>(),
+ std::declval<_Yp*>()));
+
+ // Detect an accessible and unambiguous enable_shared_from_this base.
+ template<typename _Yp, typename = void>
+ struct __has_esft_base
+ : false_type { };
+
+ template<typename _Yp>
+ struct __has_esft_base<_Yp, __void_t<__esft_base_t<_Yp>>>
+ : true_type { };
+
+ template<typename _Yp>
+ 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 _Yp>
+ typename enable_if<!__has_esft_base<_Yp>::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<typename _Tp, _Lock_policy _Lp, typename _Alloc, typename... _Args>
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 2e3da62..e8c533e 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<typename _Tp1, typename = _SafeConv<_Tp1>>
explicit
- shared_ptr(_Tp1* __p) : _Base_type(__p) { }
+ shared_ptr(_Tp1* __p) : _Base_type(__p)
+ { _M_enable_shared_from_this_with(__p); }
template<typename _Tp1, typename _Deleter, typename = _SafeConv<_Tp1>>
shared_ptr(_Tp1* __p, _Deleter __d)
- : _Base_type(__p, __d) { }
+ : _Base_type(__p, __d)
+ { _M_enable_shared_from_this_with(__p); }
template<typename _Tp1, typename _Deleter, typename _Alloc,
typename = _SafeConv<_Tp1>>
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<typename _Deleter>
shared_ptr(nullptr_t __p, _Deleter __d)
@@ -785,13 +786,20 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
#if _GLIBCXX_USE_DEPRECATED
template<typename _Tp1, typename = _Compatible<_Tp1>>
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<typename _Tp1, typename _Del,
typename = _UniqCompatible<_Tp1, _Del>>
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<typename _Tp1, typename _Alloc, typename... _Args>
friend shared_ptr<_Tp1>
@@ -863,6 +871,35 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
: _Base_type(__r, std::nothrow) { }
friend class weak_ptr<_Tp>;
+
+ template<typename _Yp>
+ 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<typename _Yp, typename = void>
+ struct __has_esft_base
+ : false_type { };
+
+ template<typename _Yp>
+ struct __has_esft_base<_Yp, __void_t<__esft_base_t<_Yp>>>
+ : __bool_constant<!is_array_v<_Tp>> { }; // ignore base for arrays
+
+ template<typename _Yp>
+ 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 _Yp>
+ typename enable_if<!__has_esft_base<_Yp>::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<typename _Tp1>
- 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<typename>
+ 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 0000000..5374f75
--- /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
+// <http://www.gnu.org/licenses/>.
+
+// { dg-do run { target c++14 } }
+
+#include <experimental/memory>
+#include <testsuite_hooks.h>
+
+struct A : std::enable_shared_from_this<A> { };
+struct B : std::experimental::enable_shared_from_this<B> { };
+struct C : A, B { };
+
+void
+test01()
+{
+ // This should not fail to compile due to ambiguous base classes:
+ std::experimental::shared_ptr<C> p(new C);
+
+ // And both base classes should have been enabled:
+ std::shared_ptr<A> pa = p->A::shared_from_this();
+ VERIFY( pa != nullptr );
+ // Can't compare pa and p because they're different types
+
+ std::experimental::shared_ptr<B> 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 0e61a3c..eb24176 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 );
