On 16/05/19 13:30 +0100, Jonathan Wakely wrote:
On 16/05/19 11:05 +0100, Jonathan Wakely wrote:
On 16/05/19 07:47 +0200, François Dumont wrote:
On 5/15/19 5:37 PM, Jonathan Wakely wrote:
François,
I noticed that _Hash_code_base and _Hashtable_base have a number of
member functions which are overloaded for const and non-const:

   const _Equal&
   _M_eq() const { return _EqualEBO::_S_cget(*this); }

   _Equal&
   _M_eq() { return _EqualEBO::_S_get(*this); }

The non-const ones seem to be unnecessary. They're used in the _M_swap
member functions, but all other uses could (and probably should) call
the const overload to get a const reference to the function object.

If we make the _M_swap members use the EBO accessors directly then we
can get rid of the non-const accessors. That makes overload resolution
simpler for the compiler (as there's only one function to choose from)
and should result in slightly smaller code when inlining is not
enabled.

Do you see any problem with this patch?


I think it is more a Pavlov behavior, always providing const and non-const no matter what.

No problem to simplify this.

OK, tested powerpc64le-linux, committed to trunk.

I don't see a need for the _Hashtable_ebo_helper member functions to
be static. They return a member of *this, but are static so *this has
to be passed as a parameter. We could just make them non-static.

It seems to have always been that way since the first version of the
patch that added the helpers:
https://gcc.gnu.org/ml/libstdc++/2011-12/msg00139.html

This patch passes all tests. I plan to commit this to trunk too.

And another one (which I've actually had sitting in a local branch for
a few years!)


commit 4cf9940963e326d09080177d7404f5aaedae67df
Author: Jonathan Wakely <jwak...@redhat.com>
Date:   Fri Dec 5 10:42:54 2014 +0000

    Replace _Equal_helper with simpler class template
    
    By defining the new helper inside _Hashtable_base it doesn't need all
    the template parameters to be provided, and by making it only
    responsible for checking a possibly-cached hash code it only has to do
    one thing.  The caller can use the equality predicate itself instead of
    duplicating that in the helper template.
    
            * include/bits/hashtable_policy.h (_Equal_helper): Remove.
            (_Hashtable_base::_Equal_hash_code): Define new class template.
            (_Hashtable_base::_M_equals): Use _Equal_hash_code instead of
            _Equal_helper.

diff --git a/libstdc++-v3/include/bits/hashtable_policy.h b/libstdc++-v3/include/bits/hashtable_policy.h
index f7db7628c69..86589e9a2d6 100644
--- a/libstdc++-v3/include/bits/hashtable_policy.h
+++ b/libstdc++-v3/include/bits/hashtable_policy.h
@@ -1403,38 +1403,6 @@ namespace __detail
       _M_h2() const { return __ebo_h2::_M_cget(); }
     };
 
-  /**
-   *  Primary class template _Equal_helper.
-   *
-   */
-  template <typename _Key, typename _Value, typename _ExtractKey,
-	    typename _Equal, typename _HashCodeType,
-	    bool __cache_hash_code>
-  struct _Equal_helper;
-
-  /// Specialization.
-  template<typename _Key, typename _Value, typename _ExtractKey,
-	   typename _Equal, typename _HashCodeType>
-  struct _Equal_helper<_Key, _Value, _ExtractKey, _Equal, _HashCodeType, true>
-  {
-    static bool
-    _S_equals(const _Equal& __eq, const _ExtractKey& __extract,
-	      const _Key& __k, _HashCodeType __c, _Hash_node<_Value, true>* __n)
-    { return __c == __n->_M_hash_code && __eq(__k, __extract(__n->_M_v())); }
-  };
-
-  /// Specialization.
-  template<typename _Key, typename _Value, typename _ExtractKey,
-	   typename _Equal, typename _HashCodeType>
-  struct _Equal_helper<_Key, _Value, _ExtractKey, _Equal, _HashCodeType, false>
-  {
-    static bool
-    _S_equals(const _Equal& __eq, const _ExtractKey& __extract,
-	      const _Key& __k, _HashCodeType, _Hash_node<_Value, false>* __n)
-    { return __eq(__k, __extract(__n->_M_v())); }
-  };
-
-
   /// Partial specialization used when nodes contain a cached hash code.
   template<typename _Key, typename _Value, typename _ExtractKey,
 	   typename _H1, typename _H2, typename _Hash>
@@ -1788,8 +1756,22 @@ namespace __detail
 						     iterator>::type;
   private:
     using _EqualEBO = _Hashtable_ebo_helper<0, _Equal>;
-    using _EqualHelper =  _Equal_helper<_Key, _Value, _ExtractKey, _Equal,
-					__hash_code, __hash_cached::value>;
+
+    template<typename _NodeT>
+      struct _Equal_hash_code
+      {
+       static bool
+       _S_equals(__hash_code, const _NodeT&)
+       { return true; }
+      };
+
+    template<typename _Ptr2>
+      struct _Equal_hash_code<_Hash_node<_Ptr2, true>>
+      {
+       static bool
+       _S_equals(__hash_code __c, const _Hash_node<_Ptr2, true>& __n)
+       { return __c == __n._M_hash_code; }
+      };
 
   protected:
     _Hashtable_base() = default;
@@ -1801,8 +1783,8 @@ namespace __detail
     bool
     _M_equals(const _Key& __k, __hash_code __c, __node_type* __n) const
     {
-      return _EqualHelper::_S_equals(_M_eq(), this->_M_extract(),
-				     __k, __c, __n);
+      return _Equal_hash_code<__node_type>::_S_equals(__c, *__n)
+	&& _M_eq()(__k, this->_M_extract()(__n->_M_v()));
     }
 
     void

Reply via email to