From 1b18663e0fbd6e253feca9e1f29e6f5b5538408d Mon Sep 17 00:00:00 2001 From: Jonathan Wakely Date: Tue, 14 May 2019 12:17:11 +0100 Subject: [PATCH] LWG 2899 - Make is_move_constructible correct for unique_ptr * include/bits/unique_ptr.h (__uniq_ptr_impl): Add move constructor, move assignment operator. (__uniq_ptr_impl::release(), __uniq_ptr_impl::reset(pointer)): Add. (__uniq_ptr_data): New class template with conditionally deleted special members. (unique_ptr, unique_ptr): Change type of data member from __uniq_ptr_impl to __uniq_ptr_data. Define move constructor and move assignment operator as defaulted. (unique_ptr::release(), unique_ptr::release()): Forward to __uniq_ptr_impl::release(). (unique_ptr::reset(pointer), unique_ptr::reset(U)): Forward to __uniq_ptr_impl::reset(pointer). * python/libstdcxx/v6/printers.py (UniquePointerPrinter.__init__): Check for new __uniq_ptr_data type. * testsuite/20_util/unique_ptr/dr2899.cc: New test. From-SVN: r271158 --- libstdc++-v3/ChangeLog | 19 +++ libstdc++-v3/include/bits/unique_ptr.h | 128 +++++++++++------- libstdc++-v3/python/libstdcxx/v6/printers.py | 4 +- .../testsuite/20_util/unique_ptr/dr2899.cc | 54 ++++++++ 4 files changed, 153 insertions(+), 52 deletions(-) create mode 100644 libstdc++-v3/testsuite/20_util/unique_ptr/dr2899.cc diff --git a/libstdc++-v3/ChangeLog b/libstdc++-v3/ChangeLog index c25f2ac7641..440a1dfd80d 100644 --- a/libstdc++-v3/ChangeLog +++ b/libstdc++-v3/ChangeLog @@ -1,3 +1,22 @@ +2019-05-14 Jonathan Wakely + + LWG 2899 - Make is_move_constructible correct for unique_ptr + * include/bits/unique_ptr.h (__uniq_ptr_impl): Add move constructor, + move assignment operator. + (__uniq_ptr_impl::release(), __uniq_ptr_impl::reset(pointer)): Add. + (__uniq_ptr_data): New class template with conditionally deleted + special members. + (unique_ptr, unique_ptr): Change type of data member from + __uniq_ptr_impl to __uniq_ptr_data. Define move + constructor and move assignment operator as defaulted. + (unique_ptr::release(), unique_ptr::release()): Forward to + __uniq_ptr_impl::release(). + (unique_ptr::reset(pointer), unique_ptr::reset(U)): Forward + to __uniq_ptr_impl::reset(pointer). + * python/libstdcxx/v6/printers.py (UniquePointerPrinter.__init__): + Check for new __uniq_ptr_data type. + * testsuite/20_util/unique_ptr/dr2899.cc: New test. + 2019-05-13 Jonathan Wakely PR libstdc++/90454.cc path construction from void* diff --git a/libstdc++-v3/include/bits/unique_ptr.h b/libstdc++-v3/include/bits/unique_ptr.h index a9e74725dfd..484c8b328e4 100644 --- a/libstdc++-v3/include/bits/unique_ptr.h +++ b/libstdc++-v3/include/bits/unique_ptr.h @@ -119,6 +119,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION /// @cond undocumented + // Manages the pointer and deleter of a unique_ptr template class __uniq_ptr_impl { @@ -153,14 +154,75 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION __uniq_ptr_impl(pointer __p, _Del&& __d) : _M_t(__p, std::forward<_Del>(__d)) { } + __uniq_ptr_impl(__uniq_ptr_impl&& __u) noexcept + : _M_t(std::move(__u._M_t)) + { __u._M_ptr() = nullptr; } + + __uniq_ptr_impl& operator=(__uniq_ptr_impl&& __u) noexcept + { + reset(__u.release()); + _M_deleter() = std::forward<_Dp>(__u._M_deleter()); + return *this; + } + 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); } + void reset(pointer __p) noexcept + { + const pointer __old_p = _M_ptr(); + _M_ptr() = __p; + if (__old_p) + _M_deleter()(__old_p); + } + + pointer release() noexcept + { + pointer __p = _M_ptr(); + _M_ptr() = nullptr; + return __p; + } + private: tuple _M_t; }; + + // Defines move construction + assignment as either defaulted or deleted. + template ::value, + bool = is_move_assignable<_Dp>::value> + struct __uniq_ptr_data : __uniq_ptr_impl<_Tp, _Dp> + { + using __uniq_ptr_impl<_Tp, _Dp>::__uniq_ptr_impl; + __uniq_ptr_data(__uniq_ptr_data&&) = default; + __uniq_ptr_data& operator=(__uniq_ptr_data&&) = default; + }; + + template + struct __uniq_ptr_data<_Tp, _Dp, true, false> : __uniq_ptr_impl<_Tp, _Dp> + { + using __uniq_ptr_impl<_Tp, _Dp>::__uniq_ptr_impl; + __uniq_ptr_data(__uniq_ptr_data&&) = default; + __uniq_ptr_data& operator=(__uniq_ptr_data&&) = delete; + }; + + template + struct __uniq_ptr_data<_Tp, _Dp, false, true> : __uniq_ptr_impl<_Tp, _Dp> + { + using __uniq_ptr_impl<_Tp, _Dp>::__uniq_ptr_impl; + __uniq_ptr_data(__uniq_ptr_data&&) = delete; + __uniq_ptr_data& operator=(__uniq_ptr_data&&) = default; + }; + + template + struct __uniq_ptr_data<_Tp, _Dp, false, false> : __uniq_ptr_impl<_Tp, _Dp> + { + using __uniq_ptr_impl<_Tp, _Dp>::__uniq_ptr_impl; + __uniq_ptr_data(__uniq_ptr_data&&) = delete; + __uniq_ptr_data& operator=(__uniq_ptr_data&&) = delete; + }; /// @endcond /// 20.7.1.2 unique_ptr for single objects. @@ -171,7 +233,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION using _DeleterConstraint = typename __uniq_ptr_impl<_Tp, _Up>::_DeleterConstraint::type; - __uniq_ptr_impl<_Tp, _Dp> _M_t; + __uniq_ptr_data<_Tp, _Dp> _M_t; public: using pointer = typename __uniq_ptr_impl<_Tp, _Dp>::pointer; @@ -255,8 +317,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION // Move constructors. /// Move constructor. - unique_ptr(unique_ptr&& __u) noexcept - : _M_t(__u.release(), std::forward(__u.get_deleter())) { } + unique_ptr(unique_ptr&&) = default; /** @brief Converting constructor from another type * @@ -298,24 +359,16 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION /** @brief Move assignment operator. * - * @param __u The object to transfer ownership from. - * - * Invokes the deleter first if this object owns a pointer. + * Invokes the deleter if this object owns a pointer. */ - unique_ptr& - operator=(unique_ptr&& __u) noexcept - { - reset(__u.release()); - get_deleter() = std::forward(__u.get_deleter()); - return *this; - } + unique_ptr& operator=(unique_ptr&&) = default; /** @brief Assignment from another type. * * @param __u The object to transfer ownership from, which owns a * convertible pointer to a non-array object. * - * Invokes the deleter first if this object owns a pointer. + * Invokes the deleter if this object owns a pointer. */ template typename enable_if< __and_< @@ -380,11 +433,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION /// Release ownership of any stored pointer. pointer release() noexcept - { - pointer __p = get(); - _M_t._M_ptr() = pointer(); - return __p; - } + { return _M_t.release(); } /** @brief Replace the stored pointer. * @@ -397,10 +446,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION { static_assert(__is_invocable::value, "unique_ptr's deleter must be invocable with a pointer"); - using std::swap; - swap(_M_t._M_ptr(), __p); - if (__p != pointer()) - get_deleter()(std::move(__p)); + _M_t.reset(std::move(__p)); } /// Exchange the pointer and deleter with another object. @@ -427,7 +473,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION using _DeleterConstraint = typename __uniq_ptr_impl<_Tp, _Up>::_DeleterConstraint::type; - __uniq_ptr_impl<_Tp, _Dp> _M_t; + __uniq_ptr_data<_Tp, _Dp> _M_t; template using __remove_cv = typename remove_cv<_Up>::type; @@ -536,8 +582,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION _DelUnref&&>) = delete; /// Move constructor. - unique_ptr(unique_ptr&& __u) noexcept - : _M_t(__u.release(), std::forward(__u.get_deleter())) { } + unique_ptr(unique_ptr&&) = default; /// Creates a unique_ptr that owns nothing. template> @@ -564,24 +609,17 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION /** @brief Move assignment operator. * - * @param __u The object to transfer ownership from. - * - * Invokes the deleter first if this object owns a pointer. + * Invokes the deleter if this object owns a pointer. */ unique_ptr& - operator=(unique_ptr&& __u) noexcept - { - reset(__u.release()); - get_deleter() = std::forward(__u.get_deleter()); - return *this; - } + operator=(unique_ptr&&) = default; /** @brief Assignment from another type. * * @param __u The object to transfer ownership from, which owns a * convertible pointer to an array object. * - * Invokes the deleter first if this object owns a pointer. + * Invokes the deleter if this object owns a pointer. */ template typename @@ -638,11 +676,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION /// Release ownership of any stored pointer. pointer release() noexcept - { - pointer __p = get(); - _M_t._M_ptr() = pointer(); - return __p; - } + { return _M_t.release(); } /** @brief Replace the stored pointer. * @@ -664,18 +698,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION >> void reset(_Up __p) noexcept - { - pointer __ptr = __p; - using std::swap; - swap(_M_t._M_ptr(), __ptr); - if (__ptr != nullptr) - get_deleter()(__ptr); - } + { _M_t.reset(std::move(__p)); } void reset(nullptr_t = nullptr) noexcept - { - reset(pointer()); - } + { reset(pointer()); } /// Exchange the pointer and deleter with another object. void diff --git a/libstdc++-v3/python/libstdcxx/v6/printers.py b/libstdc++-v3/python/libstdcxx/v6/printers.py index 19d367295df..eae06f93c34 100644 --- a/libstdc++-v3/python/libstdcxx/v6/printers.py +++ b/libstdc++-v3/python/libstdcxx/v6/printers.py @@ -182,7 +182,9 @@ class UniquePointerPrinter: def __init__ (self, typename, val): self.val = val impl_type = val.type.fields()[0].type.tag - if is_specialization_of(impl_type, '__uniq_ptr_impl'): # New implementation + # Check for new implementations first: + if is_specialization_of(impl_type, '__uniq_ptr_data') \ + or is_specialization_of(impl_type, '__uniq_ptr_impl'): self.pointer = val['_M_t']['_M_t']['_M_head_impl'] elif is_specialization_of(impl_type, 'tuple'): self.pointer = val['_M_t']['_M_head_impl'] diff --git a/libstdc++-v3/testsuite/20_util/unique_ptr/dr2899.cc b/libstdc++-v3/testsuite/20_util/unique_ptr/dr2899.cc new file mode 100644 index 00000000000..b528841c862 --- /dev/null +++ b/libstdc++-v3/testsuite/20_util/unique_ptr/dr2899.cc @@ -0,0 +1,54 @@ +// Copyright (C) 2019 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 compile { target c++11 } } + +#include + +struct Del { + Del() = default; + Del(Del&&) = delete; + + void operator()(int*) const; +}; + +static_assert(!std::is_move_constructible>::value); +static_assert(std::is_move_constructible>::value); + +struct Del2 { + Del2() = default; + Del2(Del2&&) = default; + Del2& operator=(Del2&&) = delete; + Del2& operator=(const Del2&) = default; + + void operator()(int*) const; +}; + +static_assert(!std::is_move_assignable>::value); +static_assert(std::is_move_assignable>::value); + +struct Del3 { + Del3() = default; + Del3(Del3&&) = default; + Del3& operator=(Del3&&) = default; + Del3& operator=(const Del3&) = delete; + + void operator()(int*) const; +}; + +static_assert(std::is_move_assignable>::value); +static_assert(!std::is_move_assignable>::value);