Re: vector lightweight debug mode

[FranÃois Dumont <frs dot dumont at gmail dot com>]

On 16/09/2015 22:29, Jonathan Wakely wrote:
>
>>>
>>>>         constexpr bool __move_storage =
>>>>           _Alloc_traits::_S_propagate_on_move_assign()
>>>>           || _Alloc_traits::_S_always_equal();
>>>> @@ -778,7 +779,10 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
>>>>        */
>>>>       reference
>>>>       operator[](size_type __n) _GLIBCXX_NOEXCEPT
>>>> -      { return *(this->_M_impl._M_start + __n); }
>>>> +      {
>>>> +    __glibcxx_assert(__n < size());
>>>> +    return *(this->_M_impl._M_start + __n);
>>>> +      }
>>>
>>> This could use __glibcxx_requires_subscript(__n), see the attached
>>> patch.
>>
>>    I thought you didn't want to use anything from debug.h so I try to
>> do with only __glibcxx_assert coming from c++config. I think your patch
>> is missing include of debug.h.
>>
>>    But I was going to propose to use _Error_formatter also in this
>> mode, I do not see any reason to not do so. The attached patch does just
>> that.
>
> That pulls in extra dependencies on I/O and fprintf and things, which
> can cause code size to increase. Is it really worth it?

Not that much dependencies. We only need formatters.h in this mode which
has the following common includes:

#include <bits/c++config.h>
#include <bits/cpp_type_traits.h>

and if rtti is enabled the less common:

#include <typeinfo>

We would just leverage on the good job done to diagnose problems.

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

(Written before Christopher reply:)

At least program will compile only if iterator is coming from a vector
of the same type. So behavior is undefined only if user pass an invalid
iterator which is exactly what this check tries to detect, isn't it
paradoxical ? If this undefined behavior results in the program abortion
this is what should happen anyway. If it doesn't abort then the program
will definitely not behaves as expected so this check doesn't make
anything worst, no ?

François