Extend usage of C++11 direct init in __debug::vector

[Jonathan Wakely]

On 16/10/18 10:20 +0100, Jonathan Wakely wrote:
>On 15/10/18 22:45 +0200, François Dumont wrote:
>>On 10/15/2018 12:10 PM, Jonathan Wakely wrote:
>>>On 15/10/18 07:23 +0200, François Dumont wrote:
>>>>This patch extend usage of C++11 direct initialization in 
>>>>__debug::vector and makes some calls to operator - more 
>>>>consistent.
>>>>
>>>>Note that I also rewrote following expression in erase method:
>>>>
>>>>-Â Â Â  Â  return begin() + (__first.base() - cbegin().base());
>>>>+Â Â Â  Â  return { _Base::begin() + (__first.base() - 
>>>>_Base::cbegin()), this };
>>>>
>>>>The latter version was building 2 safe iterators and 
>>>>incrementing 1 with the additional debug check inherent to such 
>>>>an operation whereas the new version just build 1 safe iterator 
>>>>with directly the expected offset.
>>>
>>>Makes sense.
>>>
>>>>2018-10-15  François Dumont <fdumont@gcc.gnu.org>
>>>>
>>>>Â Â Â  * include/debug/vector (vector<>::cbegin()): Use C++11 direct
>>>>Â Â Â  initialization.
>>>>Â Â Â  (vector<>::cend()): Likewise.
>>>>Â Â Â  (vector<>::emplace(const_iterator, _Args&&...)): Likewise and use
>>>>Â Â Â  consistent iterator comparison.
>>>>Â Â Â  (vector<>::insert(const_iterator, size_type, const _Tp&)): Likewise.
>>>>Â Â Â  (vector<>::insert(const_iterator, _InputIterator, _InputIterator)):
>>>>Â Â Â  Likewise.
>>>>Â Â Â  (vector<>::erase(const_iterator)): Likewise.
>>>>Â Â Â  (vector<>::erase(const_iterator, const_iterator)): Likewise.
>>>>
>>>>Tested under Linux x86_64 Debug mode and committed.
>>>>
>>>>François
>>>>
>>>
>>>>@@ -542,7 +542,8 @@ namespace __debug
>>>>Â Â Â Â Â  {
>>>>Â Â Â Â __glibcxx_check_insert(__position);
>>>>Â Â Â Â bool __realloc = this->_M_requires_reallocation(this->size() + 1);
>>>>-Â Â Â  difference_type __offset = __position.base() - _Base::begin();
>>>>+Â Â Â  difference_type __offset
>>>>+Â Â Â Â Â  = __position.base() - 
>>>>__position._M_get_sequence()->_M_base().begin();
>>>
>>>What's the reason for this change?
>>>
>>>Doesn't __glibcxx_check_insert(__position) already ensure that
>>>__position is attached to *this, and so _Base::begin() returns the
>>>same thing as __position._M_get_sequence()->_M_base().begin() ?
>>>
>>>If they're equivalent, the original code seems more readable.
>>>
>>>
>>>
>>This is the consistent iterator comparison part. Depending on C++ 
>>mode __position can be iterator or const_iterator.
>>
>>As _M_get_sequence() return type depends on iterator type we always 
>>have iterator - iterator or const_iterator - const_iterator so no 
>>conversion.
>
>
>It used to work without a conversion. It only involves a conversion
>because you removed this operator a few weeks ago:
>
>-  template<typename _IteratorL, typename _IteratorR, typename _Sequence>
>-    inline typename _Safe_iterator<_IteratorL, _Sequence,
>-                       std::random_access_iterator_tag>::difference_type
>-    operator-(const _Safe_iterator<_IteratorL, _Sequence,
>-                                  std::random_access_iterator_tag>& __lhs,
>-             const _Safe_iterator<_IteratorR, _Sequence,
>-                                  std::random_access_iterator_tag>& __rhs)
>
>So now there are extra conversions, and you're obfuscating the code to
>avoid them.
>
>Either the conversions are harmless and we don't need the operator, or
>they're too expensive and we should keep the operator.

Oh, I got confused, the operation is on the underlying base
iterator type, not the safe iterators. So doesn't that use this
overload from <bits/stl_iterator.h> ?

  // _GLIBCXX_RESOLVE_LIB_DEFECTS
  // According to the resolution of DR179 not only the various comparison
  // operators but also operator- must accept mixed iterator/const_iterator
  // parameters.
  template<typename _IteratorL, typename _IteratorR, typename _Container>
#if __cplusplus >= 201103L
    // DR 685.
    inline auto
    operator-(const __normal_iterator<_IteratorL, _Container>& __lhs,
	      const __normal_iterator<_IteratorR, _Container>& __rhs) noexcept
    -> decltype(__lhs.base() - __rhs.base())
#else
    inline typename __normal_iterator<_IteratorL, _Container>::difference_type
    operator-(const __normal_iterator<_IteratorL, _Container>& __lhs,
	      const __normal_iterator<_IteratorR, _Container>& __rhs)
#endif
    { return __lhs.base() - __rhs.base(); }

Why would that involve any conversion?