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: Fix stable_sort to work on iterators returning rvalue


(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.

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 :)

Chris

Thanks, Paolo.

PS: as a very minor nit, remember the bool test variable in test01.


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