Improve insert/emplace robustness to self insertion

François Dumont frs.dumont@gmail.com
Sat Jul 2 06:38:00 GMT 2016


On 01/07/2016 11:54, Jonathan Wakely wrote:
> On 30/06/16 21:51 +0200, François Dumont wrote:
>> On 29/06/2016 23:30, Jonathan Wakely wrote:
>>>
>>>     iterator
>>>     insert(const_iterator __position, value_type&& __x)
>>>     { return emplace(__position, std::move(__x)); }
>>>
>>> That's suboptimal, since in the general case we need an extra
>>> construction for emplacing, but we know that we don't need to do that
>>> when inserting rvalues.
>>
>>    Why not ? I realized with your remarks that I was missing some 
>> tests in the new self_insert.cc. The ones to insert an rvalue coming 
>> from the vector itself. In the attached patch there is those 2 tests, 
>> do you agree with expected behavior ? For the moment it doesn't check 
>> that the source value has been indeed moved cause it doesn't, I will 
>> update it once it does.
>
> No, I don't agree, because this is undefined behaviour:
>
>   vv.insert(vv.begin(), std::move(vv[0]));
>
> We don't need to support that case.

Ok but management of this kind of code is a nice consequence of using 
the smart insertion trick.

>
> 17.6.4.9 [res.on.arguments] says:
>
> — If a function argument binds to an rvalue reference parameter, the
>  implementation may assume that this parameter is a unique reference
>  to this argument.
>
> i.e. when passed an rvalue we can assume it is not a reference to
> something in the container.
>
> That's why we should not perform any more operations when inserting
> rvalues than we do now. Any increase in copies/moves for inserting
> rvalues is a regression, and should be avoided

Agree so in attached patch I have implemented the smart insertion trick 
to come back to optimal copies/moves. We don't need to do much to do 
better than Standard requirement and especially not additional copies/moves.

I haven't consider in this patch your remark about using allocator to 
build instance so don't hesitate to commit what you want and I will rebase.

François

-------------- next part --------------
A non-text attachment was scrubbed...
Name: vector_optim.patch
Type: text/x-patch
Size: 7687 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/libstdc++/attachments/20160702/f69ebc99/attachment.bin>


More information about the Libstdc++ mailing list