LWG 2899 - Make is_move_constructible correct for unique_ptr
authorJonathan Wakely <jwakely@redhat.com>
Tue, 14 May 2019 11:17:11 +0000 (12:17 +0100)
committerJonathan Wakely <redi@gcc.gnu.org>
Tue, 14 May 2019 11:17:11 +0000 (12:17 +0100)
* include/bits/unique_ptr.h (__uniq_ptr_impl): Add move constructor,
move assignment operator.
(__uniq_ptr_impl::release(), __uniq_ptr_impl::reset(pointer)): Add.
(__uniq_ptr_data): New class template with conditionally deleted
special members.
(unique_ptr, unique_ptr<T[], D>): Change type of data member from
__uniq_ptr_impl<T, D> to __uniq_ptr_data<T, D>. Define move
constructor and move assignment operator as defaulted.
(unique_ptr::release(), unique_ptr<T[], D>::release()): Forward to
__uniq_ptr_impl::release().
(unique_ptr::reset(pointer), unique_ptr<T[], D>::reset<U>(U)): Forward
to __uniq_ptr_impl::reset(pointer).
* python/libstdcxx/v6/printers.py (UniquePointerPrinter.__init__):
Check for new __uniq_ptr_data type.
* testsuite/20_util/unique_ptr/dr2899.cc: New test.

From-SVN: r271158

libstdc++-v3/ChangeLog
libstdc++-v3/include/bits/unique_ptr.h
libstdc++-v3/python/libstdcxx/v6/printers.py
libstdc++-v3/testsuite/20_util/unique_ptr/dr2899.cc [new file with mode: 0644]

index c25f2ac76414e7ad95a09442c5d4ab73e0d326dc..440a1dfd80d786167cfa5d004c2dc2673db6be15 100644 (file)
@@ -1,3 +1,22 @@
+2019-05-14  Jonathan Wakely  <jwakely@redhat.com>
+
+       LWG 2899 - Make is_move_constructible correct for unique_ptr
+       * include/bits/unique_ptr.h (__uniq_ptr_impl): Add move constructor,
+       move assignment operator.
+       (__uniq_ptr_impl::release(), __uniq_ptr_impl::reset(pointer)): Add.
+       (__uniq_ptr_data): New class template with conditionally deleted
+       special members.
+       (unique_ptr, unique_ptr<T[], D>): Change type of data member from
+       __uniq_ptr_impl<T, D> to __uniq_ptr_data<T, D>. Define move
+       constructor and move assignment operator as defaulted.
+       (unique_ptr::release(), unique_ptr<T[], D>::release()): Forward to
+       __uniq_ptr_impl::release().
+       (unique_ptr::reset(pointer), unique_ptr<T[], D>::reset<U>(U)): Forward
+       to __uniq_ptr_impl::reset(pointer).
+       * python/libstdcxx/v6/printers.py (UniquePointerPrinter.__init__):
+       Check for new __uniq_ptr_data type.
+       * testsuite/20_util/unique_ptr/dr2899.cc: New test.
+
 2019-05-13  Jonathan Wakely  <jwakely@redhat.com>
 
        PR libstdc++/90454.cc path construction from void*
index a9e74725dfd1338c70ce04f5b58c87b819d9ce09..484c8b328e42e070b7ca1c0fe45122faa5d71646 100644 (file)
@@ -119,6 +119,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
   /// @cond undocumented
 
+  // Manages the pointer and deleter of a unique_ptr
   template <typename _Tp, typename _Dp>
     class __uniq_ptr_impl
     {
@@ -153,14 +154,75 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       __uniq_ptr_impl(pointer __p, _Del&& __d)
        : _M_t(__p, std::forward<_Del>(__d)) { }
 
+      __uniq_ptr_impl(__uniq_ptr_impl&& __u) noexcept
+      : _M_t(std::move(__u._M_t))
+      { __u._M_ptr() = nullptr; }
+
+      __uniq_ptr_impl& operator=(__uniq_ptr_impl&& __u) noexcept
+      {
+       reset(__u.release());
+       _M_deleter() = std::forward<_Dp>(__u._M_deleter());
+       return *this;
+      }
+
       pointer&   _M_ptr() { return std::get<0>(_M_t); }
       pointer    _M_ptr() const { return std::get<0>(_M_t); }
       _Dp&       _M_deleter() { return std::get<1>(_M_t); }
       const _Dp& _M_deleter() const { return std::get<1>(_M_t); }
 
+      void reset(pointer __p) noexcept
+      {
+       const pointer __old_p = _M_ptr();
+       _M_ptr() = __p;
+       if (__old_p)
+         _M_deleter()(__old_p);
+      }
+
+      pointer release() noexcept
+      {
+       pointer __p = _M_ptr();
+       _M_ptr() = nullptr;
+       return __p;
+      }
+
     private:
       tuple<pointer, _Dp> _M_t;
     };
