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