From patchwork Thu Jan 3 23:46:27 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: [google,4_7] backport "std::unique_ptr improvements" Date: Thu, 03 Jan 2013 13:46:27 -0000 From: Lawrence Crowl X-Patchwork-Id: 209338 Message-Id: To: gcc-patches , Paul Pluzhnikov Cc: Jonathan Wakely , Geoffrey Romer Relax the restrictions on argument types to a unique_ptr instantiated on an array type. This patch is a backport of Jonathan Wakely's "std::unique_ptr improvements" http://gcc.gnu.org/ml/gcc-patches/2012-12/msg01271.html to the google 4.7 branch. The existing unique_ptr admits no conversions, not even cv-qualification conversions. The patch permits cv-qualification conversion. It prohibits derived-to-base pointer conversion, as is needed to prevent improper deletion. It permits user-defined pointer types. The latter permissiveness is under debate. Given struct base { }; struct derived : base { }; as we cannot presently distinguish between the safe struct ptr_base { operator base*() { return 0; } }; ... unique_ptr< base >( ptr_base() ); and the unsafe struct ptr_derived { operator derived*() { return 0; } }; ... unique_ptr< base >( ptr_derived() ); No existing code can encounter the unsafe part because no existing code can have user-defined pointer types. There are likely to be very few uses of user-defined pointers introduced into the code base between now and when the issue is resolved. So, backporting the patch as is seems safe enough, and not too much trouble to correct later. Tested on x86_64. Tests pass through Google's core and mantle, but fail for apparently unrelated reasons in crust. Okay for branch? Index: libstdc++-v3/testsuite/20_util/declval/requirements/1_neg.cc =================================================================== --- libstdc++-v3/testsuite/20_util/declval/requirements/1_neg.cc (revision 194742) +++ libstdc++-v3/testsuite/20_util/declval/requirements/1_neg.cc (working copy) @@ -19,7 +19,7 @@ // with this library; see the file COPYING3. If not see // . -// { dg-error "static assertion failed" "" { target *-*-* } 1776 } +// { dg-error "static assertion failed" "" { target *-*-* } 1778 } #include Index: libstdc++-v3/include/std/type_traits =================================================================== --- libstdc++-v3/include/std/type_traits (revision 194742) +++ libstdc++-v3/include/std/type_traits (working copy) @@ -1723,6 +1723,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION struct enable_if { typedef _Tp type; }; + template + using _Require = typename enable_if<__and_<_Cond...>::value>::type; // Primary template. /// Define a member typedef @c type to one of two argument types. Index: libstdc++-v3/include/bits/unique_ptr.h =================================================================== --- libstdc++-v3/include/bits/unique_ptr.h (revision 194742) +++ libstdc++-v3/include/bits/unique_ptr.h (working copy) @@ -52,7 +52,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION constexpr default_delete() noexcept = default; template::value>::type> + enable_if::value>::type> default_delete(const default_delete<_Up>&) noexcept { } void @@ -70,8 +70,23 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION template struct default_delete<_Tp[]> { + private: + template + using __remove_cv = typename remove_cv<_Up>::type; + + // Like is_base_of<_Tp, _Up> but false if unqualified types are the same + template + using __is_derived_Tp + = __and_< is_base_of<_Tp, _Up>, + __not_, __remove_cv<_Up>>> >; + + public: constexpr default_delete() noexcept = default; + template::value>::type> + default_delete(const default_delete<_Up[]>&) noexcept { } + void operator()(_Tp* __ptr) const { @@ -80,7 +95,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION delete [] __ptr; } - template void operator()(_Up*) const = delete; + template + typename enable_if<__is_derived_Tp<_Up>::value>::type + operator()(_Up*) const = delete; }; /// 20.7.12.2 unique_ptr for single objects. @@ -99,7 +116,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION typedef typename remove_reference<_Dp>::type _Del; public: - typedef decltype( __test<_Del>(0)) type; + typedef decltype(__test<_Del>(0)) type; }; typedef std::tuple __tuple_type; @@ -113,54 +130,45 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION // Constructors. constexpr unique_ptr() noexcept : _M_t() - { static_assert(!std::is_pointer::value, + { static_assert(!is_pointer::value, "constructed with null function pointer deleter"); } explicit unique_ptr(pointer __p) noexcept : _M_t(__p, deleter_type()) - { static_assert(!std::is_pointer::value, + { static_assert(!is_pointer::value, "constructed with null function pointer deleter"); } unique_ptr(pointer __p, - typename std::conditional::value, + typename std::conditional::value, deleter_type, const deleter_type&>::type __d) noexcept : _M_t(__p, __d) { } unique_ptr(pointer __p, - typename std::remove_reference::type&& __d) noexcept + typename remove_reference::type&& __d) noexcept : _M_t(std::move(__p), std::move(__d)) { static_assert(!std::is_reference::value, "rvalue deleter bound to reference"); } - constexpr unique_ptr(nullptr_t) noexcept - : _M_t() - { static_assert(!std::is_pointer::value, - "constructed with null function pointer deleter"); } + constexpr unique_ptr(nullptr_t) noexcept : unique_ptr() { } // Move constructors. unique_ptr(unique_ptr&& __u) noexcept : _M_t(__u.release(), std::forward(__u.get_deleter())) { } - template::pointer, - pointer>::value - && !std::is_array<_Up>::value - && ((std::is_reference<_Dp>::value - && std::is_same<_Ep, _Dp>::value) - || (!std::is_reference<_Dp>::value - && std::is_convertible<_Ep, _Dp>::value))> - ::type> + template::pointer, pointer>, + __not_>, + typename conditional::value, + is_same<_Ep, _Dp>, + is_convertible<_Ep, _Dp>>::type>> unique_ptr(unique_ptr<_Up, _Ep>&& __u) noexcept : _M_t(__u.release(), std::forward<_Ep>(__u.get_deleter())) { } #if _GLIBCXX_USE_DEPRECATED - template::value - && std::is_same<_Dp, - default_delete<_Tp>>::value>::type> + template, is_same<_Dp, default_delete<_Tp>>>> unique_ptr(auto_ptr<_Up>&& __u) noexcept : _M_t(__u.release(), deleter_type()) { } #endif @@ -183,12 +191,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION return *this; } - template::pointer, - pointer>::value - && !std::is_array<_Up>::value>::type> - unique_ptr& + template + typename enable_if< __and_< + is_convertible::pointer, pointer>, + __not_> + >::value, + unique_ptr&>::type operator=(unique_ptr<_Up, _Ep>&& __u) noexcept { reset(__u.release()); @@ -204,7 +212,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION } // Observers. - typename std::add_lvalue_reference::type + typename add_lvalue_reference::type operator*() const { _GLIBCXX_DEBUG_ASSERT(get() != pointer()); @@ -270,11 +278,47 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION template class unique_ptr<_Tp[], _Dp> { - typedef std::tuple<_Tp*, _Dp> __tuple_type; - __tuple_type _M_t; + // use SFINAE to determine whether _Del::pointer exists + class _Pointer + { + template + static typename _Up::pointer __test(typename _Up::pointer*); + + 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 + using __remove_cv = typename remove_cv<_Up>::type; + + // like is_base_of<_Tp, _Up> but false if unqualified types are the same + template + using __is_derived_Tp + = __and_< is_base_of<_Tp, _Up>, + __not_, __remove_cv<_Up>>> >; + + template::pointer> + using __safe_conversion = __and_< + is_convertible<_Up_pointer, _Tp_pointer>, + is_array<_Up>, + __or_<__not_>, + __not_>, + __not_<__is_derived_Tp::type>> + > + >; public: - typedef _Tp* pointer; + typedef typename _Pointer::type pointer; typedef _Tp element_type; typedef _Dp deleter_type; @@ -282,35 +326,42 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION constexpr unique_ptr() noexcept : _M_t() { static_assert(!std::is_pointer::value, - "constructed with null function pointer deleter"); } + "constructed with null function pointer deleter"); } explicit unique_ptr(pointer __p) noexcept : _M_t(__p, deleter_type()) - { static_assert(!std::is_pointer::value, - "constructed with null function pointer deleter"); } + { static_assert(!is_pointer::value, + "constructed with null function pointer deleter"); } + + template, + is_convertible<_Up*, pointer>, __is_derived_Tp<_Up>>> + explicit + unique_ptr(_Up* __p) = delete; unique_ptr(pointer __p, - typename std::conditional::value, + typename conditional::value, deleter_type, const deleter_type&>::type __d) noexcept : _M_t(__p, __d) { } unique_ptr(pointer __p, typename - std::remove_reference::type && __d) noexcept + remove_reference::type&& __d) noexcept : _M_t(std::move(__p), std::move(__d)) - { static_assert(!std::is_reference::value, + { static_assert(!is_reference::value, "rvalue deleter bound to reference"); } - constexpr unique_ptr(nullptr_t) noexcept - : _M_t() - { static_assert(!std::is_pointer::value, - "constructed with null function pointer deleter"); } - - // Move constructors. + // Move constructor. unique_ptr(unique_ptr&& __u) noexcept : _M_t(__u.release(), std::forward(__u.get_deleter())) { } - template + constexpr unique_ptr(nullptr_t) noexcept : unique_ptr() { } + + template, + typename conditional::value, + is_same<_Ep, _Dp>, + is_convertible<_Ep, _Dp>>::type + >> unique_ptr(unique_ptr<_Up, _Ep>&& __u) noexcept : _M_t(__u.release(), std::forward<_Ep>(__u.get_deleter())) { } @@ -334,7 +385,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION } template - unique_ptr& + typename + enable_if<__safe_conversion<_Up, _Ep>::value, unique_ptr&>::type operator=(unique_ptr<_Up, _Ep>&& __u) noexcept { reset(__u.release()); @@ -382,26 +434,21 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION } void - reset(pointer __p = pointer()) noexcept - { - using std::swap; - swap(std::get<0>(_M_t), __p); - if (__p != nullptr) - get_deleter()(__p); - } + reset() noexcept + { reset(pointer()); } void - reset(nullptr_t) noexcept + reset(pointer __p) noexcept { - pointer __p = get(); - std::get<0>(_M_t) = pointer(); + using std::swap; + swap(std::get<0>(_M_t), __p); if (__p != nullptr) get_deleter()(__p); } - // DR 821. - template - void reset(_Up) = delete; + template, + is_convertible<_Up*, pointer>, __is_derived_Tp<_Up>>> + void reset(_Up*) = delete; void swap(unique_ptr& __u) noexcept @@ -415,23 +462,16 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION unique_ptr& operator=(const unique_ptr&) = delete; // Disable construction from convertible pointer types. - // (N2315 - 20.6.5.3.1) - template + template, + is_convertible<_Up*, pointer>, __is_derived_Tp<_Up>>> unique_ptr(_Up*, typename - std::conditional::value, - deleter_type, const deleter_type&>::type, - typename std::enable_if::value>::type* = 0) = delete; - - template - unique_ptr(_Up*, typename std::remove_reference::type&&, - typename std::enable_if::value>::type* = 0) = delete; + conditional::value, + deleter_type, const deleter_type&>::type) = delete; - template - explicit - unique_ptr(_Up*, typename std::enable_if::value>::type* = 0) = delete; + template, + is_convertible<_Up*, pointer>, __is_derived_Tp<_Up>>> + unique_ptr(_Up*, typename + remove_reference::type&&) = delete; }; template Index: libstdc++-v3/include/bits/shared_ptr_base.h =================================================================== --- libstdc++-v3/include/bits/shared_ptr_base.h (revision 194742) +++ libstdc++-v3/include/bits/shared_ptr_base.h (working copy) @@ -621,7 +621,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION _S_create_from_up(std::unique_ptr<_Tp, _Del>&& __r, typename std::enable_if::value>::type* = 0) { - return new _Sp_counted_deleter<_Tp*, _Del, std::allocator, + typedef typename unique_ptr<_Tp, _Del>::pointer _Ptr; + return new _Sp_counted_deleter<_Ptr, _Del, std::allocator, _Lp>(__r.get(), __r.get_deleter()); } @@ -630,9 +631,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION _S_create_from_up(std::unique_ptr<_Tp, _Del>&& __r, typename std::enable_if::value>::type* = 0) { + typedef typename unique_ptr<_Tp, _Del>::pointer _Ptr; typedef typename std::remove_reference<_Del>::type _Del1; typedef std::reference_wrapper<_Del1> _Del2; - return new _Sp_counted_deleter<_Tp*, _Del2, std::allocator, + return new _Sp_counted_deleter<_Ptr, _Del2, std::allocator, _Lp>(__r.get(), std::ref(__r.get_deleter())); } @@ -851,7 +853,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION : _M_ptr(__r.get()), _M_refcount() { __glibcxx_function_requires(_ConvertibleConcept<_Tp1*, _Tp*>) - _Tp1* __tmp = __r.get(); + auto __tmp = std::__addressof(*__r.get()); _M_refcount = __shared_count<_Lp>(std::move(__r)); __enable_shared_from_this_helper(_M_refcount, __tmp, __tmp); } Index: libstdc++-v3/testsuite/20_util/shared_ptr/cons/43820_neg.cc =================================================================== --- libstdc++-v3/testsuite/20_util/shared_ptr/cons/43820_neg.cc (revision 194742) +++ libstdc++-v3/testsuite/20_util/shared_ptr/cons/43820_neg.cc (working copy) @@ -32,9 +32,9 @@ void test01() { X* px = 0; std::shared_ptr p1(px); // { dg-error "here" } - // { dg-error "incomplete" "" { target *-*-* } 775 } + // { dg-error "incomplete" "" { target *-*-* } 777 } std::shared_ptr p9(ap()); // { dg-error "here" } - // { dg-error "incomplete" "" { target *-*-* } 869 } + // { dg-error "incomplete" "" { target *-*-* } 871 } } Index: libstdc++-v3/testsuite/20_util/shared_ptr/cons/unique_ptr.cc =================================================================== --- libstdc++-v3/testsuite/20_util/shared_ptr/cons/unique_ptr.cc (revision 194742) +++ libstdc++-v3/testsuite/20_util/shared_ptr/cons/unique_ptr.cc (working copy) @@ -1,6 +1,6 @@ -// { dg-options "-std=gnu++0x" } +// { dg-options "-std=gnu++11" } -// Copyright (C) 2008, 2009 Free Software Foundation +// Copyright (C) 2008-2012 Free Software Foundation // // 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 @@ -17,14 +17,14 @@ // with this library; see the file COPYING3. If not see // . -// 20.7.12.2 Template class shared_ptr [util.smartptr.shared] +// 20.7.2.2 Class template shared_ptr [util.smartptr.shared] #include #include struct A { }; -// 20.7.12.2.1 shared_ptr constructors [util.smartptr.shared.const] +// 20.7.2.2.1 shared_ptr constructors [util.smartptr.shared.const] // Construction from unique_ptr int Index: libstdc++-v3/testsuite/20_util/unique_ptr/modifiers/reset_neg.cc =================================================================== --- libstdc++-v3/testsuite/20_util/unique_ptr/modifiers/reset_neg.cc (revision 194742) +++ libstdc++-v3/testsuite/20_util/unique_ptr/modifiers/reset_neg.cc (working copy) @@ -1,7 +1,7 @@ // { dg-do compile } // { dg-options "-std=gnu++0x" } -// Copyright (C) 2008, 2009, 2010 Free Software Foundation +// Copyright (C) 2008-2012 Free Software Foundation // // 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 @@ -32,7 +32,7 @@ struct B : A void test01() { std::unique_ptr up; - up.reset(new A[3]); // { dg-error "deleted" } + up.reset(new A[3]); // { dg-error "invalid conversion" } } // { dg-prune-output "include" } Index: libstdc++-v3/testsuite/20_util/unique_ptr/cons/ptr_deleter_neg.cc =================================================================== --- libstdc++-v3/testsuite/20_util/unique_ptr/cons/ptr_deleter_neg.cc (revision 194742) +++ libstdc++-v3/testsuite/20_util/unique_ptr/cons/ptr_deleter_neg.cc (working copy) @@ -1,7 +1,7 @@ // { dg-options "-std=gnu++0x" } // { dg-do compile } -// Copyright (C) 2010 Free Software Foundation +// Copyright (C) 2010-2012 Free Software Foundation // // 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 @@ -18,10 +18,9 @@ // with this library; see the file COPYING3. If not see // . -// 20.6.11 Template class unique_ptr [unique.ptr] +// 20.7.1 Class template unique_ptr [unique.ptr] #include -#include using std::unique_ptr; @@ -30,9 +29,9 @@ using std::unique_ptr; void test01() { - unique_ptr p1; // { dg-error "here" } + unique_ptr p1; // { dg-error "here" } - unique_ptr p2(nullptr); // { dg-error "here" } + unique_ptr p2(nullptr); // { dg-error "here" } unique_ptr p3(new int); // { dg-error "here" } } @@ -40,9 +39,9 @@ test01() void test02() { - unique_ptr p1; // { dg-error "here" } + unique_ptr p1; // { dg-error "here" } - unique_ptr p2(nullptr); // { dg-error "here" } + unique_ptr p2(nullptr); // { dg-error "here" } unique_ptr p3(new int[1]); // { dg-error "here" } } Index: libstdc++-v3/testsuite/20_util/unique_ptr/cons/pointer_array_convertible_neg.cc =================================================================== --- libstdc++-v3/testsuite/20_util/unique_ptr/cons/pointer_array_convertible_neg.cc (revision 194742) +++ libstdc++-v3/testsuite/20_util/unique_ptr/cons/pointer_array_convertible_neg.cc (working copy) @@ -1,7 +1,7 @@ // { dg-do compile } -// { dg-options "-std=gnu++0x" } +// { dg-options "-std=gnu++11" } -// Copyright (C) 2008, 2009 Free Software Foundation +// Copyright (C) 2008-2012 Free Software Foundation // // 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 @@ -29,7 +29,7 @@ struct B : A virtual ~B() { } }; -// 20.4.5.1 unique_ptr constructors [unique.ptr.cons] +// 20.7.1.3.1 unique_ptr constructors [unique.ptr.runtime.ctor] // Construction from pointer of derived type void Index: libstdc++-v3/testsuite/20_util/unique_ptr/requirements/pointer_type.cc =================================================================== --- libstdc++-v3/testsuite/20_util/unique_ptr/requirements/pointer_type.cc (revision 194742) +++ libstdc++-v3/testsuite/20_util/unique_ptr/requirements/pointer_type.cc (working copy) @@ -1,7 +1,7 @@ // { dg-do compile } -// { dg-options "-std=gnu++0x" } +// { dg-options "-std=gnu++11" } -// Copyright (C) 2010, 2011 Free Software Foundation +// Copyright (C) 2010-2012 Free Software Foundation // // 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 @@ -18,10 +18,9 @@ // with this library; see the file COPYING3. If not see // . -// 20.6.11 Template class unique_ptr [unique.ptr.single] +// 20.7.1.2 unique_ptr for single objects [unique.ptr.single] #include -#include struct A { Index: libstdc++-v3/testsuite/20_util/unique_ptr/assign/48635_neg.cc =================================================================== --- libstdc++-v3/testsuite/20_util/unique_ptr/assign/48635_neg.cc (revision 194742) +++ libstdc++-v3/testsuite/20_util/unique_ptr/assign/48635_neg.cc (working copy) @@ -41,10 +41,10 @@ void f() std::unique_ptr ub(nullptr, b); std::unique_ptr ud(nullptr, d); ub = std::move(ud); -// { dg-error "use of deleted function" "" { target *-*-* } 195 } +// { dg-error "use of deleted function" "" { target *-*-* } 203 } std::unique_ptr uba(nullptr, b); std::unique_ptr uda(nullptr, d); uba = std::move(uda); -// { dg-error "use of deleted function" "" { target *-*-* } 341 } +// { dg-error "use of deleted function" "" { target *-*-* } 393 } } Index: libstdc++-v3/testsuite/20_util/default_delete/48631_neg.cc =================================================================== --- libstdc++-v3/testsuite/20_util/default_delete/48631_neg.cc (revision 194742) +++ libstdc++-v3/testsuite/20_util/default_delete/48631_neg.cc (working copy) @@ -27,4 +27,4 @@ struct D : B { }; D d; std::default_delete db; typedef decltype(db(&d)) type; // { dg-error "use of deleted function" } -// { dg-error "declared here" "" { target *-*-* } 83 } +// { dg-error "declared here" "" { target *-*-* } 100 }