On 17 September 2015 at 21:52, Christopher Jefferson
<ch...@bubblescope.net> wrote:
> ---------- Forwarded message ----------
> From: Christopher Jefferson <ch...@bubblescope.net>
> Date: 17 September 2015 at 18:59
> Subject: Re: vector lightweight debug mode
> To: Jonathan Wakely <jwak...@redhat.com>
>
>
> On 16 September 2015 at 21:29, Jonathan Wakely <jwak...@redhat.com> wrote:
>> On 16/09/15 21:37 +0200, François Dumont wrote:
>>
>>>>> @@ -1051,6 +1071,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
>>>>> iterator
>>>>> insert(const_iterator __position, size_type __n, const
>>>>> value_type& __x)
>>>>> {
>>>>> + __glibcxx_assert(__position >= cbegin() && __position <= cend());
>>>>> difference_type __offset = __position - cbegin();
>>>>> _M_fill_insert(begin() + __offset, __n, __x);
>>>>> return begin() + __offset;
>>>>
>>>>
>>>> This is undefined behaviour, so I'd rather not add this check (I know
>>>> it's on the google branch, but it's still undefined behaviour).
>>>
>>>
>>> Why ? Because of the >= operator usage ? Is the attached patch better ?
>>> < and == operators are well defined for a random access iterator, no ?
>>
>>
>> No, because it is undefined to compare iterators that belong to
>> different containers, or to compare pointers that point to different
>> arrays.
>
> While that's true, on the other hand it's defined behaviour when the
> assert passes, and in the case where the thing it's trying to check
> fails, we are off into undefined-land anyway.
>
> A defined check would be to check if __offset is < 0 or > size(). Once
> again if it's false we are undefined, but the assert line itself is
> then defined behaviour.
That's a good point, but it still means an optimiser could remove the
checks, because it is impossible for them to fail in a correct
program.
That would be no worse than not having the checks at all, but it could
make them unreliable.
