This is the mail archive of the
libstdc++@gcc.gnu.org
mailing list for the libstdc++ project.
Re: Fix stable_sort to work on iterators returning rvalue
- From: Paolo Carlini <paolo dot carlini at oracle dot com>
- To: Chris Jefferson <chris at bubblescope dot net>
- Cc: François Dumont <frs dot dumont at gmail dot com>, "libstdc++ at gcc dot gnu dot org" <libstdc++ at gcc dot gnu dot org>
- Date: Mon, 28 May 2012 12:23:10 +0200
- Subject: Re: Fix stable_sort to work on iterators returning rvalue
- References: <4FBFEE9B.5080504@gmail.com> <4FC0035D.2010502@oracle.com> <4FC34FFE.9080502@bubblescope.net>
Hi,
On 05/28/2012 12:14 PM, Chris Jefferson wrote:
(Resending due to broken mail client).
On 25/05/12 23:10, Paolo Carlini wrote:
On 05/25/2012 10:42 PM, François Dumont wrote:
Hi
The issues I had when trying to use stable_sort with
move_iterator was in fact not all coming from a missing swap
overload. In fact stable_sort do not accept iterators with
dereference operator returning pure rvalue. I use
std::vector<bool>::iterator to illustrate this problem. The problem
was in fact coming from __uninitialized_construct_buf that was
taking a lvalue reference. I simply modify the function and the
underlying helper struct to dereference the iterator as late as
possible that is to say only when we need to pass it to the
std::move function.
...
Ok to commit ?
Patch looks pretty good to me. It would be great if the usual friends
of libstdc++ could also have a look, in particular Chris (if I
remember correctly he invented this stuff)
My main concern is one I have mentioned previously. I'm unsure all our
code works with things like move_iterator, even when it correctly
compiles. We previously took out internal uses of move_iterator,
because while the code compiled it did not work correctly. Look at bug
: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48038 for any example.
With this change, code which uses move_iterator, and operator< which
pass by value, will cause values to be 'eaten' during the sort. Now,
the standard in my opinion doesn't say this would be a bug, but it
certainly seems like it would be unpleasant.
How to fix this? The simplest way might be to wrap all predicates /
operator< in a wrapper which just takes lvalue references. Not sure if
it is worth going to that much pain just to fix this.
Hug. Thus, if I understand correctly the general idea, and considering
that nobody complained so far wrt the released versions of the library,
isn't this something that Francois could do at once with his work in
mainline about removing the duplicate algorithms? Or I misunderstand? I
would like to see either something neatly belonging to that work (and
not making it even more complex) or something maybe a bit ugly but safe
and restricted to stable_sort. Is it possible?
Thanks!
Paolo.