[PATCH][Hashtable 4/6] Clean local_iterator implementation

Jonathan Wakely jwakely@redhat.com
Fri Jul 17 10:45:40 GMT 2020


On 17/11/19 22:03 +0100, François Dumont wrote:
>Simplify local_iterator implementation. It makes local_iterator and 
>iterator comparable which is used in debug containers.
>
>    * include/bits/hashtable_policy.h (_Node_iterator_base()): New.
>    (operator==(const _Node_iterator_base&, const _Node_iterator_base&)):
>    Make hidden friend.
>    (operator!=(const _Node_iterator_base&, const _Node_iterator_base&)):
>    Make hidden friend.
>    (_Local_iterator_base<>): Inherits _Node_iterator_base.
>    (_Local_iterator_base<>::_M_cur): Remove.
>    (_Local_iterator_base<>::_M_curr()): Remove.
>    (operator==(const _Local_iterator_base&, const _Local_iterator_base&)):
>    Remove.
>    (operator!=(const _Local_iterator_base&, const _Local_iterator_base&)):
>    Remove.
>    * include/debug/unordered_map (unordered_map<>::_M_invalidate): Adapt.
>    (unordered_multimap<>::_M_invalidate): Adapt.
>    * include/debug/unordered_set (unordered_set<>::_M_invalidate): Adapt.
>    (unordered_multiset<>::_M_invalidate): Adapt.
>
>Tested under Linux x86_64.

OK for master, with one (non-essential) change mentioned below ...

>diff --git a/libstdc++-v3/include/bits/hashtable_policy.h b/libstdc++-v3/include/bits/hashtable_policy.h
>index f330f7f811b..5cc943b3d22 100644
>--- a/libstdc++-v3/include/bits/hashtable_policy.h
>+++ b/libstdc++-v3/include/bits/hashtable_policy.h
>@@ -301,27 +301,24 @@ namespace __detail
> 
>       __node_type*  _M_cur;
> 
>+      _Node_iterator_base() = default;
>       _Node_iterator_base(__node_type* __p) noexcept
>       : _M_cur(__p) { }
> 
>       void
>       _M_incr() noexcept
>       { _M_cur = _M_cur->_M_next(); }
>-    };
> 
>-  template<typename _Value, typename _Cache_hash_code>
>-    inline bool
>-    operator==(const _Node_iterator_base<_Value, _Cache_hash_code>& __x,
>-	       const _Node_iterator_base<_Value, _Cache_hash_code >& __y)
>-    noexcept
>-    { return __x._M_cur == __y._M_cur; }
>+      friend bool
>+      operator==(const _Node_iterator_base& __x, const _Node_iterator_base& __y)
>+      noexcept
>+      { return __x._M_cur == __y._M_cur; }
> 
>-  template<typename _Value, typename _Cache_hash_code>
>-    inline bool
>-    operator!=(const _Node_iterator_base<_Value, _Cache_hash_code>& __x,
>-	       const _Node_iterator_base<_Value, _Cache_hash_code>& __y)
>-    noexcept
>-    { return __x._M_cur != __y._M_cur; }
>+      friend bool
>+      operator!=(const _Node_iterator_base& __x, const _Node_iterator_base& __y)
>+      noexcept
>+      { return __x._M_cur != __y._M_cur; }

This operator!= can be surrounded by:

#if __cpp_impl_three_way_comparison < 201907L
...
#endif

because when three-way comparisons are supported (i.e. C++20 and
later) the operator!= can be generated by the compiler.

>+    };
> 
>   /// Node iterators, used to iterate through all the hashtable.
>   template<typename _Value, typename _Constant_iterators,
>@@ -345,7 +342,7 @@ namespace __detail
> 						  const _Value&, _Value&>::type;
> 
>       _Node_iterator() noexcept
>-      : __base_type(0) { }
>+      : __base_type(nullptr) { }
> 
>       explicit
>       _Node_iterator(__node_type* __p) noexcept
>@@ -394,7 +391,7 @@ namespace __detail
>       typedef const _Value&				reference;
> 
>       _Node_const_iterator() noexcept
>-      : __base_type(0) { }
>+      : __base_type(nullptr) { }
> 
>       explicit
>       _Node_const_iterator(__node_type* __p) noexcept
>@@ -1426,9 +1423,11 @@ namespace __detail
>     struct _Local_iterator_base<_Key, _Value, _ExtractKey,
> 				_H1, _H2, _Hash, __hash_cached_t>
>     : private _Hashtable_ebo_helper<0, _H2>
>+    , _Node_iterator_base<_Value, __hash_cached_t>

I was initially worried this is an ABI break, but it just moves the
_M_cur member into a new base class, which will be at the same offset
into the class, and won't change the sie or alignment. So I think it's
OK.


>@@ -1576,33 +1572,10 @@ namespace __detail
>       _M_destroy() { this->_M_h()->~__hash_code_base(); }
> 
>     public:
>-      const void*
>-      _M_curr() const { return _M_cur; }  // for equality ops and debug mode
>-
>       std::size_t
>       _M_get_bucket() const { return _M_bucket; }  // for debug mode
>     };
> 
>-  template<typename _Key, typename _Value, typename _ExtractKey,
>-	   typename _H1, typename _H2, typename _Hash,
>-	   typename _Cache_hash_code>
>-    inline bool
>-    operator==(const _Local_iterator_base<_Key, _Value, _ExtractKey,
>-	       _H1, _H2, _Hash, _Cache_hash_code>& __x,
>-	       const _Local_iterator_base<_Key, _Value, _ExtractKey,
>-	       _H1, _H2, _Hash, _Cache_hash_code>& __y)
>-    { return __x._M_curr() == __y._M_curr(); }
>-
>-  template<typename _Key, typename _Value, typename _ExtractKey,
>-	   typename _H1, typename _H2, typename _Hash,
>-	   typename _Cache_hash_code>
>-    inline bool
>-    operator!=(const _Local_iterator_base<_Key, _Value, _ExtractKey,
>-	       _H1, _H2, _Hash, _Cache_hash_code>& __x,
>-	       const _Local_iterator_base<_Key, _Value, _ExtractKey,
>-	       _H1, _H2, _Hash, _Cache_hash_code>& __y)
>-    { return __x._M_curr() != __y._M_curr(); }
>-

Removing these operators (and making the ones for the base hidden
friends) is a nice improvement.



More information about the Gcc-patches mailing list