[v3] Fix management of non empty hash functor

[François Dumont]

Hi

     No one to validate this patch ?

     Note that it can be considered as a bug fix because without it 
using unordered containers with a non-empty hash functor will produce 
bad code for local iterators.

François

On 01/10/2013 10:02 PM, François Dumont wrote:
> Hi
>
>     Here is an other version of this patch. Indeed there were no need 
> to expose many stuff public. Inheriting from _Hash_code_base is fine, 
> it is not final and it deals with EBO itself. I only kept usage of 
> _Hashtable_ebo_helper when embedding H2 functor. As it is an extension 
> we could have impose it not to be final but it doesn't cost a lot to 
> deal with it. Finally I only needed a single friend declaration to get 
> access to the H2 part of _Hash_code_base.
>
>     I didn't touch the default cache policy for the moment except 
> reducing constraints on the hash functor. I prefer to submit an other 
> patch to change when we cache or not depending on the hash functor 
> expected performance.
>
>     I also took the time to replace some typedef expressions with 
> using ones. I really know what is the rule about using one or the 
> other but I remembered that Benjamin spent quite some time changing 
> typedef in using so I prefer to stick to this approach in this file, 
> even if there are still some typedef left.
>
>     Tested under linux x86_64 normal and debug modes.
>
> 2013-01-10  François Dumont  <fdumont@gcc.gnu.org>
>
>     * include/bits/hashtable_policy.h (_Local_iterator_base): Use
>     _Hashtable_ebo_helper to embed necessary functors into the
>     local_iterator when necessary. Pass information about functors
>     involved in hash code by copy.
>     * include/bits/hashtable.h (__cache_default): Do not cache for
>     builtin integral types unless the hash functor is not noexcept
>     qualified or is not default constructible. Adapt static assertions
>     and local iteraror instantiations.
>     * include/debug/unordered_set
>     (std::__debug::unordered_set<>::erase): Detect local iterators to
>     invalidate using contained node rather than generating a dummy
>     local_iterator instance.
>     (std::__debug::unordered_multiset<>::erase): Likewise.
>     * include/debug/unordered_map
>     (std::__debug::unordered_map<>::erase): Likewise.
>     (std::__debug::unordered_multimap<>::erase): Likewise.
>     * testsuite/performance/23_containers/insert_erase/41975.cc: Test
>     std::tr1 and std versions of unordered_set regardless of any
>     macro. Add test on default cache behavior.
>     * testsuite/performance/23_containers/insert/54075.cc: Likewise.
>     * testsuite/23_containers/unordered_set/instantiation_neg.cc:
>     Adapt line number.
>     * testsuite/23_containers/unordered_set/
>     not_default_constructible_hash_neg.cc: New.
>     * testsuite/23_containers/unordered_set/buckets/swap.cc: New.
>
> If you agree with the patch tell me where and when to apply it.
>
> François
>
>
> On 01/04/2013 12:17 PM, Paolo Carlini wrote:
>> Hi,
>>
>> On 12/13/2012 10:32 PM, François Dumont wrote:
>>> Hi
>>>
>>>     As part of a performance patch proposed in an other mailing 
>>> thread was a patch to improve management of hash functor with state. 
>>> This part is I think less sensible than the performance patch so I 
>>> propose it independently. I only would like to commit the 
>>> modification on the performance tests here if you don't mind.
>>>
>>>     Thanks to this patch caching the hash code or not doesn't depend 
>>> on the hash functor to be empty of final anymore. I only keep the 
>>> default constructible condition so that local_iterator can be 
>>> default constructible, considering it is a Standard request.
>> I'm finally having a closer look at this work of yours (sorry aboutt 
>> the delay!) and I think we want something similar for 4.8.0. However, 
>> to be honest, I'm not convinced we are implementing the general idea 
>> in the best way, in particular I don't like the much more complex 
>> access control structure, _Hash_code_base loses encapsulation, etc. 
>> Did you consider maybe adding friend declarations in a few places?
>>
>> Jon, do you have suggestiong? The idea of managing to get rid of the 
>> empty & !final requirement for dispatching seems right to me.
>>
>> By the way, I'm also not convinced that is_integral is the right 
>> category, I think is_scalar for example is better: pointers are 
>> common and very similar in terms of std::hash, likewise floating 
>> point quantities (with the possible exception of long double, but I 
>> don't think we should spend time on it).
>>
>> Paolo.
>>
>