diff mbox

Fix _Hashtable extension

Message ID 532CB1A7.5020906@gmail.com
State New
Headers show

Commit Message

François Dumont March 21, 2014, 9:39 p.m. UTC
Hi

     Here is a patch to fix _Hashtable Standard extension type which is 
almost unusable at the moment if instantiated with anything else that 
the types used for the std unordered containers that is to say 
__detail::_Default_ranged_hash and __detail::_Mod_range_hashing.

     It is a really safe patch so I would propose it for current trunk 
but at the same time it only impacts a Standard extension and it hasn't 
been reported by anyone so just tell me when to apply it.

2014-03-21  François Dumont <fdumont@gcc.gnu.org>

     * include/bits/hashtable.h (_Hashtable(allocator_type)): Fix call
     to delegated constructor.
     (_Hashtable(size_type, _H1, key_equal, allocator_type)): Likewise.
     (_Hashtable<_It>(_It, _It, size_type, _H1, key_equal, allocator_type)):
     Likewise.
     (_Hashtable(
     initializer_list, size_type, _H1, key_equal, allocator_type)): 
Likewise.

Tested under Linux x86_64.

François

Comments

Jonathan Wakely March 21, 2014, 10:59 p.m. UTC | #1
On 21/03/14 22:39 +0100, François Dumont wrote:
>Hi
>
>    Here is a patch to fix _Hashtable Standard extension type which 
>is almost unusable at the moment if instantiated with anything else 
>that the types used for the std unordered containers that is to say 
>__detail::_Default_ranged_hash and __detail::_Mod_range_hashing.

Good catch.

Also, it seems that this specialization is missing the "hasher"
typedef:

   /// Specialization: ranged hash function, no caching hash codes.  H1
   /// and H2 are provided but ignored.  We define a dummy hash code type.
   template<typename _Key, typename _Value, typename _ExtractKey,
      typename _H1, typename _H2, typename _Hash>
     struct _Hash_code_base<_Key, _Value, _ExtractKey, _H1, _H2, _Hash, false>
     : private _Hashtable_ebo_helper<0, _ExtractKey>,
       private _Hashtable_ebo_helper<1, _Hash>
     {

 From the comments I think it is intentional, is that right?


>    It is a really safe patch so I would propose it for current trunk 
>but at the same time it only impacts a Standard extension and it 
>hasn't been reported by anyone so just tell me when to apply it.

As it doesn't fix a regression and apparently isn't affecting anyone I
think it would be safer to add it to trunk after the 4.9 branch is
created.
Jonathan Wakely March 21, 2014, 11:10 p.m. UTC | #2
On 21/03/14 22:59 +0000, Jonathan Wakely wrote:
>On 21/03/14 22:39 +0100, François Dumont wrote:
>>   It is a really safe patch so I would propose it for current 
>>trunk but at the same time it only impacts a Standard extension and 
>>it hasn't been reported by anyone so just tell me when to apply it.
>
>As it doesn't fix a regression and apparently isn't affecting anyone I
>think it would be safer to add it to trunk after the 4.9 branch is
>created.

Actually, you're right, it's definitely safe and I'm being overly
cautious :-)  Please commit to the trunk, thanks!
François Dumont March 23, 2014, 8:56 p.m. UTC | #3
On 21/03/2014 23:59, Jonathan Wakely wrote:
> On 21/03/14 22:39 +0100, François Dumont wrote:
>> Hi
>>
>>    Here is a patch to fix _Hashtable Standard extension type which is 
>> almost unusable at the moment if instantiated with anything else that 
>> the types used for the std unordered containers that is to say 
>> __detail::_Default_ranged_hash and __detail::_Mod_range_hashing.
>
> Good catch.
>
> Also, it seems that this specialization is missing the "hasher"
> typedef:
>
>   /// Specialization: ranged hash function, no caching hash codes.  H1
>   /// and H2 are provided but ignored.  We define a dummy hash code type.
>   template<typename _Key, typename _Value, typename _ExtractKey,
>      typename _H1, typename _H2, typename _Hash>
>     struct _Hash_code_base<_Key, _Value, _ExtractKey, _H1, _H2, _Hash, 
> false>
>     : private _Hashtable_ebo_helper<0, _ExtractKey>,
>       private _Hashtable_ebo_helper<1, _Hash>
>     {
>
> From the comments I think it is intentional, is that right?

It seems intentional to me too even if I haven't written this code. In 
this case the user is supposed to provide a functor that gives the 
bucket index directly from the key so no real hasher even if there might 
be one in this functor.

Patch committed.

François
diff mbox

Patch

Index: include/bits/hashtable.h
===================================================================
--- include/bits/hashtable.h	(revision 207322)
+++ include/bits/hashtable.h	(working copy)
@@ -372,9 +372,8 @@ 
       // Use delegating constructors.
       explicit
       _Hashtable(const allocator_type& __a)
-	: _Hashtable(10, _H1(), __detail::_Mod_range_hashing(),
-		     __detail::_Default_ranged_hash(), key_equal(),
-		     __key_extract(), __a)
+      : _Hashtable(10, _H1(), _H2(), _Hash(), key_equal(),
+		   __key_extract(), __a)
       { }
 
       explicit
@@ -382,8 +381,7 @@ 
 		 const _H1& __hf = _H1(),
 		 const key_equal& __eql = key_equal(),
 		 const allocator_type& __a = allocator_type())
-      : _Hashtable(__n, __hf, __detail::_Mod_range_hashing(),
-		   __detail::_Default_ranged_hash(), __eql,
+      : _Hashtable(__n, __hf, _H2(), _Hash(), __eql,
 		   __key_extract(), __a)
       { }
 
@@ -393,8 +391,7 @@ 
 		   const _H1& __hf = _H1(),
 		   const key_equal& __eql = key_equal(),
 		   const allocator_type& __a = allocator_type())
-	: _Hashtable(__f, __l, __n, __hf, __detail::_Mod_range_hashing(),
-		     __detail::_Default_ranged_hash(), __eql,
+	: _Hashtable(__f, __l, __n, __hf, _H2(), _Hash(), __eql,
 		     __key_extract(), __a)
 	{ }
 
@@ -403,9 +400,7 @@ 
 		 const _H1& __hf = _H1(),
 		 const key_equal& __eql = key_equal(),
 		 const allocator_type& __a = allocator_type())
-      : _Hashtable(__l.begin(), __l.end(), __n, __hf,
-		   __detail::_Mod_range_hashing(),
-		   __detail::_Default_ranged_hash(), __eql,
+      : _Hashtable(__l.begin(), __l.end(), __n, __hf, _H2(), _Hash(), __eql,
 		   __key_extract(), __a)
       { }