vector lightweight debug mode

[Jonathan Wakely]

On 16/09/15 21:37 +0200, François Dumont wrote:
>On 14/09/2015 21:50, Jonathan Wakely wrote:
>> On 14/09/15 20:27 +0200, François Dumont wrote:
>>> diff --git a/libstdc++-v3/include/bits/stl_vector.h
>>> b/libstdc++-v3/include/bits/stl_vector.h
>>> index 305d446..89a9aec 100644
>>> --- a/libstdc++-v3/include/bits/stl_vector.h
>>> +++ b/libstdc++-v3/include/bits/stl_vector.h
>>> @@ -449,6 +449,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
>>>       vector&
>>>       operator=(vector&& __x) noexcept(_Alloc_traits::_S_nothrow_move())
>>>       {
>>> +    __glibcxx_assert(this != &__x);
>>
>> Please don't do this, it fails in valid programs. The standard needs
>> to be fixed in this regard.
>
>The debug mode check should be removed too then.

Yes.


>>
>>>         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?


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