Hi
Is this less ambitious version ok ?
Thanks
On 25/05/22 07:09, François Dumont wrote:
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++
<libstd...@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.
