public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug libstdc++/48635] New: [C++0x] unique_ptr<T, D&> moves the deleter instead of copying it
@ 2011-04-15 21:01 daniel.kruegler at googlemail dot com
2011-04-15 21:46 ` [Bug libstdc++/48635] " daniel.kruegler at googlemail dot com
` (9 more replies)
0 siblings, 10 replies; 11+ messages in thread
From: daniel.kruegler at googlemail dot com @ 2011-04-15 21:01 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48635
Summary: [C++0x] unique_ptr<T, D&> moves the deleter instead of
copying it
Product: gcc
Version: 4.7.0
Status: UNCONFIRMED
Severity: normal
Priority: P3
Component: libstdc++
AssignedTo: unassigned@gcc.gnu.org
ReportedBy: daniel.kruegler@googlemail.com
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>.
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Bug libstdc++/48635] [C++0x] unique_ptr<T, D&> moves the deleter instead of copying it
2011-04-15 21:01 [Bug libstdc++/48635] New: [C++0x] unique_ptr<T, D&> moves the deleter instead of copying it daniel.kruegler at googlemail dot com
@ 2011-04-15 21:46 ` daniel.kruegler at googlemail dot com
2011-04-16 0:25 ` paolo.carlini at oracle dot com
` (8 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: daniel.kruegler at googlemail dot com @ 2011-04-15 21:46 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48635
--- Comment #1 from Daniel Krügler <daniel.kruegler at googlemail dot com> 2011-04-15 21:44:56 UTC ---
(In reply to comment #0)
The exactly same problem exists for the specialization std::unique_ptr<T[],
D&>.
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Bug libstdc++/48635] [C++0x] unique_ptr<T, D&> moves the deleter instead of copying it
2011-04-15 21:01 [Bug libstdc++/48635] New: [C++0x] unique_ptr<T, D&> moves the deleter instead of copying it daniel.kruegler at googlemail dot com
2011-04-15 21:46 ` [Bug libstdc++/48635] " daniel.kruegler at googlemail dot com
@ 2011-04-16 0:25 ` paolo.carlini at oracle dot com
2011-04-16 0:58 ` paolo at gcc dot gnu.org
` (7 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: paolo.carlini at oracle dot com @ 2011-04-16 0:25 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48635
Paolo Carlini <paolo.carlini at oracle dot com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|UNCONFIRMED |NEW
Last reconfirmed| |2011.04.16 00:25:26
AssignedTo|unassigned at gcc dot |paolo.carlini at oracle dot
|gnu.org |com
Target Milestone|--- |4.6.1
Ever Confirmed|0 |1
--- Comment #2 from Paolo Carlini <paolo.carlini at oracle dot com> 2011-04-16 00:25:26 UTC ---
Thanks, Daniel. I'll fix this momentarily.
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Bug libstdc++/48635] [C++0x] unique_ptr<T, D&> moves the deleter instead of copying it
2011-04-15 21:01 [Bug libstdc++/48635] New: [C++0x] unique_ptr<T, D&> moves the deleter instead of copying it daniel.kruegler at googlemail dot com
2011-04-15 21:46 ` [Bug libstdc++/48635] " daniel.kruegler at googlemail dot com
2011-04-16 0:25 ` paolo.carlini at oracle dot com
@ 2011-04-16 0:58 ` paolo at gcc dot gnu.org
2011-04-16 1:01 ` paolo.carlini at oracle dot com
` (6 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: paolo at gcc dot gnu.org @ 2011-04-16 0:58 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48635
--- Comment #3 from paolo at gcc dot gnu.org <paolo at gcc dot gnu.org> 2011-04-16 00:55:49 UTC ---
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Bug libstdc++/48635] [C++0x] unique_ptr<T, D&> moves the deleter instead of copying it
2011-04-15 21:01 [Bug libstdc++/48635] New: [C++0x] unique_ptr<T, D&> moves the deleter instead of copying it daniel.kruegler at googlemail dot com
` (2 preceding siblings ...)
2011-04-16 0:58 ` paolo at gcc dot gnu.org
@ 2011-04-16 1:01 ` paolo.carlini at oracle dot com
2011-04-16 1:01 ` paolo at gcc dot gnu.org
` (5 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: paolo.carlini at oracle dot com @ 2011-04-16 1:01 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48635
Paolo Carlini <paolo.carlini at oracle dot com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|NEW |RESOLVED
Resolution| |FIXED
--- Comment #5 from Paolo Carlini <paolo.carlini at oracle dot com> 2011-04-16 00:56:57 UTC ---
Done. Fixed mainline and 4_6-branch.
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Bug libstdc++/48635] [C++0x] unique_ptr<T, D&> moves the deleter instead of copying it
2011-04-15 21:01 [Bug libstdc++/48635] New: [C++0x] unique_ptr<T, D&> moves the deleter instead of copying it daniel.kruegler at googlemail dot com
` (3 preceding siblings ...)
2011-04-16 1:01 ` paolo.carlini at oracle dot com
@ 2011-04-16 1:01 ` paolo at gcc dot gnu.org
2011-04-17 19:18 ` daniel.kruegler at googlemail dot com
` (4 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: paolo at gcc dot gnu.org @ 2011-04-16 1:01 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48635
--- Comment #4 from paolo at gcc dot gnu.org <paolo at gcc dot gnu.org> 2011-04-16 00:55:59 UTC ---
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Bug libstdc++/48635] [C++0x] unique_ptr<T, D&> moves the deleter instead of copying it
2011-04-15 21:01 [Bug libstdc++/48635] New: [C++0x] unique_ptr<T, D&> moves the deleter instead of copying it daniel.kruegler at googlemail dot com
` (4 preceding siblings ...)
2011-04-16 1:01 ` paolo at gcc dot gnu.org
@ 2011-04-17 19:18 ` daniel.kruegler at googlemail dot com
2011-04-17 19:45 ` paolo.carlini at oracle dot com
` (3 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: daniel.kruegler at googlemail dot com @ 2011-04-17 19:18 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48635
--- Comment #6 from Daniel Krügler <daniel.kruegler at googlemail dot com> 2011-04-17 19:18:01 UTC ---
(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
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Bug libstdc++/48635] [C++0x] unique_ptr<T, D&> moves the deleter instead of copying it
2011-04-15 21:01 [Bug libstdc++/48635] New: [C++0x] unique_ptr<T, D&> moves the deleter instead of copying it daniel.kruegler at googlemail dot com
` (5 preceding siblings ...)
2011-04-17 19:18 ` daniel.kruegler at googlemail dot com
@ 2011-04-17 19:45 ` paolo.carlini at oracle dot com
2011-04-17 20:12 ` daniel.kruegler at googlemail dot com
` (2 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: paolo.carlini at oracle dot com @ 2011-04-17 19:45 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48635
--- Comment #7 from Paolo Carlini <paolo.carlini at oracle dot com> 2011-04-17 19:44:51 UTC ---
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;
}
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Bug libstdc++/48635] [C++0x] unique_ptr<T, D&> moves the deleter instead of copying it
2011-04-15 21:01 [Bug libstdc++/48635] New: [C++0x] unique_ptr<T, D&> moves the deleter instead of copying it daniel.kruegler at googlemail dot com
` (6 preceding siblings ...)
2011-04-17 19:45 ` paolo.carlini at oracle dot com
@ 2011-04-17 20:12 ` daniel.kruegler at googlemail dot com
2011-04-17 21:46 ` paolo at gcc dot gnu.org
2011-04-17 21:47 ` paolo at gcc dot gnu.org
9 siblings, 0 replies; 11+ messages in thread
From: daniel.kruegler at googlemail dot com @ 2011-04-17 20:12 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48635
--- Comment #8 from Daniel Krügler <daniel.kruegler at googlemail dot com> 2011-04-17 20:12:09 UTC ---
(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.
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Bug libstdc++/48635] [C++0x] unique_ptr<T, D&> moves the deleter instead of copying it
2011-04-15 21:01 [Bug libstdc++/48635] New: [C++0x] unique_ptr<T, D&> moves the deleter instead of copying it daniel.kruegler at googlemail dot com
` (7 preceding siblings ...)
2011-04-17 20:12 ` daniel.kruegler at googlemail dot com
@ 2011-04-17 21:46 ` paolo at gcc dot gnu.org
2011-04-17 21:47 ` paolo at gcc dot gnu.org
9 siblings, 0 replies; 11+ messages in thread
From: paolo at gcc dot gnu.org @ 2011-04-17 21:46 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48635
--- Comment #9 from paolo at gcc dot gnu.org <paolo at gcc dot gnu.org> 2011-04-17 21:46:15 UTC ---
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Bug libstdc++/48635] [C++0x] unique_ptr<T, D&> moves the deleter instead of copying it
2011-04-15 21:01 [Bug libstdc++/48635] New: [C++0x] unique_ptr<T, D&> moves the deleter instead of copying it daniel.kruegler at googlemail dot com
` (8 preceding siblings ...)
2011-04-17 21:46 ` paolo at gcc dot gnu.org
@ 2011-04-17 21:47 ` paolo at gcc dot gnu.org
9 siblings, 0 replies; 11+ messages in thread
From: paolo at gcc dot gnu.org @ 2011-04-17 21:47 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48635
--- Comment #10 from paolo at gcc dot gnu.org <paolo at gcc dot gnu.org> 2011-04-17 21:46:23 UTC ---
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
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2011-04-17 21:47 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-04-15 21:01 [Bug libstdc++/48635] New: [C++0x] unique_ptr<T, D&> moves the deleter instead of copying it daniel.kruegler at googlemail dot com
2011-04-15 21:46 ` [Bug libstdc++/48635] " daniel.kruegler at googlemail dot com
2011-04-16 0:25 ` paolo.carlini at oracle dot com
2011-04-16 0:58 ` paolo at gcc dot gnu.org
2011-04-16 1:01 ` paolo.carlini at oracle dot com
2011-04-16 1:01 ` paolo at gcc dot gnu.org
2011-04-17 19:18 ` daniel.kruegler at googlemail dot com
2011-04-17 19:45 ` paolo.carlini at oracle dot com
2011-04-17 20:12 ` daniel.kruegler at googlemail dot com
2011-04-17 21:46 ` paolo at gcc dot gnu.org
2011-04-17 21:47 ` paolo at gcc dot gnu.org
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).