Message ID | 2fe3937d-1b9a-a547-bb41-225d3d5426a2@gmail.com |
---|---|
State | New |
Headers | show |
Series | [_Hashtable] Fix insertion of range of type convertible to value_type PR 56112 | expand |
Gentle reminder, it is important to have this for gcc 12. On 15/02/22 10:05, François Dumont wrote: > We have a regression regarding management of types convertible to > value_type. It is an occurrence of PR 56112 but for the insert method. > > libstdc++: [_Hashtable] Insert range of types convertible to > value_type PR 56112 > > Fix insertion of range of types convertible to value_type. > > libstdc++-v3/ChangeLog: > > PR libstdc++/56112 > * include/bits/hashtable.h > (_Hashtable<>::_M_insert_unique_aux): New. > (_Hashtable<>::_S_to_value): New. > (_Hashtable<>::_M_insert(_Arg&&, const _NodeGenerator&, > true_type)): Use latters. > * testsuite/23_containers/unordered_map/cons/56112.cc: Use > dg-do compile. > * testsuite/23_containers/unordered_set/cons/56112.cc: New > test. > > Tested under Linux x86_64. > > Ok to commit ? > > François
On Mon, 21 Feb 2022 at 18:00, François Dumont via Libstdc++ < libstdc++@gcc.gnu.org> wrote: > Gentle reminder, it is important to have this for gcc 12. > Well, it's been broken since 4.8, so another year wouldn't be the end of the world ;-) I did start reviewing it, but I was trying to find a simpler way to solve it than adding all those overloads. I'll take another look tomorrow and either approve your patch or suggest something else. > > On 15/02/22 10:05, François Dumont wrote: > > We have a regression regarding management of types convertible to > > value_type. It is an occurrence of PR 56112 but for the insert method. > > > > libstdc++: [_Hashtable] Insert range of types convertible to > > value_type PR 56112 > > > > Fix insertion of range of types convertible to value_type. > > > > libstdc++-v3/ChangeLog: > > > > PR libstdc++/56112 > > * include/bits/hashtable.h > > (_Hashtable<>::_M_insert_unique_aux): New. > > (_Hashtable<>::_S_to_value): New. > > (_Hashtable<>::_M_insert(_Arg&&, const _NodeGenerator&, > > true_type)): Use latters. > > * testsuite/23_containers/unordered_map/cons/56112.cc: Use > > dg-do compile. > > * testsuite/23_containers/unordered_set/cons/56112.cc: New > > test. > > > > Tested under Linux x86_64. > > > > Ok to commit ? > > > > François > > >
On 21/02/22 21:54, Jonathan Wakely wrote: > > > On Mon, 21 Feb 2022 at 18:00, François Dumont via Libstdc++ > <libstdc++@gcc.gnu.org <mailto:libstdc%2B%2B@gcc.gnu.org>> wrote: > > Gentle reminder, it is important to have this for gcc 12. > > > Well, it's been broken since 4.8, so another year wouldn't be the end > of the world ;-) Sorry for the pressure, I thought I had broken it with my fix of PR 96088. Which moreover was in gcc 11. So indeed not mandatory for gcc 12 but still nice to fix eventually. Thanks > > I did start reviewing it, but I was trying to find a simpler way to > solve it than adding all those overloads. I'll take another look > tomorrow and either approve your patch or suggest something else. > > > > On 15/02/22 10:05, François Dumont wrote: > > We have a regression regarding management of types convertible to > > value_type. It is an occurrence of PR 56112 but for the insert > method. > > > > libstdc++: [_Hashtable] Insert range of types convertible to > > value_type PR 56112 > > > > Fix insertion of range of types convertible to value_type. > > > > libstdc++-v3/ChangeLog: > > > > PR libstdc++/56112 > > * include/bits/hashtable.h > > (_Hashtable<>::_M_insert_unique_aux): New. > > (_Hashtable<>::_S_to_value): New. > > (_Hashtable<>::_M_insert(_Arg&&, const _NodeGenerator&, > > true_type)): Use latters. > > * > testsuite/23_containers/unordered_map/cons/56112.cc: Use > > dg-do compile. > > * > testsuite/23_containers/unordered_set/cons/56112.cc: New > > test. > > > > Tested under Linux x86_64. > > > > Ok to commit ? > > > > François > >
diff --git a/libstdc++-v3/include/bits/hashtable.h b/libstdc++-v3/include/bits/hashtable.h index 5e1a417f7cd..5a502c02fe0 100644 --- a/libstdc++-v3/include/bits/hashtable.h +++ b/libstdc++-v3/include/bits/hashtable.h @@ -898,14 +898,51 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION template<typename _Arg, typename _NodeGenerator> std::pair<iterator, bool> - _M_insert(_Arg&& __arg, const _NodeGenerator& __node_gen, - true_type /* __uks */) + _M_insert_unique_aux(_Arg&& __arg, const _NodeGenerator& __node_gen) { return _M_insert_unique( _S_forward_key(_ExtractKey{}(std::forward<_Arg>(__arg))), std::forward<_Arg>(__arg), __node_gen); } + template<typename _Kt> + static typename std::enable_if<std::is_same<_ExtractKey, + __detail::_Identity>::value, + _Kt&&>::type + _S_to_value(_Kt&& __x) noexcept + { return std::forward<_Kt>(__x); } + + template<typename _Kt, typename _Val> + static typename std::enable_if<std::is_same<_ExtractKey, + __detail::_Select1st>::value, + const std::pair<_Kt, _Val>&>::type + _S_to_value(const std::pair<_Kt, _Val>& __x) noexcept + { return __x; } + + template<typename _Kt, typename _Val> + static typename std::enable_if<std::is_same<_ExtractKey, + __detail::_Select1st>::value, + std::pair<_Kt, _Val>&&>::type + _S_to_value(std::pair<_Kt, _Val>&& __x) noexcept + { return std::move(__x); } + + static value_type&& + _S_to_value(value_type&& __x) noexcept + { return std::move(__x); } + + static const value_type& + _S_to_value(const value_type& __x) noexcept + { return __x; } + + template<typename _Arg, typename _NodeGenerator> + std::pair<iterator, bool> + _M_insert(_Arg&& __arg, const _NodeGenerator& __node_gen, + true_type /* __uks */) + { + return _M_insert_unique_aux( + _S_to_value(std::forward<_Arg>(__arg)), __node_gen); + } + template<typename _Arg, typename _NodeGenerator> iterator _M_insert(_Arg&& __arg, const _NodeGenerator& __node_gen, diff --git a/libstdc++-v3/testsuite/23_containers/unordered_map/cons/56112.cc b/libstdc++-v3/testsuite/23_containers/unordered_map/cons/56112.cc index c4cdeee234c..8ec0d89af69 100644 --- a/libstdc++-v3/testsuite/23_containers/unordered_map/cons/56112.cc +++ b/libstdc++-v3/testsuite/23_containers/unordered_map/cons/56112.cc @@ -1,4 +1,4 @@ -// { dg-do run { target c++11 } } +// { dg-do compile { target c++11 } } // Copyright (C) 2013-2022 Free Software Foundation, Inc. // @@ -25,20 +25,23 @@ struct Key explicit Key(const int* p) : value(p) { } ~Key() { value = nullptr; } - bool operator==(const Key& k) const { return *value == *k.value; } + bool operator==(const Key& k) const + { return *value == *k.value; } const int* value; }; struct hash { - std::size_t operator()(const Key& k) const noexcept { return *k.value; } + std::size_t operator()(const Key& k) const noexcept + { return *k.value; } }; struct S { int value; - operator std::pair<const Key, int>() const { return {Key(&value), value}; } + operator std::pair<const Key, int>() const + { return {Key(&value), value}; } }; int main() @@ -46,4 +49,7 @@ int main() S s[1] = { {2} }; std::unordered_map<Key, int, hash> m(s, s+1); std::unordered_multimap<Key, int, hash> mm(s, s+1); + + m.insert(s, s + 1); + mm.insert(s, s + 1); } diff --git a/libstdc++-v3/testsuite/23_containers/unordered_set/cons/56112.cc b/libstdc++-v3/testsuite/23_containers/unordered_set/cons/56112.cc new file mode 100644 index 00000000000..aa34ed4c603 --- /dev/null +++ b/libstdc++-v3/testsuite/23_containers/unordered_set/cons/56112.cc @@ -0,0 +1,55 @@ +// { dg-do compile { target c++11 } } + +// Copyright (C) 2022 Free Software Foundation, Inc. +// +// This file is part of the GNU ISO C++ Library. This library is free +// software; you can redistribute it and/or modify it under the +// terms of the GNU General Public License as published by the +// Free Software Foundation; either version 3, or (at your option) +// any later version. + +// This library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License along +// with this library; see the file COPYING3. If not see +// <http://www.gnu.org/licenses/>. + +#include <unordered_map> +#include <utility> + +struct Key +{ + explicit Key(const int* p) : value(p) { } + ~Key() { value = nullptr; } + + bool operator==(const Key& k) const + { return *value == *k.value; } + + const int* value; +}; + +struct hash +{ + std::size_t operator()(const Key& k) const noexcept + { return *k.value; } +}; + +struct S +{ + int value; + operator Key() const + { return Key(&value); } +}; + +int main() +{ + S s[1] = { {2} }; + std::unordered_set<Key, hash> m(s, s+1); + std::unordered_multiset<Key, hash> mm(s, s+1); + + m.insert(s, s + 1); + mm.insert(s, s + 1); +}