The following program using 4.7.0 20110409 (experimental) should print "copy", but it prints "move" instead: //------------- #include <memory> #include <utility> #include <iostream> struct Deleter { Deleter() = default; Deleter(const Deleter&) = default; Deleter(Deleter&&) = default; Deleter& operator=(const Deleter&) { std::cout << "copy" << std::endl; return *this; } Deleter& operator=(Deleter&&) { std::cout << "move" << std::endl; return *this; } template<class T> void operator()(T* p) const { delete p; } }; int main() { Deleter d1, d2; std::unique_ptr<int, Deleter&> p1(new int, d1), p2(nullptr, d2); p2 = std::move(p1); } //------------- The reason for the failure is, that the library implementation of the move-assignment operator of std::unique_ptr (and of it's templated variant) uses std::move to transfer the deleter from the source to the target, but as of [unique.ptr.single.asgn] p. 2 and p. 6 it shall use std::forward instead. Doing it correctly will ensure that the deleter is copied and not moved in this case, which is an intended design aim of std::unique_ptr with deleters that are lvalue-references. The necessary fix is to replace the usage of std::move at the two places by std::forward<deleter_type>.
(In reply to comment #0) The exactly same problem exists for the specialization std::unique_ptr<T[], D&>.
Thanks, Daniel. I'll fix this momentarily.
Author: paolo Date: Sat Apr 16 00:55:43 2011 New Revision: 172532 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=172532 Log: 2011-04-15 Daniel Krugler <daniel.kruegler@googlemail.com> Paolo Carlini <paolo.carlini@oracle.com> PR libstdc++/48635 * include/bits/unique_ptr.h (unique_ptr<>::operator=(unique_ptr&&), unique_ptr<>::operator=(unique_ptr<>&&), unique_ptr<_Tp[],>::operator=(unique_ptr&&), unique_ptr<_Tp[],>::operator=(unique_ptr<>&&)): Forward the deleter instead of moving it. * testsuite/20_util/unique_ptr/assign/48635.cc: New. Added: trunk/libstdc++-v3/testsuite/20_util/unique_ptr/assign/48635.cc Modified: trunk/libstdc++-v3/ChangeLog trunk/libstdc++-v3/include/bits/unique_ptr.h
Author: paolo Date: Sat Apr 16 00:55:53 2011 New Revision: 172533 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=172533 Log: 2011-04-15 Daniel Krugler <daniel.kruegler@googlemail.com> Paolo Carlini <paolo.carlini@oracle.com> PR libstdc++/48635 * include/bits/unique_ptr.h (unique_ptr<>::operator=(unique_ptr&&), unique_ptr<>::operator=(unique_ptr<>&&), unique_ptr<_Tp[],>::operator=(unique_ptr&&), unique_ptr<_Tp[],>::operator=(unique_ptr<>&&)): Forward the deleter instead of moving it. * testsuite/20_util/unique_ptr/assign/48635.cc: New. Added: branches/gcc-4_6-branch/libstdc++-v3/testsuite/20_util/unique_ptr/assign/48635.cc Modified: branches/gcc-4_6-branch/libstdc++-v3/ChangeLog branches/gcc-4_6-branch/libstdc++-v3/include/bits/unique_ptr.h
Done. Fixed mainline and 4_6-branch.
(In reply to comment #5) > Done. Fixed mainline and 4_6-branch. Looks good, thanks, but I just recognize that there is a library issue in regard to the template version. The FDIS correctly applied the corresponding proposal, but for some reason that is unclear, [unique.ptr.single.asgn] p. 6 describes the effects as "std::forward<D>(u.get_deleter())". I verified with discussion with Howard that this should have been "std::forward<E>(u.get_deleter())" instead, as it was suggested in http://www.open-std.org/jtc1/sc22/wg21/docs/lwg-defects.html#983
Ok... Do we have testcases for that? By the way, I noticed that in the templated *constructor* we have been using D instead of E in the forward, and that doesn't seem correct vs the FDIS itself. What do you think? The below regtests fine: Index: include/bits/unique_ptr.h =================================================================== --- include/bits/unique_ptr.h (revision 172616) +++ include/bits/unique_ptr.h (working copy) @@ -153,7 +153,7 @@ && std::is_convertible<_Ep, _Dp>::value))> ::type> unique_ptr(unique_ptr<_Up, _Ep>&& __u) - : _M_t(__u.release(), std::forward<deleter_type>(__u.get_deleter())) + : _M_t(__u.release(), std::forward<_Ep>(__u.get_deleter())) { } #if _GLIBCXX_USE_DEPRECATED @@ -186,7 +186,7 @@ operator=(unique_ptr<_Up, _Ep>&& __u) { reset(__u.release()); - get_deleter() = std::forward<deleter_type>(__u.get_deleter()); + get_deleter() = std::forward<_Ep>(__u.get_deleter()); return *this; } @@ -306,7 +306,7 @@ template<typename _Up, typename _Ep> unique_ptr(unique_ptr<_Up, _Ep>&& __u) - : _M_t(__u.release(), std::forward<deleter_type>(__u.get_deleter())) + : _M_t(__u.release(), std::forward<_Ep>(__u.get_deleter())) { } // Destructor. @@ -326,7 +326,7 @@ operator=(unique_ptr<_Up, _Ep>&& __u) { reset(__u.release()); - get_deleter() = std::forward<deleter_type>(__u.get_deleter()); + get_deleter() = std::forward<_Ep>(__u.get_deleter()); return *this; }
(In reply to comment #7) > Ok... Do we have testcases for that? I have two test cases for the *assignment* situation, I invented them out of my head and I hope the code below does not too many typos and thinkos: 1) This program should be well-formed according to FDIS wording, but ill-formed according to the intended wording: #include <memory> #include <utility> struct D; struct B { B& operator=(D&) = delete; template<class T> void operator()(T*) const {} }; struct D : B {}; int main() { B b; D d; std::unique_ptr<void, B&> ub(nullptr, b); std::unique_ptr<void, D&> ud(nullptr, d); ub = std::move(ud); // Should be rejected } 2) The same scenario here: struct D2; struct B2 { B2& operator=(D2&) = delete; template<class T> void operator()(T*) const {} }; struct D2 { B2 b; operator B2&() { return b; } template<class T> void operator()(T*) const {} }; int main() { B2 b; D2 d; std::unique_ptr<void, B2&> ub(nullptr, b); std::unique_ptr<void, D2&> ud(nullptr, d); ub = std::move(ud); // Should be rejected } > By the way, I noticed that in the templated *constructor* we have been using D > instead of E in the forward, and that doesn't seem correct vs the FDIS itself. > What do you think? The below regtests fine: I agree that the suggested fixes are necessary and they look correct to me.
Author: paolo Date: Sun Apr 17 21:46:11 2011 New Revision: 172619 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=172619 Log: 2011-04-17 Daniel Krugler <daniel.kruegler@googlemail.com> Paolo Carlini <paolo.carlini@oracle.com> PR libstdc++/48635 (again) * include/bits/unique_ptr.h (unique_ptr<>::unique_ptr(unique_ptr<>&&), unique_ptr<_Tp[]>::unique_ptr(unique_ptr<>&&), unique_ptr<>::operator=(unique_ptr<>&&), unique_ptr<_Tp[]>::operator=(unique_ptr<>&&)): Use forward<_Ep>, not forward<_Dp>, to forward the deleter. * testsuite/20_util/unique_ptr/assign/48635_neg.cc: New. Added: trunk/libstdc++-v3/testsuite/20_util/unique_ptr/assign/48635_neg.cc Modified: trunk/libstdc++-v3/ChangeLog trunk/libstdc++-v3/include/bits/unique_ptr.h
Author: paolo Date: Sun Apr 17 21:46:20 2011 New Revision: 172620 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=172620 Log: 2011-04-17 Daniel Krugler <daniel.kruegler@googlemail.com> Paolo Carlini <paolo.carlini@oracle.com> PR libstdc++/48635 (again) * include/bits/unique_ptr.h (unique_ptr<>::unique_ptr(unique_ptr<>&&), unique_ptr<_Tp[]>::unique_ptr(unique_ptr<>&&), unique_ptr<>::operator=(unique_ptr<>&&), unique_ptr<_Tp[]>::operator=(unique_ptr<>&&)): Use forward<_Ep>, not forward<_Dp>, to forward the deleter. * testsuite/20_util/unique_ptr/assign/48635_neg.cc: New. Added: branches/gcc-4_6-branch/libstdc++-v3/testsuite/20_util/unique_ptr/assign/48635_neg.cc Modified: branches/gcc-4_6-branch/libstdc++-v3/ChangeLog branches/gcc-4_6-branch/libstdc++-v3/include/bits/unique_ptr.h