[Bug libstdc++/115285] [12/13/14/15 Regression] std::unordered_set can have duplicate value

[Jonathan Wakely]

On 28/10/24 21:51 +0100, François Dumont wrote:
>
>>On 24/10/2024 21:49, Jonathan Wakely wrote:
>>>On Thu, 24 Oct 2024 at 19:43, François Dumont<frs.dumont@gmail.com>  wrote:
>>>>Committed as trivial the attached patch.
>>>>
>>>>      libstdc++: Fix test broken when using COW std::string
>>>>
>>>>      libstdc++-v3/ChangeLog:
>>>>
>>>>              * testsuite/23_containers/unordered_map/96088.cc (test03):
>>>Thanks! I think it was affecting the unordered_set test too.
>>>
>Hi
>
>This was indeed revealing a problem in my attempt to fix this bug.
>
>Here is the complete patch that I will backport if validated.
>
>libstdc++: [_Hashtable] Avoid temporaries when inserting existing key
>
>Following PR 115285 fix, the hashtable _S_forward_key responsible for 
>finding
>the best way to transmit the key part of the argument parameter to the hash
>functor was modified to return a key_type instance for any argument 
>type. Only
>the overloads for const key_type& and key_type&& were still preventing 
>a key_type
>instanciation.
>
>It revealed that a overload for key_type& was missing. To fix this 
>problem we now
>deal with all sort of key_type of reference type in the unique 
>_S_forward_key static
>method and remove the overloads. When decayed type of the input 
>instance is the same
>as key_type we avoid the instanciation. Note that for the comparison 
>we also decay
>key_type to properly managed when unordered container is instantiated 
>with a const
>type.
>
>libstdc++-v3/ChangeLog:
>
>        * include/bits/hashtable_policy.h
>        (_ConvertToValueType<_Select1st, 
>_Value>::operator(std::pair<>&)): New,
>        allow consistent behavior with _ConvertToValueType<_Identity, 
>_Value>.
>        * include/bits/hashtable.h
>        (_S_forward_key<_Kt>(_Kt&&)): Adapt to return _Kt&& when 
>decayed _Kt is
>        key_type, return a new key_type instance otherwise.
>        (_S_forward_key(const key_type&)): Remove.
>        (_S_forward_key(key_type&&)): Remove.
>        * testsuite/23_containers/unordered_set/96088.cc (test03): 
>Adapt expected
>        allocation increment.
>
>Tested under linux x64, both std::string abi.
>
>Ok to commit ?

No, because it's still not a complete fix, just more special cases.

I think the attached patch (actually two patches, as there's one to
add __is_pair first) fixes PR 115285, including all the other cases I
added in comment 14. It also avoids regressing PR 96088.

This seems small and safe enough to backport. On trunk I still plan to
follow this with the bigger refactoring.

Tests are still running ...


-------------- next part --------------
A non-text attachment was scrubbed...
Name: patch.txt
Type: text/x-patch
Size: 8568 bytes
Desc: not available
URL: <https://gcc.gnu.org/pipermail/libstdc++/attachments/20241105/4db13d1f/attachment.bin>