On 01/07/16 10:54 +0100, 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.
>
>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.


Here's what I'm planning to commit soon. This also fixes a problem
where the temporaries we create are not constructed+destroyed with the
allocator, which they must be.