Improve insert/emplace robustness to self insertion

[François Dumont]

On 29/06/2016 11:10, Jonathan Wakely wrote:
> On 28/06/16 21:59 +0200, François Dumont wrote:
>> @@ -303,16 +301,20 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
>>       emplace(const_iterator __position, _Args&&... __args)
>>       {
>>     const size_type __n = __position - begin();
>
> It looks like this should use __position - cbegin(), to avoid an
> implicit conversion from iterator to const_iterator, and ...
>
>> -    if (this->_M_impl._M_finish != this->_M_impl._M_end_of_storage
>> -        && __position == end())
>> -      {
>> -        _Alloc_traits::construct(this->_M_impl, 
>> this->_M_impl._M_finish,
>> -                     std::forward<_Args>(__args)...);
>> -        ++this->_M_impl._M_finish;
>> -      }
>> +    if (this->_M_impl._M_finish != this->_M_impl._M_end_of_storage)
>> +      if (__position == end())
>
> This could be __position == cend(), and ...
>
>> +        {
>> +          _Alloc_traits::construct(this->_M_impl, 
>> this->_M_impl._M_finish,
>> +                       std::forward<_Args>(__args)...);
>> +          ++this->_M_impl._M_finish;
>> +        }
>> +      else
>> +        _M_insert_aux(begin() + (__position - cbegin()),
>
> This could use __n, and ...
>
>> + std::forward<_Args>(__args)...);
>>     else
>> -      _M_insert_aux(begin() + (__position - cbegin()),
>> -            std::forward<_Args>(__args)...);
>> +      _M_realloc_insert_aux(begin() + (__position - cbegin()),
>
> This could also use __n.
>
>> + std::forward<_Args>(__args)...);
>> +
>>     return iterator(this->_M_impl._M_start + __n);
>>       }
>>
> .
>
I tried those changes too but started having failing tests in 
vector/ext_pointer so prefer to not touch that for the moment. I think 
the compilation error was coming from the change of begin() + 
(__position - cbegin()) into begin() + __n because of overloaded 
operator+. The 2 other changes should be fine for a future patch.

As asked here is now a patch to only fix the robustness issue. The 
consequence is that it is reverting the latest optimization as, without 
smart algo, we always need to do a copy to protect against insertion of 
values contained in the vector as shown by new tests.

François
-------------- next part --------------
A non-text attachment was scrubbed...
Name: vector_self_insert.patch
Type: text/x-patch
Size: 14995 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/libstdc++/attachments/20160629/652abe8c/attachment.bin>