[PATCH][_Hashtable] Fix insertion of range of type convertible to value_type PR 105714

[François Dumont]

Here is the patch to fix just what is described in PR 105714.

     libstdc++: [_Hashtable] Insert range of types convertible to 
value_type PR 105714

     Fix insertion of range of types convertible to value_type.

     libstdc++-v3/ChangeLog:

             PR libstdc++/105714
             * include/bits/hashtable_policy.h (_ValueTypeEnforcer): New.
             * include/bits/hashtable.h 
(_Hashtable<>::_M_insert_unique_aux): New.
             (_Hashtable<>::_M_insert(_Arg&&, const _NodeGenerator&, 
true_type)): Use latters.
             (_Hashtable<>::_M_insert(_Arg&&, const _NodeGenerator&, 
false_type)): Likewise.
             (_Hashtable(_InputIterator, _InputIterator, size_type, 
const _Hash&, const _Equal&,
             const allocator_type&, true_type)): Use this.insert range.
             (_Hashtable(_InputIterator, _InputIterator, size_type, 
const _Hash&, const _Equal&,
             const allocator_type&, false_type)): Use _M_insert.
             * testsuite/23_containers/unordered_map/cons/56112.cc: 
Check how many times conversion
             is done.
             * testsuite/23_containers/unordered_map/insert/105714.cc: 
New test.
             * testsuite/23_containers/unordered_set/insert/105714.cc: 
New test.

Tested under Linux x64, ok to commit ?

François

On 24/05/22 12:31, Jonathan Wakely wrote:
> On Tue, 24 May 2022 at 11:22, Jonathan Wakely wrote:
>> On Tue, 24 May 2022 at 11:18, Jonathan Wakely wrote:
>>> On Thu, 5 May 2022 at 18:38, François Dumont via Libstdc++
>>> <libstdc++@gcc.gnu.org> wrote:
>>>> Hi
>>>>
>>>> Renewing my patch to fix PR 56112 but for the insert methods, I totally
>>>> change it, now works also with move-only key types.
>>>>
>>>> I let you Jonathan find a better name than _ValueTypeEnforcer as usual :-)
>>>>
>>>> libstdc++: [_Hashtable] Insert range of types convertible to value_type
>>>> PR 56112
>>>>
>>>> Fix insertion of range of types convertible to value_type. Fix also when
>>>> this value_type
>>>> has a move-only key_type which also allow converted values to be moved.
>>>>
>>>> libstdc++-v3/ChangeLog:
>>>>
>>>>           PR libstdc++/56112
>>>>           * include/bits/hashtable_policy.h (_ValueTypeEnforcer): New.
>>>>           * include/bits/hashtable.h
>>>> (_Hashtable<>::_M_insert_unique_aux): New.
>>>>           (_Hashtable<>::_M_insert(_Arg&&, const _NodeGenerator&,
>>>> true_type)): Use latters.
>>>>           (_Hashtable<>::_M_insert(_Arg&&, const _NodeGenerator&,
>>>> false_type)): Likewise.
>>>>           (_Hashtable(_InputIterator, _InputIterator, size_type, const
>>>> _Hash&, const _Equal&,
>>>>           const allocator_type&, true_type)): Use this.insert range.
>>>>           (_Hashtable(_InputIterator, _InputIterator, size_type, const
>>>> _Hash&, const _Equal&,
>>>>           const allocator_type&, false_type)): Use _M_insert.
>>>>           * testsuite/23_containers/unordered_map/cons/56112.cc: Check
>>>> how many times conversion
>>>>           is done.
>>>>           (test02): New test case.
>>>>           * testsuite/23_containers/unordered_set/cons/56112.cc: New test.
>>>>
>>>> Tested under Linux x86_64.
>>>>
>>>> Ok to commit ?
>>> No, sorry.
>>>
>>> The new test02 function in 23_containers/unordered_map/cons/56112.cc
>>> doesn't compile with libc++ or MSVC either, are you sure that test is
>>> valid? I don't think it is, because S2 is not convertible to
>>> pair<const MoveOnlyKey, int>. None of the pair constructors are
>>> viable, because the move constructor would require two user-defined
>>> conversions (from S2 to pair<MoveOnlyKey, int> and then from
>>> pair<MoveOnlyKey, int> to pair<const MoveOnlyKey, int>). A conversion
>>> sequence cannot have more than one user-defined conversion using a
>>> constructor or converion operator. So if your patch makes that
>>> compile, it's a bug in the new code. I haven't analyzed that code to
>>> see where the problem is, I'm just looking at the test results and the
>>> changes in behaviour.
>> I meant to include this link showing that libc++ and MSVC reject
>> test02() as well:
>>
>> https://godbolt.org/z/j7E9f6bd4
> I've created https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105717 for
> the insertion bug, rather than reopening PR 56112.
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: pr105714.patch
Type: text/x-patch
Size: 9372 bytes
Desc: not available
URL: <https://gcc.gnu.org/pipermail/libstdc++/attachments/20220525/064f7403/attachment-0001.bin>