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

2017-01-04 Thread Jonathan Wakely 

On 29/12/16 22:10 +0200, Ville Voutilainen wrote:

On 29 December 2016 at 21:57, Ville Voutilainen
 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 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.

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

2016-12-29 Thread Ville Voutilainen 
On 29 December 2016 at 21:57, Ville Voutilainen
 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 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.
diff --git a/libstdc++-v3/include/bits/unique_ptr.h 
b/libstdc++-v3/include/bits/unique_ptr.h
index f994c59..211043f 100644
--- a/libstdc++-v3/include/bits/unique_ptr.h
+++ b/libstdc++-v3/include/bits/unique_ptr.h
@@ -158,7 +158,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 {
   template 
   using _DeleterConstraint =
-   typename __uniq_ptr_impl<_Tp, _Up>::_DeleterConstraint;
+   typename __uniq_ptr_impl<_Tp, _Up>::_DeleterConstraint::type;
 
   __uniq_ptr_impl<_Tp, _Dp> _M_t;
 
@@ -184,7 +184,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
   /// Default constructor, creates a unique_ptr that owns nothing.
   template ::type>
+   typename = _DeleterConstraint<_Up>>
constexpr unique_ptr() noexcept
: _M_t()
 { }
@@ -196,7 +196,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
* The deleter will be value-initialized.
*/
   template ::type>
+   typename = _DeleterConstraint<_Up>>
explicit
unique_ptr(pointer __p) noexcept
: _M_t(__p)
@@ -229,7 +229,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
   /// Creates a unique_ptr that owns nothing.
   template ::type>
+   typename = _DeleterConstraint<_Up>>
constexpr unique_ptr(nullptr_t) noexcept : unique_ptr() { }
 
   // Move constructors.
@@ -398,7 +398,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 {
   template 
   using _DeleterConstraint =
-   typename __uniq_ptr_impl<_Tp, _Up>::_DeleterConstraint;
+   typename __uniq_ptr_impl<_Tp, _Up>::_DeleterConstraint::type;
 
   __uniq_ptr_impl<_Tp, _Dp> _M_t;
 
@@ -449,7 +449,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
   /// Default constructor, creates a unique_ptr that owns nothing.
   template ::type>
+   typename = _DeleterConstraint<_Up>>
constexpr unique_ptr() noexcept
: _M_t()
 { }
@@ -463,7 +463,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
*/
   template::type,
+  typename = _DeleterConstraint<_Vp>,
   typename = typename enable_if<
  __safe_conversion_raw<_Up>::value, bool>::type>
explicit
@@ -510,7 +510,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
   /// Creates a unique_ptr that owns nothing.
   template ::type>
+   typename = _DeleterConstraint<_Up>>
constexpr unique_ptr(nullptr_t) noexcept : unique_ptr() { }
 
   templatediff --git a/libstdc++-v3/include/bits/unique_ptr.h 
b/libstdc++-v3/include/bits/unique_ptr.h
index 56e6ec0..211043f 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_default_constructible<_Dp>>::value>;
+
   using pointer = typename _Ptr<_Tp, _Dp>::type;
 
   __uniq_ptr_impl() = default;
@@ -152,6 +156,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   template >
 class unique_ptr
 {
+  template 
+  using _DeleterConstraint =
+   typename __uniq_ptr_impl<_Tp, _Up>::_DeleterConstraint::type;
+
   __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::value,
-"constructed with null function pointer deleter"); }
+  template >
+   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::value,
-"constructed with null function pointer deleter"); }
+  template >
+   explicit
+   unique_ptr(pointer __p) noexcept
+   : _M_t(__p)
+{ }
 
   /** Takes ownership of

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

2016-12-29 Thread Ville Voutilainen 
On 22 December 2016 at 19:11, Jonathan Wakely  wrote:
>>   /// Default constructor, creates a unique_ptr that owns nothing.
>> +  template > +   typename enable_if<
>> + __and_<__not_>,
>> +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 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::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  

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 --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_default_constructible<_Dp>>::value>;
+
   using pointer = typename _Ptr<_Tp, _Dp>::type;
 
   __uniq_ptr_impl() = default;
@@ -152,6 +156,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   template >
 class unique_ptr
 {
+  template 
+  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::value,
-"constructed with null function pointer deleter"); }
+  template ::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::value,
-"constructed with null function pointer deleter"); }
+  template ::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 ::type>
+   constexpr unique_ptr(nullptr_t) noexcept : unique_ptr() { }
 
   // Move constructors.
 
@@ -384,6 +396,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   template
 class unique_ptr<_Tp[], _Dp>
 {
+  template 
+  using _DeleterConstraint =
+   typename __uniq_ptr_impl<_Tp, _Up>::_DeleterConstraint;
+
   __uniq_ptr_impl<_Tp, _Dp> _M_t;
 
   template
@@ -432,10 +448,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   // Constructors.
 
   /// Default constructor, creates a unique_ptr that owns nothing.
-  constexpr unique_ptr() noexcept
-  : _M_t()
-  {

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

2016-12-22 Thread Jonathan Wakely 

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 >,
+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 specialization).
What do you think?


  constexpr unique_ptr() noexcept
  : _M_t()
-  { static_assert(!is_pointer::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.

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_default_constructible<_Dp>>>::value>;
+
   using pointer = typename _Ptr<_Tp, _Dp>::type;
 
   __uniq_ptr_impl() = default;
@@ -152,6 +155,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   template >
 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::value,
-		 "constructed with null function pointer deleter"); }
+  template
+	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::value,
-		 "constructed with null function pointer deleter"); }
+  template
+	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
+	constexpr unique_ptr(nullptr_t) noexcept : unique_ptr() { }
 
   // Move constructors.