This is the mail archive of the
mailing list for the libstdc++ project.
Re: Fix stable_sort to work on iterators returning rvalue
- From: Christopher Jefferson <chris at bubblescope dot net>
- To: François Dumont <frs dot dumont at gmail dot com>
- Cc: "libstdc++ at gcc dot gnu dot org" <libstdc++ at gcc dot gnu dot org>
- Date: Mon, 28 May 2012 20:59:22 +0100
- Subject: Re: Fix stable_sort to work on iterators returning rvalue
- References: <4FC36F8F.email@example.com> <4FC3CB5F.firstname.lastname@example.org>
On 28 May 2012, at 20:00, François Dumont wrote:
> On 05/28/2012 12:11 PM, Christopher Jefferson wrote:
>> 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.
>> At the very least, I would want to see a bunch of tests which make sure we don't have any other code which accidentally 'eats' users data. Make sure you catch all the various bits of sort, some of which get triggered rarely. I realise that is putting other people to a higher standard than I put myself, but we have learnt this is a tricky area :)
> Does it mean that you refuse the patch ?
> The patch purpose is not to make the code compatible with move_iterator but to make it compatible with iterator types that return pure rvalue through their dereference operator. As a side effect std::move_iterator are going to compile too which might be bad but is it really a reason to forbid other kind of iterators ?
> Of course we should find a good way to handle move_iterator, and I can spend some time on it, but I think that it should be the subject of a dedicated patch.
Sorry, just to clarify (also, I have been having problems with my mail client).
I believe this patch is good, I withdraw any complaint.
I believe there is a serious issue with move_iterator in use with almost all standard libraries, but it is disconnected from this patch.