Re: [PATCH] PR77990 refactor unique_ptr to encapsulate tuple
On 17/10/16 14:37 +0100, Jonathan Wakely wrote:
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, 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): 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.
Committed to trunk. Here's the smaller patch I'm committing to the
branches.
commit 81ea53cd80fe7aaa3a323dba58bdb2259d672be3
Author: Jonathan Wakely
Date: Wed Oct 19 10:21:58 2016 +0100
PR77990 fix unique_ptr for non-copyable deleters
PR libstdc++/77990
* include/bits/unique_ptr.h (unique_ptr::unique_ptr(pointer)): Set
pointer member after value-initialization of tuple.
* testsuite/20_util/unique_ptr/assign/48635_neg.cc: Adjust dg-errors.
* testsuite/20_util/unique_ptr/cons/77990.cc: New test.
* testsuite/20_util/unique_ptr/cons/cv_qual.cc: Adjust dg-error.
diff --git a/libstdc++-v3/include/bits/unique_ptr.h b/libstdc++-v3/include/bits/unique_ptr.h
index c4a6d2f..8e30287 100644
--- a/libstdc++-v3/include/bits/unique_ptr.h
+++ b/libstdc++-v3/include/bits/unique_ptr.h
@@ -168,9 +168,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
*/
explicit
unique_ptr(pointer __p) noexcept
- : _M_t(__p, deleter_type())
- { static_assert(!is_pointer::value,
- "constructed with null function pointer deleter"); }
+ : _M_t()
+ {
+ std::get<0>(_M_t) = __p;
+ static_assert(!is_pointer::value,
+ "constructed with null function pointer deleter");
+ }
/** Takes ownership of a pointer.
*
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 16ff8ae..785f9ad 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
@@ -43,10 +43,10 @@ void f()
std::unique_ptr 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 *-*-* } 272 }
std::unique_ptr uba(nullptr, b);
std::unique_ptr uda(nullptr, d);
uba = std::move(uda); // { dg-error "no match" }
-// { dg-error "no type" "" { target *-*-* } 537 }
+// { dg-error "no type" "" { target *-*-* } 540 }
}
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 000..1acc313
--- /dev/null
+++ b/libstdc++-v3/testsuite/20_util/unique_ptr/cons/77990.cc
@@ -0,0 +1,28 @@
+// 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
Re: [PATCH] PR77990 refactor unique_ptr to encapsulate tuple
On 10/18/2016 12:54 PM, Jonathan Wakely wrote: > I'll wait a bit longer for any objections, as the refactoring could be > seen as unnecessary churn, but I think it's valuable housekeeping. Having stared at std::unique_ptr a lot recently, I like this, FWIW. Thanks, Pedro Alves
Re: [PATCH] PR77990 refactor unique_ptr to encapsulate tuple
On 17/10/16 14:37 +0100, Jonathan Wakely wrote:
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
ops, a dangling sentence. I meant to say that the tuple implementation
details leaks into the definition of lots of members, which have to
say std::get<0>(_M_t) or std::get<1>(_M_t) instead of using more
natural member names.
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, 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.
I'll wait a bit longer for any objections, as the refactoring could be
seen as unnecessary churn, but I think it's valuable housekeeping.
[PATCH] PR77990 refactor unique_ptr to encapsulate tuple
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, 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): 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
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): 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 >
-class unique_ptr
+ template
+class __uniq_ptr_impl
{
- // use SFINAE to determine whether _Del::pointer exists
- class _Pointer
- {
- template
- static typename _Up::pointer __test(typename _Up::pointer*);
+ template
+ struct _Ptr
+ {
+ using type = _Up*;
+ };
- template
- static _Tp* __test(...);
-
- typedef typename remove_reference<_Dp>::type _Del;
-
- public:
- typedef decltype(__test<_Del>(0)) type;
- };
-
- typedef std::tuple __tuple_type;
- __tuple_type _M_t;
+ template
+ struct
+ _Ptr<_Up, _Ep, __void_t::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
+ __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 _M_t;
+};
+
+ /// 20.7.1.2 unique_ptr for single objects.
+ template >
+class unique_ptr
+{
+ __uniq_ptr_impl<_Tp, _Dp> _M_t;
+
+public:
+ using pointer =
