diff mbox

safe iterator simplification

Message ID 527BFF89.2020406@gmail.com
State New
Headers show

Commit Message

François Dumont Nov. 7, 2013, 9 p.m. UTC 
Hi

     Here is a patch to simplify a little safe iterator implementation. 
Returning a sequence pointer from a safe iterator and a const sequence 
pointer from a const safe iterator allows to avoid inconsistent usages 
of iterator like comparing iterator with a const_iterator. This way 
__get_distance can be simplified to only take one type of iterator, 
_M_valid_range is not a template method anymore. I also modified 
_BeforeBeginHelper so that it also generates consistent comparisons of 
iterators.

2013-11-08  François Dumont  <fdumont@gcc.gnu.org>

     * include/debug/safe_iterator.h (_BeforeBeginHelper<>::_S_Is):
     Take only a const safe iterator reference.
     (_BeforeBeingHelper<>::_S_Is_beginnest): Likewise.
     (__get_distance): Take only one type of iterator.
     (_Safe_iterator<>::_M_valid_range<>): Not template anymore.
     (_Safe_iterator<>::_M_get_sequence()): Return pointer to const
     sequence from a const_iterator and a pointer to sequence from an
     iterator.
     * include/debug/safe_iterator.tcc: Adapt.
     * include/debug/safe_local_iterator.h
     (_Safe_local_iterator<>::_M_valid_range<>): Not template anymore.
     (_Safe_local_iterator<>::_M_get_sequence()): Return pointer to
     const sequence from a const_iterator and a pointer to sequence
     from an iterator.
     * include/debug/safe_local_iterator.tcc: Adapt.
     * include/debug/forward_list
     (_BeforeBeginHelpers<std::__debug::forward_list<>>): Adapt.


Tested undex Linux x86_64 debug mode.

Ok to commit ?

François

Comments

Jonathan Wakely Nov. 8, 2013, 11:29 a.m. UTC | #1 
On 7 November 2013 21:00, François Dumont wrote:
>
>     * include/debug/safe_iterator.h (_BeforeBeginHelper<>::_S_Is):
>     Take only a const safe iterator reference.
>     (_BeforeBeingHelper<>::_S_Is_beginnest): Likewise.

There's a typo here, "Being" not "Begin"

> Ok to commit ?

Yes, with the tiny fix to the ChangeLog, thanks!
diff mbox

Patch

