This is the mail archive of the libstdc++@gcc.gnu.org mailing list for the libstdc++ project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] PR77990 refactor unique_ptr to encapsulate tuple


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.


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>

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]