diff mbox

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

Message ID CAFk2RUb9Gkv+0T526+gh=9xL_JzTCjbQa18RM4Jr9S6bJ4=ysw@mail.gmail.com
State New
Headers show

Commit Message

Ville Voutilainen Dec. 29, 2016, 7:57 p.m. UTC 
On 22 December 2016 at 19:11, Jonathan Wakely <jwakely@redhat.com> wrote:
>>       /// 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?

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.

>>       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.

Hopefully fixed correctly in the new patch, please double-check.

>> --- /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

Should just be 2016, fixed.

2016-12-29  Ville Voutilainen  <ville.voutilainen@gmail.com>

    Implement 2801, Default-constructibility of unique_ptr.
    * include/bits/unique_ptr.h (__uniq_ptr_impl::_DeleterConstraint): New.
    (unique_ptr::_DeleterConstraint): Likewise.
    (unique_ptr()): Constrain.
    (unique_ptr(pointer)): Likewise.
    (unique_ptr(nullptr_t)): Likewise.
    (unique_ptr<_Tp[], _Dp>::_DeleterConstraint): New.
    (unique_ptr<_Tp[], _Dp>::unique_ptr()): Constrain.
    (unique_ptr<_Tp[], _Dp>::unique_ptr(_Up)): Likewise.
    (unique_ptr<_Tp[], _Dp>::unique_ptr(nullptr_t)): Likewise.
    * testsuite/20_util/unique_ptr/assign/48635_neg.cc: Adjust.
    * testsuite/20_util/unique_ptr/cons/cv_qual_neg.cc: Likewise.
    * testsuite/20_util/unique_ptr/cons/default.cc: New.
    * testsuite/20_util/unique_ptr/cons/ptr_deleter_neg.cc: Adjust.
diff mbox

Patch