Index: include/debug/safe_iterator.tcc
===================================================================
--- include/debug/safe_iterator.tcc	(revision 204419)
+++ include/debug/safe_iterator.tcc	(working copy)
@@ -36,27 +36,22 @@ 
     _Safe_iterator<_Iterator, _Sequence>::
     _M_can_advance(const difference_type& __n) const
     {
-      typedef typename _Sequence::const_iterator const_debug_iterator;
-      typedef typename const_debug_iterator::iterator_type const_iterator;
-
       if (this->_M_singular())
 	return false;
       if (__n == 0)
 	return true;
       if (__n < 0)
 	{
-	  const_iterator __begin = _M_get_sequence()->_M_base().begin();
 	  std::pair<difference_type, _Distance_precision> __dist =
-	    __get_distance(__begin, base());
+	    __get_distance(_M_get_sequence()->_M_base().begin(), base());
 	  bool __ok =  ((__dist.second == __dp_exact && __dist.first >= -__n)
 			|| (__dist.second != __dp_exact && __dist.first > 0));
 	  return __ok;
 	}
       else
 	{
-	  const_iterator __end = _M_get_sequence()->_M_base().end();
 	  std::pair<difference_type, _Distance_precision> __dist =
-	    __get_distance(base(), __end);
+	    __get_distance(base(), _M_get_sequence()->_M_base().end());
 	  bool __ok = ((__dist.second == __dp_exact && __dist.first >= __n)
 		       || (__dist.second != __dp_exact && __dist.first > 0));
 	  return __ok;
@@ -64,42 +59,41 @@ 
     }
 
   template<typename _Iterator, typename _Sequence>
-    template<typename _Other>
-      bool
-      _Safe_iterator<_Iterator, _Sequence>::
-      _M_valid_range(const _Safe_iterator<_Other, _Sequence>& __rhs) const
-      {
-	if (!_M_can_compare(__rhs))
-	  return false;
+    bool
+    _Safe_iterator<_Iterator, _Sequence>::
+    _M_valid_range(const _Safe_iterator& __rhs) const
+    {
+      if (!_M_can_compare(__rhs))
+	return false;
 
-	/* Determine if we can order the iterators without the help of
-	   the container */
-	std::pair<difference_type, _Distance_precision> __dist =
-	  __get_distance(base(), __rhs.base());
-	switch (__dist.second) {
-	case __dp_equality:
-	  if (__dist.first == 0)
-	    return true;
-	  break;
-
-	case __dp_sign:
-	case __dp_exact:
-	  return __dist.first >= 0;
-	}
-
-	/* We can only test for equality, but check if one of the
-	   iterators is at an extreme. */
-	/* Optim for classic [begin, it) or [it, end) ranges, limit checks
-	 * when code is valid.  Note, for the special case of forward_list,
-	 * before_begin replaces the role of begin.  */ 
-	if (_M_is_beginnest() || __rhs._M_is_end())
+      /* Determine if we can order the iterators without the help of
+	 the container */
+      std::pair<difference_type, _Distance_precision> __dist =
+	__get_distance(base(), __rhs.base());
+      switch (__dist.second) {
+      case __dp_equality:
+	if (__dist.first == 0)
 	  return true;
-	if (_M_is_end() || __rhs._M_is_beginnest())
-	  return false;
+	break;
 
-	// Assume that this is a valid range; we can't check anything else
-	return true;
+      case __dp_sign:
+      case __dp_exact:
+	return __dist.first >= 0;
       }
+
+      /* We can only test for equality, but check if one of the
+	 iterators is at an extreme. */
+      /* Optim for classic [begin, it) or [it, end) ranges, limit checks
+       * when code is valid.  Note, for the special case of forward_list,
+       * before_begin replaces the role of begin.  */ 
+      if (_M_is_beginnest() || __rhs._M_is_end())
+	return true;
+      if (_M_is_end() || __rhs._M_is_beginnest())
+	return false;
+
+      // Assume that this is a valid range; we can't check anything else
+      return true;
+    }
 } // namespace __gnu_debug
 
 #endif
Index: include/debug/forward_list
===================================================================
--- include/debug/forward_list	(revision 204419)
+++ include/debug/forward_list	(working copy)
@@ -785,16 +785,19 @@ 
     struct _BeforeBeginHelper<std::__debug::forward_list<_Tp, _Alloc> >
     {
       typedef std::__debug::forward_list<_Tp, _Alloc> _Sequence;
-      typedef typename _Sequence::const_iterator _It;
-      typedef typename _It::iterator_type _BaseIt;
 
-      static bool
-      _S_Is(_BaseIt __it, const _Sequence* __seq)
-      { return __it == __seq->_M_base().cbefore_begin(); }
+      template<typename _Iterator>
+	static bool
+	_S_Is(const _Safe_iterator<_Iterator, _Sequence>& __it)
+	{
+	  return
+	    __it.base() == __it._M_get_sequence()->_M_base().before_begin();
+	}
 
-      static bool
-      _S_Is_Beginnest(_BaseIt __it, const _Sequence* __seq)
-      { return _S_Is(__it, __seq); }
+      template<typename _Iterator>
+	static bool
+	_S_Is_Beginnest(const _Safe_iterator<_Iterator, _Sequence>& __it)
+	{ return _S_Is(__it); }
     };
 
 #ifndef _GLIBCXX_DEBUG_PEDANTIC
Index: include/debug/safe_local_iterator.tcc
===================================================================
--- include/debug/safe_local_iterator.tcc	(revision 204419)
+++ include/debug/safe_local_iterator.tcc	(working copy)
@@ -32,21 +32,20 @@ 
 namespace __gnu_debug
 {
   template<typename _Iterator, typename _Sequence>
-    template<typename _Other>
-      bool
-      _Safe_local_iterator<_Iterator, _Sequence>::
-      _M_valid_range(const _Safe_local_iterator<_Other, _Sequence>& __rhs) const
-      {
-	if (!_M_can_compare(__rhs))
-	  return false;
-	if (_M_bucket != __rhs._M_bucket)
-	  return false;
+    bool
+    _Safe_local_iterator<_Iterator, _Sequence>::
+    _M_valid_range(const _Safe_local_iterator& __rhs) const
+    {
+      if (!_M_can_compare(__rhs))
+	return false;
+      if (_M_bucket != __rhs._M_bucket)
+	return false;
 
-	/* Determine if we can order the iterators without the help of
-	   the container */
-	std::pair<difference_type, _Distance_precision> __dist =
-	  __get_distance(base(), __rhs.base());
-	switch (__dist.second)
+      /* Determine if we can order the iterators without the help of
+	 the container */
+      std::pair<difference_type, _Distance_precision> __dist =
+	__get_distance(base(), __rhs.base());
+      switch (__dist.second)
 	{
 	case __dp_equality:
 	  if (__dist.first == 0)
@@ -58,18 +57,18 @@ 
 	  return __dist.first >= 0;
 	}
 
-	/* We can only test for equality, but check if one of the
-	   iterators is at an extreme. */
-	/* Optim for classic [begin, it) or [it, end) ranges, limit checks
-	 * when code is valid. */
-	if (_M_is_begin() || __rhs._M_is_end())
-	  return true;
-	if (_M_is_end() || __rhs._M_is_begin())
-	  return false;
-
-	// Assume that this is a valid range; we can't check anything else
+      /* We can only test for equality, but check if one of the
+	 iterators is at an extreme. */
+      /* Optim for classic [begin, it) or [it, end) ranges, limit checks
+       * when code is valid. */
+      if (_M_is_begin() || __rhs._M_is_end())
 	return true;
-      }
+      if (_M_is_end() || __rhs._M_is_begin())
+	return false;
+
+      // Assume that this is a valid range; we can't check anything else
+      return true;
+    }
 } // namespace __gnu_debug
 
 #endif
Index: include/debug/safe_iterator.h
===================================================================
--- include/debug/safe_iterator.h	(revision 204419)
+++ include/debug/safe_iterator.h	(working copy)
@@ -44,16 +44,15 @@ 
   template <typename _Sequence>
     struct _BeforeBeginHelper
     {
-      typedef typename _Sequence::const_iterator _It;
-      typedef typename _It::iterator_type _BaseIt;
+      template<typename _Iterator>
+	static bool
+	_S_Is(const _Safe_iterator<_Iterator, _Sequence>&)
+	{ return false; }
 
-      static bool
-      _S_Is(_BaseIt, const _Sequence*)
-      { return false; }
-
-      static bool
-      _S_Is_Beginnest(_BaseIt __it, const _Sequence* __seq)
-      { return __it == __seq->_M_base().begin(); }
+      template<typename _Iterator>
+	static bool
+	_S_Is_Beginnest(const _Safe_iterator<_Iterator, _Sequence>& __it)
+	{ return __it.base() == __it._M_get_sequence()->_M_base().begin(); }
     };
 
   /** Iterators that derive from _Safe_iterator_base can be determined singular
@@ -76,26 +75,26 @@ 
   /** Determine the distance between two iterators with some known
    *	precision.
   */
-  template<typename _Iterator1, typename _Iterator2>
-    inline std::pair<typename std::iterator_traits<_Iterator1>::difference_type,
+  template<typename _Iterator>
+    inline std::pair<typename std::iterator_traits<_Iterator>::difference_type,
 		     _Distance_precision>
-    __get_distance(const _Iterator1& __lhs, const _Iterator2& __rhs,
+    __get_distance(const _Iterator& __lhs, const _Iterator& __rhs,
 		   std::random_access_iterator_tag)
     { return std::make_pair(__rhs - __lhs, __dp_exact); }
 
-  template<typename _Iterator1, typename _Iterator2>
-    inline std::pair<typename std::iterator_traits<_Iterator1>::difference_type,
+  template<typename _Iterator>
+    inline std::pair<typename std::iterator_traits<_Iterator>::difference_type,
 		     _Distance_precision>
-    __get_distance(const _Iterator1& __lhs, const _Iterator2& __rhs,
+    __get_distance(const _Iterator& __lhs, const _Iterator& __rhs,
 		   std::forward_iterator_tag)
     { return std::make_pair(__lhs == __rhs? 0 : 1, __dp_equality); }
 
-  template<typename _Iterator1, typename _Iterator2>
-    inline std::pair<typename std::iterator_traits<_Iterator1>::difference_type,
+  template<typename _Iterator>
+    inline std::pair<typename std::iterator_traits<_Iterator>::difference_type,
 		     _Distance_precision>
-    __get_distance(const _Iterator1& __lhs, const _Iterator2& __rhs)
+    __get_distance(const _Iterator& __lhs, const _Iterator& __rhs)
     {
-      typedef typename std::iterator_traits<_Iterator1>::iterator_category
+      typedef typename std::iterator_traits<_Iterator>::iterator_category
 	  _Category;
       return __get_distance(__lhs, __rhs, _Category());
     }
@@ -115,6 +114,7 @@ 
     class _Safe_iterator : public _Safe_iterator_base
     {
       typedef _Safe_iterator _Self;
+      typedef typename _Sequence::const_iterator _Const_iterator;
 
       /// The underlying iterator
       _Iterator _M_current;
@@ -122,10 +122,7 @@ 
       /// Determine if this is a constant iterator.
       bool
       _M_constant() const
-      {
-	typedef typename _Sequence::const_iterator const_iterator;
-	return std::__are_same<const_iterator, _Safe_iterator>::__value;
-      }
+      { return std::__are_same<_Const_iterator, _Safe_iterator>::__value; }
 
       typedef std::iterator_traits<_Iterator> _Traits;
 
@@ -445,37 +442,39 @@ 
       _M_can_advance(const difference_type& __n) const;
 
       // Is the iterator range [*this, __rhs) valid?
-      template<typename _Other>
-        bool
-        _M_valid_range(const _Safe_iterator<_Other, _Sequence>& __rhs) const;
+      bool
+      _M_valid_range(const _Safe_iterator& __rhs) const;
 
       // The sequence this iterator references.
-      const _Sequence*
+      typename
+      __gnu_cxx::__conditional_type<std::__are_same<_Const_iterator,
+						    _Safe_iterator>::__value,
+				    const _Sequence*,
+				    _Sequence*>::__type
       _M_get_sequence() const
-      { return static_cast<const _Sequence*>(_M_sequence); }
+      { return static_cast<_Sequence*>(_M_sequence); }
 
       /// Is this iterator equal to the sequence's begin() iterator?
-      bool _M_is_begin() const
+      bool
+      _M_is_begin() const
       { return base() == _M_get_sequence()->_M_base().begin(); }
 
       /// Is this iterator equal to the sequence's end() iterator?
-      bool _M_is_end() const
+      bool
+      _M_is_end() const
       { return base() == _M_get_sequence()->_M_base().end(); }
 
       /// Is this iterator equal to the sequence's before_begin() iterator if
       /// any?
-      bool _M_is_before_begin() const
-      {
-	return _BeforeBeginHelper<_Sequence>::_S_Is(base(), _M_get_sequence());
-      }
+      bool
+      _M_is_before_begin() const
+      { return _BeforeBeginHelper<_Sequence>::_S_Is(*this); }
 
       /// Is this iterator equal to the sequence's before_begin() iterator if
       /// any or begin() otherwise?
-      bool _M_is_beginnest() const
-      {
-	return _BeforeBeginHelper<_Sequence>::_S_Is_Beginnest(base(),
-							  _M_get_sequence());
-      }
+      bool
+      _M_is_beginnest() const
+      { return _BeforeBeginHelper<_Sequence>::_S_Is_Beginnest(*this); }
     };
 
   template<typename _IteratorL, typename _IteratorR, typename _Sequence>
Index: include/debug/safe_local_iterator.h
===================================================================
--- include/debug/safe_local_iterator.h	(revision 204419)
+++ include/debug/safe_local_iterator.h	(working copy)
@@ -52,6 +52,7 @@ 
     class _Safe_local_iterator : public _Safe_local_iterator_base
     {
       typedef _Safe_local_iterator _Self;
+      typedef typename _Sequence::const_local_iterator _Const_local_iterator;
       typedef typename _Sequence::size_type size_type;
 
       /// The underlying iterator
@@ -64,8 +65,8 @@ 
       bool
       _M_constant() const
       {
-	typedef typename _Sequence::const_local_iterator const_iterator;
-	return std::__are_same<const_iterator, _Safe_local_iterator>::__value;
+	return std::__are_same<_Const_local_iterator,
+			       _Safe_local_iterator>::__value;
       }
 
       typedef std::iterator_traits<_Iterator> _Traits;
@@ -253,15 +254,17 @@ 
       { return !this->_M_singular() && !_M_is_end(); }
 
       // Is the iterator range [*this, __rhs) valid?
-      template<typename _Other>
-	bool
-	_M_valid_range(const _Safe_local_iterator<_Other,
-						  _Sequence>& __rhs) const;
+      bool
+      _M_valid_range(const _Safe_local_iterator& __rhs) const;
 
       // The sequence this iterator references.
-      const _Sequence*
+      typename
+      __gnu_cxx::__conditional_type<std::__are_same<_Const_local_iterator,
+						    _Safe_local_iterator>::__value,
+				    const _Sequence*,
+				    _Sequence*>::__type
       _M_get_sequence() const
-      { return static_cast<const _Sequence*>(_M_sequence); }
+      { return static_cast<_Sequence*>(_M_sequence); }
 
       /// Is this iterator equal to the sequence's begin() iterator?
       bool _M_is_begin() const