We are incorrectly requiring unique_ptr deleters to be copyable here:
explicit
unique_ptr(pointer __p) noexcept
: _M_t(__p, deleter_type())
{ }
We could just do:
explicit
unique_ptr(pointer __p) noexcept
: _M_t()
{ std::get<0>(_M_t) = __p; }
But having to deal directly with the std::tuple inside unique_ptr has
been bothering me for some time. The tuple is used so we get the empty
base-class optimisation for the deleter, but that implementation
detail
This patch refactors unique_ptr to put the std::tuple member into a
new type which provides named accessors for the tuple elements, so we
can stop using get<0> and get<1>. That new type can also provide a
single-argument constructor to fix the copyable requirement for
deleters. This also removes the code for deducing the pointer type
which is duplciated in unique_ptr and unique_ptr<T[], D>, and while in
the neighbourhood I changed it from old-school SFINAE using overloaded
functions to the new hotness with __void_t<>.
I intend to commit this to trunk, but on the branches I'll just fix
the constructor as shown above, as it's a smaller change.
PR libstdc++/77990
* include/bits/unique_ptr.h (__uniq_ptr_impl): New type to
encapsulate implementation details.
(unique_ptr::unique_ptr(_Up)): Don't copy deleter object.
(unique_ptr::get, unique_ptr::get_deleter, unique_ptr::release):
Call member functions of implementation object.
(unique_ptr<T[], D>): Likewise.
* python/libstdcxx/v6/printers.py (UniquePointerPrinter): Adjust for
new implementation.
* python/libstdcxx/v6/xmethods.py (UniquePtrGetWorker): Likewise.
* testsuite/20_util/unique_ptr/assign/48635_neg.cc: Adjust dg-error
lines.
* testsuite/20_util/unique_ptr/assign/cv_qual.cc: Likewise.
* testsuite/20_util/unique_ptr/cons/cv_qual.cc: Likewise.
* testsuite/20_util/unique_ptr/cons/77990.cc: New test.
Tested powerpc64le-linux.
commit 48463e9c51e49d0c8972b5128ab8640aa90bc6a3
Author: Jonathan Wakely <jwak...@redhat.com>
Date: Fri Oct 14 22:56:11 2016 +0100
PR77990 refactor unique_ptr to encapsulate tuple
PR libstdc++/77990
* include/bits/unique_ptr.h (__uniq_ptr_impl): New type to
encapsulate implementation details.
(unique_ptr::unique_ptr(_Up)): Don't copy deleter object.
(unique_ptr::get, unique_ptr::get_deleter, unique_ptr::release):
Call member functions of implementation object.
(unique_ptr<T[], D>): Likewise.
* python/libstdcxx/v6/printers.py (UniquePointerPrinter): Adjust for
new implementation.
* python/libstdcxx/v6/xmethods.py (UniquePtrGetWorker): Likewise.
* testsuite/20_util/unique_ptr/assign/48635_neg.cc: Adjust dg-error
lines.
* testsuite/20_util/unique_ptr/assign/cv_qual.cc: Likewise.
* testsuite/20_util/unique_ptr/cons/cv_qual.cc: Likewise.
* testsuite/20_util/unique_ptr/cons/77990.cc: New test.
diff --git a/libstdc++-v3/include/bits/unique_ptr.h b/libstdc++-v3/include/bits/unique_ptr.h
index a36794d..e32b6c9 100644
--- a/libstdc++-v3/include/bits/unique_ptr.h
+++ b/libstdc++-v3/include/bits/unique_ptr.h
@@ -111,33 +111,51 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
}
};
- /// 20.7.1.2 unique_ptr for single objects.
- template <typename _Tp, typename _Dp = default_delete<_Tp> >
- class unique_ptr
+ template <typename _Tp, typename _Dp>
+ class __uniq_ptr_impl
{
- // use SFINAE to determine whether _Del::pointer exists
- class _Pointer
- {
- template<typename _Up>
- static typename _Up::pointer __test(typename _Up::pointer*);
+ template <typename _Up, typename _Ep, typename = void>
+ struct _Ptr
+ {
+ using type = _Up*;
+ };
- template<typename _Up>
- static _Tp* __test(...);
-
- typedef typename remove_reference<_Dp>::type _Del;
-
- public:
- typedef decltype(__test<_Del>(0)) type;
- };
-
- typedef std::tuple<typename _Pointer::type, _Dp> __tuple_type;
- __tuple_type _M_t;
+ template <typename _Up, typename _Ep>
+ struct
+ _Ptr<_Up, _Ep, __void_t<typename remove_reference<_Ep>::type::pointer>>
+ {
+ using type = typename remove_reference<_Ep>::type::pointer;
+ };
public:
- typedef typename _Pointer::type pointer;
- typedef _Tp element_type;
- typedef _Dp deleter_type;
+ using pointer = typename _Ptr<_Tp, _Dp>::type;
+ __uniq_ptr_impl() = default;
+ __uniq_ptr_impl(pointer __p) : _M_t() { _M_ptr() = __p; }
+
+ template<typename _Del>
+ __uniq_ptr_impl(pointer __p, _Del&& __d)
+ : _M_t(__p, std::forward<_Del>(__d)) { }
+
+ pointer& _M_ptr() { return std::get<0>(_M_t); }
+ pointer _M_ptr() const { return std::get<0>(_M_t); }
+ _Dp& _M_deleter() { return std::get<1>(_M_t); }
+ const _Dp& _M_deleter() const { return std::get<1>(_M_t); }
+
+ private:
+ tuple<pointer, _Dp> _M_t;
+ };
+
+ /// 20.7.1.2 unique_ptr for single objects.
+ template <typename _Tp, typename _Dp = default_delete<_Tp>>
+ class unique_ptr
+ {
+ __uniq_ptr_impl<_Tp, _Dp> _M_t;
+
+ public:
+ using pointer = typename __uniq_ptr_impl<_Tp, _Dp>::pointer;
+ using element_type = _Tp;
+ using deleter_type = _Dp;
// helper template for detecting a safe conversion from another
// unique_ptr
@@ -168,7 +186,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
*/
explicit
unique_ptr(pointer __p) noexcept
- : _M_t(__p, deleter_type())
+ : _M_t(__p)
{ static_assert(!is_pointer<deleter_type>::value,
"constructed with null function pointer deleter"); }
@@ -231,7 +249,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
/// Destructor, invokes the deleter if the stored pointer is not null.
~unique_ptr() noexcept
{
- auto& __ptr = std::get<0>(_M_t);
+ auto& __ptr = _M_t._M_ptr();
if (__ptr != nullptr)
get_deleter()(__ptr);
__ptr = pointer();
@@ -302,17 +320,17 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
/// Return the stored pointer.
pointer
get() const noexcept
- { return std::get<0>(_M_t); }
+ { return _M_t._M_ptr(); }
/// Return a reference to the stored deleter.
deleter_type&
get_deleter() noexcept
- { return std::get<1>(_M_t); }
+ { return _M_t._M_deleter(); }
/// Return a reference to the stored deleter.
const deleter_type&
get_deleter() const noexcept
- { return std::get<1>(_M_t); }
+ { return _M_t._M_deleter(); }
/// Return @c true if the stored pointer is not null.
explicit operator bool() const noexcept
@@ -325,7 +343,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
release() noexcept
{
pointer __p = get();
- std::get<0>(_M_t) = pointer();
+ _M_t._M_ptr() = pointer();
return __p;
}
@@ -339,7 +357,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
reset(pointer __p = pointer()) noexcept
{
using std::swap;
- swap(std::get<0>(_M_t), __p);
+ swap(_M_t._M_ptr(), __p);
if (__p != pointer())
get_deleter()(__p);
}
@@ -364,23 +382,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
template<typename _Tp, typename _Dp>
class unique_ptr<_Tp[], _Dp>
{
- // use SFINAE to determine whether _Del::pointer exists
- class _Pointer
- {
- template<typename _Up>
- static typename _Up::pointer __test(typename _Up::pointer*);
-
- template<typename _Up>
- static _Tp* __test(...);
-
- typedef typename remove_reference<_Dp>::type _Del;
-
- public:
- typedef decltype(__test<_Del>(0)) type;
- };
-
- typedef std::tuple<typename _Pointer::type, _Dp> __tuple_type;
- __tuple_type _M_t;
+ __uniq_ptr_impl<_Tp, _Dp> _M_t;
template<typename _Up>
using __remove_cv = typename remove_cv<_Up>::type;
@@ -391,11 +393,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
= __and_< is_base_of<_Tp, _Up>,
__not_<is_same<__remove_cv<_Tp>, __remove_cv<_Up>>> >;
-
public:
- typedef typename _Pointer::type pointer;
- typedef _Tp element_type;
- typedef _Dp deleter_type;
+ using pointer = typename __uniq_ptr_impl<_Tp, _Dp>::pointer;
+ using element_type = _Tp;
+ using deleter_type = _Dp;
// helper template for detecting a safe conversion from another
// unique_ptr
@@ -446,7 +447,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
__safe_conversion_raw<_Up>::value, bool>::type>
explicit
unique_ptr(_Up __p) noexcept
- : _M_t(__p, deleter_type())
+ : _M_t(__p)
{ static_assert(!is_pointer<deleter_type>::value,
"constructed with null function pointer deleter"); }
@@ -499,7 +500,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
/// Destructor, invokes the deleter if the stored pointer is not null.
~unique_ptr()
{
- auto& __ptr = std::get<0>(_M_t);
+ auto& __ptr = _M_t._M_ptr();
if (__ptr != nullptr)
get_deleter()(__ptr);
__ptr = pointer();
@@ -562,17 +563,17 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
/// Return the stored pointer.
pointer
get() const noexcept
- { return std::get<0>(_M_t); }
+ { return _M_t._M_ptr(); }
/// Return a reference to the stored deleter.
deleter_type&
get_deleter() noexcept
- { return std::get<1>(_M_t); }
+ { return _M_t._M_deleter(); }
/// Return a reference to the stored deleter.
const deleter_type&
get_deleter() const noexcept
- { return std::get<1>(_M_t); }
+ { return _M_t._M_deleter(); }
/// Return @c true if the stored pointer is not null.
explicit operator bool() const noexcept
@@ -585,7 +586,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
release() noexcept
{
pointer __p = get();
- std::get<0>(_M_t) = pointer();
+ _M_t._M_ptr() = pointer();
return __p;
}
@@ -612,7 +613,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
{
pointer __ptr = __p;
using std::swap;
- swap(std::get<0>(_M_t), __ptr);
+ swap(_M_t._M_ptr(), __ptr);
if (__ptr != nullptr)
get_deleter()(__ptr);
}
diff --git a/libstdc++-v3/python/libstdcxx/v6/printers.py b/libstdc++-v3/python/libstdcxx/v6/printers.py
index 714e1b8..bad42b4 100644
--- a/libstdc++-v3/python/libstdcxx/v6/printers.py
+++ b/libstdc++-v3/python/libstdcxx/v6/printers.py
@@ -126,7 +126,13 @@ class UniquePointerPrinter:
self.val = val
def to_string (self):
- v = self.val['_M_t']['_M_head_impl']
+ impl_type = self.val.type.fields()[0].type.tag
+ if impl_type.startswith('std::__uniq_ptr_impl<'): # New implementation
+ v = self.val['_M_t']['_M_t']['_M_head_impl']
+ elif impl_type.startswith('std::tuple<'):
+ v = self.val['_M_t']['_M_head_impl']
+ else:
+ raise ValueError("Unsupported implementation for unique_ptr: %s" % self.val.type.fields()[0].type.tag)
return ('std::unique_ptr<%s> containing %s' % (str(v.type.target()),
str(v)))
diff --git a/libstdc++-v3/python/libstdcxx/v6/xmethods.py b/libstdc++-v3/python/libstdcxx/v6/xmethods.py
index 605cda1..045b661 100644
--- a/libstdc++-v3/python/libstdcxx/v6/xmethods.py
+++ b/libstdc++-v3/python/libstdcxx/v6/xmethods.py
@@ -575,7 +575,12 @@ class UniquePtrGetWorker(gdb.xmethod.XMethodWorker):
return self._elem_type.pointer()
def __call__(self, obj):
- return obj['_M_t']['_M_head_impl']
+ impl_type = obj.dereference().type.fields()[0].type.tag
+ if impl_type.startswith('std::__uniq_ptr_impl<'): # New implementation
+ return obj['_M_t']['_M_t']['_M_head_impl']
+ elif impl_type.startswith('std::tuple<'):
+ return obj['_M_t']['_M_head_impl']
+ return None
class UniquePtrDerefWorker(UniquePtrGetWorker):
def __init__(self, elem_type):
diff --git a/libstdc++-v3/testsuite/20_util/unique_ptr/assign/48635_neg.cc b/libstdc++-v3/testsuite/20_util/unique_ptr/assign/48635_neg.cc
index 7f6d27c..311e8d5 100644
--- a/libstdc++-v3/testsuite/20_util/unique_ptr/assign/48635_neg.cc
+++ b/libstdc++-v3/testsuite/20_util/unique_ptr/assign/48635_neg.cc
@@ -42,10 +42,10 @@ void f()
std::unique_ptr<int, D&> ud(nullptr, d);
ub = std::move(ud); // { dg-error "no match" }
ub2 = ud; // { dg-error "no match" }
-// { dg-error "no type" "" { target *-*-* } 269 }
+// { dg-error "no type" "" { target *-*-* } 287 }
std::unique_ptr<int[], B&> uba(nullptr, b);
std::unique_ptr<int[], D&> uda(nullptr, d);
uba = std::move(uda); // { dg-error "no match" }
-// { dg-error "no type" "" { target *-*-* } 537 }
+// { dg-error "no type" "" { target *-*-* } 538 }
}
diff --git a/libstdc++-v3/testsuite/20_util/unique_ptr/cons/77990.cc b/libstdc++-v3/testsuite/20_util/unique_ptr/cons/77990.cc
new file mode 100644
index 0000000..e3e9a20
--- /dev/null
+++ b/libstdc++-v3/testsuite/20_util/unique_ptr/cons/77990.cc
@@ -0,0 +1,27 @@
+// 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 compile { target c++11 } }
+
+#include <memory>
+
+struct D {
+ D() = default;
+ D(const D&) = delete;
+ void operator()(int*);
+};
+std::unique_ptr<int, D> p((int*)nullptr); // PR libstdc++/77990
diff --git a/libstdc++-v3/testsuite/20_util/unique_ptr/cons/cv_qual.cc b/libstdc++-v3/testsuite/20_util/unique_ptr/cons/cv_qual.cc
index fcd5c5e..829d112 100644
--- a/libstdc++-v3/testsuite/20_util/unique_ptr/cons/cv_qual.cc
+++ b/libstdc++-v3/testsuite/20_util/unique_ptr/cons/cv_qual.cc
@@ -105,7 +105,7 @@ test07()
std::unique_ptr<const A[]> cA3(p); // { dg-error "no matching function" }
std::unique_ptr<volatile A[]> vA3(p); // { dg-error "no matching function" }
std::unique_ptr<const volatile A[]> cvA3(p); // { dg-error "no matching function" }
- // { dg-error "no type" "" { target *-*-* } 445 }
+ // { dg-error "no type" "" { target *-*-* } 446 }
}
template<typename T>