+
+  // Defines move construction + assignment as either defaulted or deleted.
+  template <typename _Tp, typename _Dp,
+           bool = is_move_constructible<_Dp>::value,
+           bool = is_move_assignable<_Dp>::value>
+    struct __uniq_ptr_data : __uniq_ptr_impl<_Tp, _Dp>
+    {
+      using __uniq_ptr_impl<_Tp, _Dp>::__uniq_ptr_impl;
+      __uniq_ptr_data(__uniq_ptr_data&&) = default;
+      __uniq_ptr_data& operator=(__uniq_ptr_data&&) = default;
+    };
+
+  template <typename _Tp, typename _Dp>
+    struct __uniq_ptr_data<_Tp, _Dp, true, false> : __uniq_ptr_impl<_Tp, _Dp>
+    {
+      using __uniq_ptr_impl<_Tp, _Dp>::__uniq_ptr_impl;
+      __uniq_ptr_data(__uniq_ptr_data&&) = default;
+      __uniq_ptr_data& operator=(__uniq_ptr_data&&) = delete;
+    };
+
+  template <typename _Tp, typename _Dp>
+    struct __uniq_ptr_data<_Tp, _Dp, false, true> : __uniq_ptr_impl<_Tp, _Dp>
+    {
+      using __uniq_ptr_impl<_Tp, _Dp>::__uniq_ptr_impl;
+      __uniq_ptr_data(__uniq_ptr_data&&) = delete;
+      __uniq_ptr_data& operator=(__uniq_ptr_data&&) = default;
+    };
+
+  template <typename _Tp, typename _Dp>
+    struct __uniq_ptr_data<_Tp, _Dp, false, false> : __uniq_ptr_impl<_Tp, _Dp>
+    {
+      using __uniq_ptr_impl<_Tp, _Dp>::__uniq_ptr_impl;
+      __uniq_ptr_data(__uniq_ptr_data&&) = delete;
+      __uniq_ptr_data& operator=(__uniq_ptr_data&&) = delete;
+    };
   /// @endcond
 
   /// 20.7.1.2 unique_ptr for single objects.
@@ -171,7 +233,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
        using _DeleterConstraint =
          typename __uniq_ptr_impl<_Tp, _Up>::_DeleterConstraint::type;
 
-      __uniq_ptr_impl<_Tp, _Dp> _M_t;
+      __uniq_ptr_data<_Tp, _Dp> _M_t;
 
     public:
       using pointer      = typename __uniq_ptr_impl<_Tp, _Dp>::pointer;
@@ -255,8 +317,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       // Move constructors.
 
       /// Move constructor.
-      unique_ptr(unique_ptr&& __u) noexcept
-      : _M_t(__u.release(), std::forward<deleter_type>(__u.get_deleter())) { }
+      unique_ptr(unique_ptr&&) = default;
 
       /** @brief Converting constructor from another type
        *
@@ -298,24 +359,16 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
       /** @brief Move assignment operator.
        *
-       * @param __u  The object to transfer ownership from.
-       *
-       * Invokes the deleter first if this object owns a pointer.
+       * Invokes the deleter if this object owns a pointer.
        */
-      unique_ptr&
-      operator=(unique_ptr&& __u) noexcept
-      {
-       reset(__u.release());
-       get_deleter() = std::forward<deleter_type>(__u.get_deleter());
-       return *this;
-      }
+      unique_ptr& operator=(unique_ptr&&) = default;
 
       /** @brief Assignment from another type.
        *
        * @param __u  The object to transfer ownership from, which owns a
        *             convertible pointer to a non-array object.
        *
-       * Invokes the deleter first if this object owns a pointer.
+       * Invokes the deleter if this object owns a pointer.
        */
       template<typename _Up, typename _Ep>
         typename enable_if< __and_<
@@ -380,11 +433,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       /// Release ownership of any stored pointer.
       pointer
       release() noexcept
