[PATCH][Hashtable 5/6] Remove H1/H2 template parameters

[Jonathan Wakely]

On 17/08/20 19:13 +0200, François Dumont via Libstdc++ wrote:
>Hi
>
>Â Â Â  Here is the new proposal.
>
>Â Â Â  As we can't remove template parameters I simply restore those that 
>I tried to pass differently _H2 and _ExtractKey, so eventually I only 
>remove usage of _Hash which I renamed in _Unused. Maybe I can keep the 
>doc about it in hashtable.h and just add a remark saying that it is 
>now unused.
>
>Â Â Â  For _RangeHash, formerly _H2, and _ExtractKey I just stop 
>maintaining any storage. When we need those I always use a value 
>initialized instance. I kind of prefer the value initialization syntax 
>because you can't confuse it with a function call but let me know if 
>it is wrong and I should use _ExtractKey() or _RangeHash(). I also add 
>some static assertions about those types regarding their noexcept 
>qualifications.
>
>Â Â Â  I also included in this patch the few changes left from [Hashtable 
>0/6] which are mostly _M_insert_unique_node and _M_insert_multi_node 
>signature cleanup as the key part can be extracted from the inserted 
>node.
>
>Â Â Â  Tested under Linux x86_64, ok to commit ?
>
>François
>
>On 06/08/20 11:27 am, Jonathan Wakely wrote:
>>On 06/08/20 08:35 +0200, François Dumont wrote:
>>>On 17/07/20 1:35 pm, Jonathan Wakely wrote:
>>>>I really like the general idea of getting rid of some of the
>>>>complexity and not supporting infinite customization. But we can do
>>>>that without changing mangled names of the _Hashtable specialiations.
>>>>
>>>>
>>>I didn't thought we need to keep abi compatibility for extensions.
>>
>>These aren't extensions though, they're part of std::unordered_map
>>etc.
>>
>>Just because something like _Vector_base is an internal type rather
>>than something defined in the standard doesn't mean we can just change
>>its ABI, because that would change the ABI of std::vector. It the same
>>here.
>>
>>Changing _Hashtable affects all users of std::unordered_map etc.
>>
>>
>

>diff --git a/libstdc++-v3/include/bits/hashtable.h b/libstdc++-v3/include/bits/hashtable.h
>index 7b772a475e3..1ba32a3c7e2 100644
>--- a/libstdc++-v3/include/bits/hashtable.h
>+++ b/libstdc++-v3/include/bits/hashtable.h
>@@ -311,35 +303,37 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> 		    "Cache the hash code or qualify your functors involved"
> 		    " in hash code and bucket index computation with noexcept");
> 
>-      // When hash codes are cached local iterator inherits from H2 functor
>-      // which must then be default constructible.
>-      static_assert(__if_hash_cached<is_default_constructible<_H2>>::value,
>+      // To get bucket index we need _RangeHash not to throw.
>+      static_assert(is_nothrow_default_constructible<_RangeHash>::value,
> 		    "Functor used to map hash code to bucket index"
>-		    " must be default constructible");
>+		    " is nothrow default constructible");

Please phrase this as "must be nothrow default constructible".

>+      static_assert(noexcept(
>+	std::declval<const _RangeHash&>()((std::size_t)0, (std::size_t)0)),
>+		"Functor used to map hash code to bucket index is noexcept");

Same here, "must be noexcept".

Otherwise this looks great, thanks. Please push.