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. The new 23_containers/unordered_set/cons/56112.cc test fails for GCC 11 but passes for GCC 12, even without your patch. Is it actually testing some other change, not this patch, and not the 2013 fix for PR 56112?
