Re: Review Hashtable extract node API

[Jonathan Wakely <jwakely at redhat dot com>]

On 18/06/19 07:52 +0200, François Dumont wrote:
> A small regression noticed while merging.
>
> We shouldn't keep on using a moved-from key_type instance.
>
> Ok to commit ? Feel free to do it if you prefer, I'll do so at end of 
> Europe day otherwise.


> diff --git a/libstdc++-v3/include/bits/hashtable_policy.h b/libstdc++-v3/include/bits/hashtable_policy.h
> index f5809c7443a..7e89e1b44c4 100644
> --- a/libstdc++-v3/include/bits/hashtable_policy.h
> +++ b/libstdc++-v3/include/bits/hashtable_policy.h
> @@ -743,7 +743,8 @@ namespace __detail
> 	std::tuple<>()
>       };
>       auto __pos
> -	= __h->_M_insert_unique_node(__k, __bkt, __code, __node._M_node);
> +	= __h->_M_insert_unique_node(__h->_M_extract()(__node._M_node->_M_v()),
> +				     __bkt, __code, __node._M_node);
>       __node._M_node = nullptr;
>       return __pos->second;
>     }

I can't create an example where this causes a problem, because the key
passed to _M_insert_unique_node is never used. So it doesn't matter
that it's been moved from.

So I have to wonder why we just added the key parameter to that
function, if it's never used.

As far as I can tell, it would only be used for a non-default range
hash function, and I don't care about that. Frankly I find the
policy-based _Hashtable completely unmaintainable and I'd gladly get
rid of all of it that isn't needed for the std::unordered_xxx
containers. The non-standard extensions are not used by anybody,
apparently not tested properly (or this regression should have been
noticed) and make the code too complicated.

We're adding new parameters that have to be passed around even though
they're never used by 99.999% of users. No wonder the code is only
fast at -O3.

</rant>