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

Message ID 2fe3937d-1b9a-a547-bb41-225d3d5426a2@gmail.com
State New
Headers
Series [_Hashtable] Fix insertion of range of type convertible to value_type PR 56112 |

Commit Message

François Dumont Feb. 15, 2022, 9:05 a.m. UTC
  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
  

Comments

François Dumont Feb. 21, 2022, 5:59 p.m. UTC | #1
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
  
Jonathan Wakely Feb. 21, 2022, 8:54 p.m. UTC | #2
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
>
>
>
  
François Dumont Feb. 21, 2022, 9:33 p.m. UTC | #3
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
>
>
  

Patch

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);
+}