-      {
-       pointer __p = get();
-       _M_t._M_ptr() = pointer();
-       return __p;
-      }
+      { return _M_t.release(); }
 
       /** @brief Replace the stored pointer.
        *
@@ -397,10 +446,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       {
        static_assert(__is_invocable<deleter_type&, pointer>::value,
                      "unique_ptr's deleter must be invocable with a pointer");
-       using std::swap;
-       swap(_M_t._M_ptr(), __p);
-       if (__p != pointer())
-         get_deleter()(std::move(__p));
+       _M_t.reset(std::move(__p));
       }
 
       /// Exchange the pointer and deleter with another object.
@@ -427,7 +473,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       using _DeleterConstraint =
        typename __uniq_ptr_impl<_Tp, _Up>::_DeleterConstraint::type;
 
-      __uniq_ptr_impl<_Tp, _Dp> _M_t;
+      __uniq_ptr_data<_Tp, _Dp> _M_t;
 
       template<typename _Up>
        using __remove_cv = typename remove_cv<_Up>::type;
@@ -536,8 +582,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
                                 _DelUnref&&>) = delete;
 
       /// Move constructor.
-      unique_ptr(unique_ptr&& __u) noexcept
-      : _M_t(__u.release(), std::forward<deleter_type>(__u.get_deleter())) { }
+      unique_ptr(unique_ptr&&) = default;
 
       /// Creates a unique_ptr that owns nothing.
       template<typename _Del = _Dp, typename = _DeleterConstraint<_Del>>
@@ -564,24 +609,17 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
       /** @brief Move assignment operator.
        *
-       * @param __u  The object to transfer ownership from.
-       *
-       * Invokes the deleter first if this object owns a pointer.
+       * Invokes the deleter if this object owns a pointer.
        */
       unique_ptr&
-      operator=(unique_ptr&& __u) noexcept
-      {
-       reset(__u.release());
-       get_deleter() = std::forward<deleter_type>(__u.get_deleter());
-       return *this;
-      }
+      operator=(unique_ptr&&) = default;
 
       /** @brief Assignment from another type.
        *
        * @param __u  The object to transfer ownership from, which owns a
        *             convertible pointer to an array object.
        *
-       * Invokes the deleter first if this object owns a pointer.
+       * Invokes the deleter if this object owns a pointer.
        */
       template<typename _Up, typename _Ep>
        typename
@@ -638,11 +676,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       /// Release ownership of any stored pointer.
       pointer
       release() noexcept
-      {
-       pointer __p = get();
-       _M_t._M_ptr() = pointer();
-       return __p;
-      }
+      { return _M_t.release(); }
 
       /** @brief Replace the stored pointer.
        *
@@ -664,18 +698,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
                >>
       void
       reset(_Up __p) noexcept
-      {
-       pointer __ptr = __p;
-       using std::swap;
-       swap(_M_t._M_ptr(), __ptr);
-       if (__ptr != nullptr)
-         get_deleter()(__ptr);
-      }
+      { _M_t.reset(std::move(__p)); }
 
       void reset(nullptr_t = nullptr) noexcept
-      {
-        reset(pointer());
-      }
+      { reset(pointer()); }
 
       /// Exchange the pointer and deleter with another object.
       void
index 19d367295dfb64d67970f25a97bc7bf77971311d..eae06f93c345813efc8a80f965546b50324094c3 100644 (file)
@@ -182,7 +182,9 @@ class UniquePointerPrinter:
     def __init__ (self, typename, val):
         self.val = val
         impl_type = val.type.fields()[0].type.tag
-        if is_specialization_of(impl_type, '__uniq_ptr_impl'): # New implementation
+        # Check for new implementations first:
+        if is_specialization_of(impl_type, '__uniq_ptr_data') \
+            or is_specialization_of(impl_type, '__uniq_ptr_impl'):
             self.pointer = val['_M_t']['_M_t']['_M_head_impl']
         elif is_specialization_of(impl_type, 'tuple'):
             self.pointer = val['_M_t']['_M_head_impl']
diff --git a/libstdc++-v3/testsuite/20_util/unique_ptr/dr2899.cc b/libstdc++-v3/testsuite/20_util/unique_ptr/dr2899.cc
new file mode 100644 (file)
index 0000000..b528841
--- /dev/null
@@ -0,0 +1,54 @@
+// Copyright (C) 2019 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/>.
+
+// { dg-do compile { target c++11 } }
+
+#include <memory>
+
+struct Del {
+  Del() = default;
+  Del(Del&&) = delete;
+
+  void operator()(int*) const;
+};
+
+static_assert(!std::is_move_constructible<std::unique_ptr<int, Del>>::value);
+static_assert(std::is_move_constructible<std::unique_ptr<int, Del&>>::value);
+
+struct Del2 {
+  Del2() = default;
+  Del2(Del2&&) = default;
+  Del2& operator=(Del2&&) = delete;
+  Del2& operator=(const Del2&) = default;
+
+  void operator()(int*) const;
+};
+
+static_assert(!std::is_move_assignable<std::unique_ptr<int, Del2>>::value);
+static_assert(std::is_move_assignable<std::unique_ptr<int, Del2&>>::value);
+
+struct Del3 {
+  Del3() = default;
+  Del3(Del3&&) = default;
+  Del3& operator=(Del3&&) = default;
+  Del3& operator=(const Del3&) = delete;
+
+  void operator()(int*) const;
+};
+
+static_assert(std::is_move_assignable<std::unique_ptr<int, Del3>>::value);
+static_assert(!std::is_move_assignable<std::unique_ptr<int, Del3&>>::value);