diff mbox

[v3] Implement 2801, Default-constructibility of unique_ptr.

Message ID 20161222171123.GY895@redhat.com
State New
Headers show

Commit Message

Jonathan Wakely Dec. 22, 2016, 5:11 p.m. UTC 
On 20/12/16 22:52 +0200, Ville Voutilainen wrote:
>diff --git a/libstdc++-v3/include/bits/unique_ptr.h b/libstdc++-v3/include/bits/unique_ptr.h

>index 56e6ec0..63dff37 100644

>--- a/libstdc++-v3/include/bits/unique_ptr.h

>+++ b/libstdc++-v3/include/bits/unique_ptr.h

>@@ -175,10 +175,14 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION

>       // Constructors.

>

>       /// Default constructor, creates a unique_ptr that owns nothing.

>+      template <typename _Up = deleter_type,

>+		typename enable_if<

>+		  __and_<__not_<is_pointer<_Up>>,

>+			 is_default_constructible<_Up>>::value,

>+		  bool>::type = false>


Instead of repeating this condition half a dozen times, we could put
it in the __uniq_ptr_impl class template and reuse it, as in the
attached patch (and similarly for the unique_ptr<t[]> specialization).
What do you think?

>       constexpr unique_ptr() noexcept

>       : _M_t()

>-      { static_assert(!is_pointer<deleter_type>::value,

>-		     "constructed with null function pointer deleter"); }

>+      { }


The bodies of these constructors should be indented now that they're
templates.


>--- /dev/null

>+++ b/libstdc++-v3/testsuite/20_util/unique_ptr/cons/default.cc

>@@ -0,0 +1,40 @@

>+// { dg-do compile { target c++11 } }

>+

>+// Copyright (C) 2011-2016 Free Software Foundation, Inc.


Is this substantially copied from an existing file, or should it just
be 2016? (Not that it really matters, as I don't think we should have
copyright notices on tests like this at all, but that's something to
worry about another time.

Comments

Jonathan Wakely Jan. 4, 2017, 12:54 p.m. UTC | #1 
On 29/12/16 22:10 +0200, Ville Voutilainen wrote:
>On 29 December 2016 at 21:57, Ville Voutilainen

><ville.voutilainen@gmail.com> wrote:

>>> Instead of repeating this condition half a dozen times, we could put

>>> it in the __uniq_ptr_impl class template and reuse it, as in the

>>> attached patch (and similarly for the unique_ptr<t[]> specialization).

>>> What do you think?

>>

>> It needs to be a bit more dependent than in that patch, I think. I

>> adjusted the idea a bit,

>> a new patch attached. It cleans up the code a bit, so it's better, but

>> not a huge improvement.

>

>

>Gets a bit better by keeping the __uniq_ptr_impl as just an enable_if,

>aliasing its ::type locally

>and then using the result in the constraints. Also gets rid of a bunch

>of dg-errors in

>ptr_deleter_neg.cc. This makes me quite happy. The attached _3.diff

>shows the cleanup,

>the _4.diff shows the full patch with this cleanup applied.


Looks good. OK for trunk, thanks.
diff mbox

Patch

diff --git a/libstdc++-v3/include/bits/unique_ptr.h b/libstdc++-v3/include/bits/unique_ptr.h
index 56e6ec0..f9ab9a6 100644
--- a/libstdc++-v3/include/bits/unique_ptr.h
+++ b/libstdc++-v3/include/bits/unique_ptr.h
@@ -130,6 +130,9 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	};
 
     public:
+      using _DeleterConstraint = enable_if<
+	__and_<__not_<is_pointer<_Dp>, is_default_constructible<_Dp>>>::value>;
+
       using pointer = typename _Ptr<_Tp, _Dp>::type;
 
       __uniq_ptr_impl() = default;
@@ -152,6 +155,8 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
   template <typename _Tp, typename _Dp = default_delete<_Tp>>
     class unique_ptr
     {
+      using _DeleterConstraint = __uniq_ptr_impl<_Tp, _Dp>::_DeleterConstraint;
+
       __uniq_ptr_impl<_Tp, _Dp> _M_t;
 
     public:
@@ -175,10 +180,10 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
       // Constructors.
 
       /// Default constructor, creates a unique_ptr that owns nothing.
-      constexpr unique_ptr() noexcept
-      : _M_t()
-      { static_assert(!is_pointer<deleter_type>::value,
-		     "constructed with null function pointer deleter"); }
+      template<typename = typename _DeleterConstraint::type>
+	constexpr unique_ptr() noexcept
+	: _M_t()
+	{ }
 
       /** Takes ownership of a pointer.
        *
@@ -186,11 +191,11 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
        *
        * The deleter will be value-initialized.
        */
-      explicit
-      unique_ptr(pointer __p) noexcept
-      : _M_t(__p)
-      { static_assert(!is_pointer<deleter_type>::value,
-		     "constructed with null function pointer deleter"); }
+      template<typename = typename _DeleterConstraint::type>
+	explicit
+	unique_ptr(pointer __p) noexcept
+	: _M_t(__p)
+	{ }
 
       /** Takes ownership of a pointer.
        *
@@ -218,7 +223,8 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
 		      "rvalue deleter bound to reference"); }
 
       /// Creates a unique_ptr that owns nothing.
-      constexpr unique_ptr(nullptr_t) noexcept : unique_ptr() { }
+      template<typename = typename _DeleterConstraint::type>
+	constexpr unique_ptr(nullptr_t) noexcept : unique_ptr() { }
 
       // Move constructors.