From 86fc6ec9f366fd95d976c01bfa24c6775537ba62 Mon Sep 17 00:00:00 2001 From: Jonathan Wakely Date: Tue, 11 Sep 2018 11:55:49 +0100 Subject: [PATCH] Implement LWG 2905 changes to constrain unique_ptr constructors LWG DR 2905 says that is_constructible_v, P, D const &> should be false when D is not copy constructible. This commit implements the changes from the DR and simplifies the signatures as per https://github.com/cplusplus/draft/issues/1530 * include/bits/unique_ptr.h (__uniq_ptr_impl): Add assertions to check deleter type. (unique_ptr::unique_ptr(pointer, const deleter_type&)): Add copy constructible constraint. (unique_ptr::unique_ptr(pointer, deleter_type&&)): Disable for deleters of reference type and add move constructible constraint. (unique_ptr::unique_ptr(pointer, remove_reference_t&&)): Disable for deleters of non-reference type. Define as deleted. (unique_ptr): Likewise. * testsuite/20_util/unique_ptr/assign/48635_neg.cc: Replace dg-error directives with unstable line numbers with dg-prune-output. * testsuite/20_util/unique_ptr/cons/cv_qual_neg.cc: Likewise. * testsuite/20_util/unique_ptr/cons/lwg2905.cc: New test. * testsuite/20_util/unique_ptr/specialized_algorithms/swap_cxx17.cc: Make deleter types invocable. From-SVN: r264206 --- libstdc++-v3/ChangeLog | 19 ++++ libstdc++-v3/include/bits/unique_ptr.h | 90 +++++++++++-------- .../20_util/unique_ptr/assign/48635_neg.cc | 4 +- .../20_util/unique_ptr/cons/cv_qual_neg.cc | 2 +- .../20_util/unique_ptr/cons/lwg2905.cc | 78 ++++++++++++++++ .../specialized_algorithms/swap_cxx17.cc | 9 +- 6 files changed, 160 insertions(+), 42 deletions(-) create mode 100644 libstdc++-v3/testsuite/20_util/unique_ptr/cons/lwg2905.cc diff --git a/libstdc++-v3/ChangeLog b/libstdc++-v3/ChangeLog index 3de9031fd47..1a1a535532e 100644 --- a/libstdc++-v3/ChangeLog +++ b/libstdc++-v3/ChangeLog @@ -1,3 +1,22 @@ +2018-09-11 Jonathan Wakely + + Implement LWG 2905 changes to constrain unique_ptr constructors + * include/bits/unique_ptr.h (__uniq_ptr_impl): Add assertions to + check deleter type. + (unique_ptr::unique_ptr(pointer, const deleter_type&)): Add copy + constructible constraint. + (unique_ptr::unique_ptr(pointer, deleter_type&&)): Disable for + deleters of reference type and add move constructible constraint. + (unique_ptr::unique_ptr(pointer, remove_reference_t&&)): + Disable for deleters of non-reference type. Define as deleted. + (unique_ptr): Likewise. + * testsuite/20_util/unique_ptr/assign/48635_neg.cc: Replace dg-error + directives with unstable line numbers with dg-prune-output. + * testsuite/20_util/unique_ptr/cons/cv_qual_neg.cc: Likewise. + * testsuite/20_util/unique_ptr/cons/lwg2905.cc: New test. + * testsuite/20_util/unique_ptr/specialized_algorithms/swap_cxx17.cc: + Make deleter types invocable. + 2018-09-05 Jonathan Wakely * libsupc++/cxxabi.h (__cxa_demangle): Clarify doxygen comment. diff --git a/libstdc++-v3/include/bits/unique_ptr.h b/libstdc++-v3/include/bits/unique_ptr.h index 4c99a9c0d9b..ddc6ae0e233 100644 --- a/libstdc++-v3/include/bits/unique_ptr.h +++ b/libstdc++-v3/include/bits/unique_ptr.h @@ -139,6 +139,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION using pointer = typename _Ptr<_Tp, _Dp>::type; + static_assert( !is_rvalue_reference<_Dp>::value, + "unique_ptr's deleter type must be a function object type" + " or an lvalue reference type" ); + static_assert( __is_invocable<_Dp&, pointer&>::value, + "unique_ptr's deleter must be invocable with a pointer" ); + __uniq_ptr_impl() = default; __uniq_ptr_impl(pointer __p) : _M_t() { _M_ptr() = __p; } @@ -159,9 +165,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION template > class unique_ptr { - template - using _DeleterConstraint = - typename __uniq_ptr_impl<_Tp, _Up>::_DeleterConstraint::type; + template + using _DeleterConstraint = + typename __uniq_ptr_impl<_Tp, _Up>::_DeleterConstraint::type; __uniq_ptr_impl<_Tp, _Dp> _M_t; @@ -170,6 +176,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION using element_type = _Tp; using deleter_type = _Dp; + private: // helper template for detecting a safe conversion from another // unique_ptr template @@ -183,11 +190,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > >; + public: // Constructors. /// Default constructor, creates a unique_ptr that owns nothing. - template > + template> constexpr unique_ptr() noexcept : _M_t() { } @@ -198,8 +205,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION * * The deleter will be value-initialized. */ - template > + template> explicit unique_ptr(pointer __p) noexcept : _M_t(__p) @@ -212,27 +218,34 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION * * The deleter will be initialized with @p __d */ - unique_ptr(pointer __p, - typename conditional::value, - deleter_type, const deleter_type&>::type __d) noexcept - : _M_t(__p, __d) { } + template>> + unique_ptr(pointer __p, const deleter_type& __d) noexcept + : _M_t(__p, __d) { } /** Takes ownership of a pointer. * * @param __p A pointer to an object of @c element_type - * @param __d An rvalue reference to a deleter. + * @param __d An rvalue reference to a (non-reference) deleter. * * The deleter will be initialized with @p std::move(__d) */ - unique_ptr(pointer __p, - 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"); } + template>> + unique_ptr(pointer __p, + __enable_if_t::value, + _Del&&> __d) noexcept + : _M_t(__p, std::move(__d)) + { } + + template::type> + unique_ptr(pointer, + __enable_if_t::value, + _DelUnref&&>) = delete; /// Creates a unique_ptr that owns nothing. - template > + template> constexpr unique_ptr(nullptr_t) noexcept : unique_ptr() { } // Move constructors. @@ -454,8 +467,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION // Constructors. /// Default constructor, creates a unique_ptr that owns nothing. - template > + template> constexpr unique_ptr() noexcept : _M_t() { } @@ -485,12 +497,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION * * The deleter will be initialized with @p __d */ - template::value, bool>::type> - unique_ptr(_Up __p, - typename conditional::value, - deleter_type, const deleter_type&>::type __d) noexcept + template, + is_copy_constructible<_Del>>> + unique_ptr(_Up __p, const deleter_type& __d) noexcept : _M_t(__p, __d) { } /** Takes ownership of a pointer. @@ -501,22 +511,28 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION * * The deleter will be initialized with @p std::move(__d) */ - template::value, bool>::type> - unique_ptr(_Up __p, typename - remove_reference::type&& __d) noexcept - : _M_t(std::move(__p), std::move(__d)) - { static_assert(!is_reference::value, - "rvalue deleter bound to reference"); } + template, + is_move_constructible<_Del>>> + unique_ptr(_Up __p, + __enable_if_t::value, + _Del&&> __d) noexcept + : _M_t(std::move(__p), std::move(__d)) + { } + + template::type, + typename = _Require<__safe_conversion_raw<_Up>>> + unique_ptr(_Up, + __enable_if_t::value, + _DelUnref&&>) = delete; /// Move constructor. unique_ptr(unique_ptr&& __u) noexcept : _M_t(__u.release(), std::forward(__u.get_deleter())) { } /// Creates a unique_ptr that owns nothing. - template > + template> constexpr unique_ptr(nullptr_t) noexcept : unique_ptr() { } template ud(nullptr, d); ub = std::move(ud); // { dg-error "no match" } ub2 = ud; // { dg-error "no match" } -// { dg-error "no type" "" { target *-*-* } 307 } 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 *-*-* } 566 } } + +// { dg-prune-output "no type" } diff --git a/libstdc++-v3/testsuite/20_util/unique_ptr/cons/cv_qual_neg.cc b/libstdc++-v3/testsuite/20_util/unique_ptr/cons/cv_qual_neg.cc index c1b1c9efc64..7e820ba129a 100644 --- a/libstdc++-v3/testsuite/20_util/unique_ptr/cons/cv_qual_neg.cc +++ b/libstdc++-v3/testsuite/20_util/unique_ptr/cons/cv_qual_neg.cc @@ -39,7 +39,7 @@ test07() std::unique_ptr cA3(p); // { dg-error "no matching function" } std::unique_ptr vA3(p); // { dg-error "no matching function" } std::unique_ptr cvA3(p); // { dg-error "no matching function" } - // { dg-error "no type" "" { target *-*-* } 473 } + // { dg-prune-output "no type" } } template diff --git a/libstdc++-v3/testsuite/20_util/unique_ptr/cons/lwg2905.cc b/libstdc++-v3/testsuite/20_util/unique_ptr/cons/lwg2905.cc new file mode 100644 index 00000000000..8700630d1b3 --- /dev/null +++ b/libstdc++-v3/testsuite/20_util/unique_ptr/cons/lwg2905.cc @@ -0,0 +1,78 @@ +// Copyright (C) 2017 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 + +template +constexpr bool check() +{ return std::is_constructible, P, E>::value; } + +struct Del { void operator()(void*) const { } }; + +static_assert( ! check(), "" ); +static_assert( check(), "" ); +static_assert( check(), "" ); +static_assert( ! check(), "" ); +static_assert( check(), "" ); +static_assert( check(), "" ); + +static_assert( ! check(), "" ); +static_assert( check(), "" ); +static_assert( check(), "" ); +static_assert( ! check(), "" ); +static_assert( check(), "" ); +static_assert( check(), "" ); + +struct DelNoCopy { + DelNoCopy() = default; + DelNoCopy(const DelNoCopy&) = delete; + DelNoCopy(DelNoCopy&&) = default; + void operator()(void*) const { } +}; + +static_assert( ! check(), "" ); +static_assert( check(), "" ); +static_assert( check(), "" ); +static_assert( ! check(), "" ); +static_assert( ! check(), "" ); +static_assert( check(), "" ); + +static_assert( ! check(), "" ); +static_assert( check(), "" ); +static_assert( check(), "" ); +static_assert( ! check(), "" ); +static_assert( ! check(), "" ); +static_assert( check(), "" ); + +struct Base { virtual ~Base() { } }; +struct Derived : Base { }; + +static_assert( ! check(), "" ); +static_assert( check(), "" ); +static_assert( check(), "" ); +static_assert( ! check(), "" ); +static_assert( check(), "" ); +static_assert( check(), "" ); + +static_assert( ! check(), "" ); +static_assert( ! check(), "" ); +static_assert( ! check(), "" ); +static_assert( ! check(), "" ); +static_assert( ! check(), "" ); +static_assert( ! check(), "" ); diff --git a/libstdc++-v3/testsuite/20_util/unique_ptr/specialized_algorithms/swap_cxx17.cc b/libstdc++-v3/testsuite/20_util/unique_ptr/specialized_algorithms/swap_cxx17.cc index 298d951d319..604685c1ab1 100644 --- a/libstdc++-v3/testsuite/20_util/unique_ptr/specialized_algorithms/swap_cxx17.cc +++ b/libstdc++-v3/testsuite/20_util/unique_ptr/specialized_algorithms/swap_cxx17.cc @@ -21,13 +21,18 @@ #include // Not swappable, and unique_ptr not swappable via the generic std::swap. -struct C { }; +struct C { + void operator()(void*) const { } +}; void swap(C&, C&) = delete; static_assert( !std::is_swappable_v> ); // Not swappable, and unique_ptr not swappable via the generic std::swap. -struct D { D(D&&) = delete; }; +struct D { + D(D&&) = delete; + void operator()(void*) const { } +}; static_assert( !std::is_swappable_v> );