diff --git a/libstdc++-v3/include/bits/unique_ptr.h b/libstdc++-v3/include/bits/unique_ptr.h
index 56e6ec0..f994c59 100644
--- a/libstdc++-v3/include/bits/unique_ptr.h
+++ b/libstdc++-v3/include/bits/unique_ptr.h
@@ -130,6 +130,10 @@  _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 +156,10 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
   template <typename _Tp, typename _Dp = default_delete<_Tp>>
     class unique_ptr
     {
+      template <class _Up>
+      using _DeleterConstraint =
+	typename __uniq_ptr_impl<_Tp, _Up>::_DeleterConstraint;
+
       __uniq_ptr_impl<_Tp, _Dp> _M_t;
 
     public:
@@ -175,10 +183,11 @@  _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 _Up = _Dp,
+		typename = typename _DeleterConstraint<_Up>::type>
+	constexpr unique_ptr() noexcept
+	: _M_t()
+        { }
 
       /** Takes ownership of a pointer.
        *
@@ -186,11 +195,12 @@  _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 _Up = _Dp,
+		typename = typename _DeleterConstraint<_Up>::type>
+	explicit
+	unique_ptr(pointer __p) noexcept
+	: _M_t(__p)
+        { }
 
       /** Takes ownership of a pointer.
        *
@@ -218,7 +228,9 @@  _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 _Up = _Dp,
+		typename = typename _DeleterConstraint<_Up>::type>
+	constexpr unique_ptr(nullptr_t) noexcept : unique_ptr() { }
 
       // Move constructors.
 
@@ -384,6 +396,10 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
   template<typename _Tp, typename _Dp>
     class unique_ptr<_Tp[], _Dp>
     {
+      template <typename _Up>
+      using _DeleterConstraint =
+	typename __uniq_ptr_impl<_Tp, _Up>::_DeleterConstraint;
+
       __uniq_ptr_impl<_Tp, _Dp> _M_t;
 
       template<typename _Up>
@@ -432,10 +448,11 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
       // Constructors.
 
       /// Default constructor, creates a unique_ptr that owns nothing.
-      constexpr unique_ptr() noexcept
-      : _M_t()
-      { static_assert(!std::is_pointer<deleter_type>::value,
-		      "constructed with null function pointer deleter"); }
+      template <typename _Up = _Dp,
+		typename = typename _DeleterConstraint<_Up>::type>
+	constexpr unique_ptr() noexcept
+	: _M_t()
+        { }
 
       /** Takes ownership of a pointer.
        *
@@ -445,13 +462,14 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
        * The deleter will be value-initialized.
        */
       template<typename _Up,
-               typename = typename enable_if<
+	       typename _Vp = _Dp,
+	       typename = typename _DeleterConstraint<_Vp>::type,
+	       typename = typename enable_if<
                  __safe_conversion_raw<_Up>::value, bool>::type>
-      explicit
-      unique_ptr(_Up __p) noexcept
-      : _M_t(__p)
-      { static_assert(!is_pointer<deleter_type>::value,
-		      "constructed with null function pointer deleter"); }
+	explicit
+	unique_ptr(_Up __p) noexcept
+	: _M_t(__p)
+        { }
 
       /** Takes ownership of a pointer.
        *
@@ -491,7 +509,9 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
       : _M_t(__u.release(), std::forward<deleter_type>(__u.get_deleter())) { }
 
       /// Creates a unique_ptr that owns nothing.
-      constexpr unique_ptr(nullptr_t) noexcept : unique_ptr() { }
+      template <typename _Up = _Dp,
+		typename = typename _DeleterConstraint<_Up>::type>
+	constexpr unique_ptr(nullptr_t) noexcept : unique_ptr() { }
 
       template<typename _Up, typename _Ep,
 	       typename = _Require<__safe_conversion_up<_Up, _Ep>>>
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 e9655f1..00f1778 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
@@ -42,10 +42,10 @@  void f()
   std::unique_ptr<int, D&> ud(nullptr, d);
   ub = std::move(ud); // { dg-error "no match" }
   ub2 = ud; // { dg-error "no match" }
-// { dg-error "no type" "" { target *-*-* } 289 }
+// { dg-error "no type" "" { target *-*-* } 301 }
 
   std::unique_ptr<int[], B&> uba(nullptr, b);
   std::unique_ptr<int[], D&> uda(nullptr, d);
   uba = std::move(uda); // { dg-error "no match" }
-// { dg-error "no type" "" { target *-*-* } 540 }
+// { dg-error "no type" "" { target *-*-* } 560 }
 }
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 3e6f41b..406be1a 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<const A[]> cA3(p); // { dg-error "no matching function" }
   std::unique_ptr<volatile A[]> vA3(p); // { dg-error "no matching function" }
   std::unique_ptr<const volatile A[]> cvA3(p); // { dg-error "no matching function" }
-  // { dg-error "no type" "" { target *-*-* } 448 }
+  // { dg-error "no type" "" { target *-*-* } 467 }
 }
 
 template<typename T>
diff --git a/libstdc++-v3/testsuite/20_util/unique_ptr/cons/default.cc b/libstdc++-v3/testsuite/20_util/unique_ptr/cons/default.cc
new file mode 100644
index 0000000..ceffdcd
--- /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) 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 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
+// <http://www.gnu.org/licenses/>.
+
+#include <memory>
+#include <type_traits>
+
+static_assert(!std::is_default_constructible<std::unique_ptr<int,
+	      std::default_delete<int>&>>::value, "");
+static_assert(!std::is_default_constructible<std::unique_ptr<int,
+	      void(*)(int*)>>::value, "");
+static_assert(!std::is_constructible<std::unique_ptr<int,
+	      std::default_delete<int>&>, int*>::value, "");
+static_assert(!std::is_constructible<std::unique_ptr<int,
+	      void(*)(int*)>, int*>::value, "");
+
+static_assert(!std::is_default_constructible<std::unique_ptr<int[],
+	      std::default_delete<int>&>>::value, "");
+static_assert(!std::is_default_constructible<std::unique_ptr<int[],
+	      void(*)(int*)>>::value, "");
+static_assert(!std::is_constructible<std::unique_ptr<int[],
+	      std::default_delete<int>&>, int*>::value, "");
+static_assert(!std::is_constructible<std::unique_ptr<int[],
+	      void(*)(int*)>, int*>::value, "");
+
diff --git a/libstdc++-v3/testsuite/20_util/unique_ptr/cons/ptr_deleter_neg.cc b/libstdc++-v3/testsuite/20_util/unique_ptr/cons/ptr_deleter_neg.cc
index 1aa8d43..6196b92 100644
--- a/libstdc++-v3/testsuite/20_util/unique_ptr/cons/ptr_deleter_neg.cc
+++ b/libstdc++-v3/testsuite/20_util/unique_ptr/cons/ptr_deleter_neg.cc
@@ -23,26 +23,31 @@ 
 
 using std::unique_ptr;
 
-// { dg-prune-output "static assertion failed" }
+// { dg-error "no type" "" { target *-*-* } 187 }
+// { dg-error "no type" "" { target *-*-* } 199 }
+// { dg-error "no type" "" { target *-*-* } 232 }
+// { dg-error "no type" "" { target *-*-* } 452 }
+// { dg-error "no type" "" { target *-*-* } 466 }
+// { dg-error "no type" "" { target *-*-* } 513 }
 
 void
 test01()
 {
-  unique_ptr<long, void(*)(long*)> p1; // { dg-error "here" }
+  unique_ptr<long, void(*)(long*)> p1; // { dg-error "no matching" }
 
-  unique_ptr<short, void(*)(short*)> p2(nullptr); // { dg-error "here" }
+  unique_ptr<short, void(*)(short*)> p2(nullptr); // { dg-error "no matching" }
 
-  unique_ptr<int, void(*)(int*)> p3(new int); // { dg-error "here" }
+  unique_ptr<int, void(*)(int*)> p3(new int); // { dg-error "no matching" }
 }
 
 void
 test02()
 {
-  unique_ptr<long[], void(*)(long*)> p1; // { dg-error "here" }
+  unique_ptr<long[], void(*)(long*)> p1; // { dg-error "no matching" }
 
-  unique_ptr<short[], void(*)(short*)> p2(nullptr); // { dg-error "here" }
+  unique_ptr<short[], void(*)(short*)> p2(nullptr); // { dg-error "no matching" }
 
-  unique_ptr<int[], void(*)(int*)> p3(new int[1]); // { dg-error "here" }
+  unique_ptr<int[], void(*)(int*)> p3(new int[1]); // { dg-error "no matching" }
 }