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.
