diff mbox

PR77990 refactor unique_ptr to encapsulate tuple

Message ID 20161019093455.GV2922@redhat.com
State New
Headers show

Commit Message

Jonathan Wakely Oct. 19, 2016, 9:34 a.m. UTC 
On 17/10/16 14:37 +0100, Jonathan Wakely wrote:
>We are incorrectly requiring unique_ptr deleters to be copyable here:

>

>     explicit

>     unique_ptr(pointer __p) noexcept

>     : _M_t(__p, deleter_type())

>     { }

>

>We could just do:

>

>     explicit

>     unique_ptr(pointer __p) noexcept

>     : _M_t()

>     { std::get<0>(_M_t) = __p; }

>

>But having to deal directly with the std::tuple inside unique_ptr has

>been bothering me for some time. The tuple is used so we get the empty

>base-class optimisation for the deleter, but that implementation

>detail

>

>This patch refactors unique_ptr to put the std::tuple member into a

>new type which provides named accessors for the tuple elements, so we

>can stop using get<0> and get<1>. That new type can also provide a

>single-argument constructor to fix the copyable requirement for

>deleters. This also removes the code for deducing the pointer type

>which is duplciated in unique_ptr and unique_ptr<T[], D>, and while in

>the neighbourhood I changed it from old-school SFINAE using overloaded

>functions to the new hotness with __void_t<>.

>

>I intend to commit this to trunk, but on the branches I'll just fix

>the constructor as shown above, as it's a smaller change.

>

>

>	PR libstdc++/77990

>	* include/bits/unique_ptr.h (__uniq_ptr_impl): New type to

>	encapsulate implementation details.

>	(unique_ptr::unique_ptr(_Up)): Don't copy deleter object.

>	(unique_ptr::get, unique_ptr::get_deleter, unique_ptr::release):

>	Call member functions of implementation object.

>	(unique_ptr<T[], D>): Likewise.

>	* python/libstdcxx/v6/printers.py (UniquePointerPrinter): Adjust for

>	new implementation.

>	* python/libstdcxx/v6/xmethods.py (UniquePtrGetWorker): Likewise.

>	* testsuite/20_util/unique_ptr/assign/48635_neg.cc: Adjust dg-error

>	lines.

>	* testsuite/20_util/unique_ptr/assign/cv_qual.cc: Likewise.

>	* testsuite/20_util/unique_ptr/cons/cv_qual.cc: Likewise.

>	* testsuite/20_util/unique_ptr/cons/77990.cc: New test.

>

>Tested powerpc64le-linux.


Committed to trunk. Here's the smaller patch I'm committing to the
branches.
diff mbox

Patch

commit 81ea53cd80fe7aaa3a323dba58bdb2259d672be3
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Wed Oct 19 10:21:58 2016 +0100

    PR77990 fix unique_ptr for non-copyable deleters
    
    	PR libstdc++/77990
    	* include/bits/unique_ptr.h (unique_ptr::unique_ptr(pointer)): Set
    	pointer member after value-initialization of tuple.
    	* testsuite/20_util/unique_ptr/assign/48635_neg.cc: Adjust dg-errors.
    	* testsuite/20_util/unique_ptr/cons/77990.cc: New test.
    	* testsuite/20_util/unique_ptr/cons/cv_qual.cc: Adjust dg-error.

diff --git a/libstdc++-v3/include/bits/unique_ptr.h b/libstdc++-v3/include/bits/unique_ptr.h
index c4a6d2f..8e30287 100644
--- a/libstdc++-v3/include/bits/unique_ptr.h
+++ b/libstdc++-v3/include/bits/unique_ptr.h
@@ -168,9 +168,12 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
        */
       explicit
       unique_ptr(pointer __p) noexcept
-      : _M_t(__p, deleter_type())
-      { static_assert(!is_pointer<deleter_type>::value,
-		     "constructed with null function pointer deleter"); }
+      : _M_t()
+      {
+	std::get<0>(_M_t) = __p;
+	static_assert(!is_pointer<deleter_type>::value,
+		     "constructed with null function pointer deleter");
+      }
 
       /** Takes ownership of a pointer.
        *
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 16ff8ae..785f9ad 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
@@ -43,10 +43,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 *-*-* } 269 }
+// { dg-error "no type" "" { target *-*-* } 272 }
 
   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 *-*-* } 537 }
+// { dg-error "no type" "" { target *-*-* } 540 }
 }
diff --git a/libstdc++-v3/testsuite/20_util/unique_ptr/cons/77990.cc b/libstdc++-v3/testsuite/20_util/unique_ptr/cons/77990.cc
new file mode 100644
index 0000000..1acc313
--- /dev/null
+++ b/libstdc++-v3/testsuite/20_util/unique_ptr/cons/77990.cc
@@ -0,0 +1,28 @@ 
+// 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/>.
+
+// { dg-options "-std=gnu++11" }
+// { dg-do compile }
+
+#include <memory>
+
+struct D {
+  D() = default;
+  D(const D&) = delete;
+  void operator()(int*);
+};
+std::unique_ptr<int, D> p((int*)nullptr);  // PR libstdc++/77990
diff --git a/libstdc++-v3/testsuite/20_util/unique_ptr/cons/cv_qual.cc b/libstdc++-v3/testsuite/20_util/unique_ptr/cons/cv_qual.cc
index 078ecbb..0f69029 100644
--- a/libstdc++-v3/testsuite/20_util/unique_ptr/cons/cv_qual.cc
+++ b/libstdc++-v3/testsuite/20_util/unique_ptr/cons/cv_qual.cc
@@ -106,7 +106,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 *-*-* } 445 }
+  // { dg-error "no type" "" { target *-*-* } 448 }
 }
 
 template<typename T>