diff mbox series

Fix std::unordered_map key range insertion

Message ID 860b46c8-fc2a-3024-5fd2-5130ca8c27c8@gmail.com
State New
Headers show
Series Fix std::unordered_map key range insertion | expand

Commit Message

François Dumont Feb. 23, 2023, 9:14 p.m. UTC 
Hi

Based on my work on PR 96088 for std::map I imagine this use case of 
inserting a range of keys into an associative container. It behaves as 
operator[] by inserting a default value for each key.

I wonder if the Standard says that it should work. Or maybe we want to 
support it ?

I haven't checked if it used to work before we introduced the 
_ConvertToValueType type. I have no old enough gcc to do so.

It is not supported by std::map neither, even without 
_ConvertToValueType so I guess it was not working for std::unordered_map 
prior to it.

If it can be considered as a bug I'll create a PR.

François

Comments

François Dumont Feb. 27, 2023, 5:46 a.m. UTC | #1 
Replying to my own questions.

This use case is not valid from a Standard point of view. So not a bug 
and no need to deal with that before next stage 1.

The question left is either we want to support it ?

François

On 23/02/23 22:14, François Dumont wrote:
> Hi
>
> Based on my work on PR 96088 for std::map I imagine this use case of 
> inserting a range of keys into an associative container. It behaves as 
> operator[] by inserting a default value for each key.
>
> I wonder if the Standard says that it should work. Or maybe we want to 
> support it ?
>
> I haven't checked if it used to work before we introduced the 
> _ConvertToValueType type. I have no old enough gcc to do so.
>
> It is not supported by std::map neither, even without 
> _ConvertToValueType so I guess it was not working for 
> std::unordered_map prior to it.
>
> If it can be considered as a bug I'll create a PR.
>
> François
>
diff mbox series

Patch

diff --git a/libstdc++-v3/include/bits/hashtable_policy.h b/libstdc++-v3/include/bits/hashtable_policy.h
index cce4e2844cf..4a89d0ee1c8 100644
--- a/libstdc++-v3/include/bits/hashtable_policy.h
+++ b/libstdc++-v3/include/bits/hashtable_policy.h
@@ -125,15 +125,15 @@  namespace __detail
 	{ return std::forward<_Kt>(__k); }
     };
 
-  template<typename _Value>
-    struct _ConvertToValueType<_Select1st, _Value>
+  template<typename _Key, typename _Value>
+    struct _ConvertToValueType<_Select1st, std::pair<_Key, _Value>>
     {
-      constexpr _Value&&
-      operator()(_Value&& __x) const noexcept
+      constexpr std::pair<_Key, _Value>&&
+      operator()(std::pair<_Key, _Value>&& __x) const noexcept
       { return std::move(__x); }
 
-      constexpr const _Value&
-      operator()(const _Value& __x) const noexcept
+      constexpr const std::pair<_Key, _Value>&
+      operator()(const std::pair<_Key, _Value>& __x) const noexcept
       { return __x; }
 
       template<typename _Kt, typename _Val>
@@ -145,6 +145,26 @@  namespace __detail
 	constexpr const std::pair<_Kt, _Val>&
 	operator()(const std::pair<_Kt, _Val>& __x) const noexcept
 	{ return __x; }
+
+      template<typename _Kt>
+	using __is_cons = std::is_constructible<_Key, _Kt&&>;
+
+      template<typename _Kt>
+	using _IFcons = std::enable_if<__is_cons<_Kt>::value>;
+
+      template<typename _Kt>
+	using _IFconsp = typename _IFcons<_Kt>::type;
+
+      template<typename _Kt, typename = _IFconsp<_Kt>>
+	std::pair<_Kt, _Value>
+	operator()(_Kt&& __kt) const
+	{
+	  return {
+	    std::piecewise_construct,
+	    std::forward_as_tuple(std::forward<_Kt>(__kt)),
+	    std::tuple<>()
+	  };
+	}
     };
 
   template<typename _ExKey>
diff --git a/libstdc++-v3/testsuite/23_containers/unordered_map/96088.cc b/libstdc++-v3/testsuite/23_containers/unordered_map/96088.cc
index 754b529c67c..a3bf6ce5307 100644
--- a/libstdc++-v3/testsuite/23_containers/unordered_map/96088.cc
+++ b/libstdc++-v3/testsuite/23_containers/unordered_map/96088.cc
@@ -264,6 +264,28 @@  test03()
   }
 }
 
+void
+test04()
+{
+  const std::initializer_list<const char*> strlst =
+    { "long_str_for_dynamic_allocating" };
+  __gnu_test::counter::reset();
+  std::unordered_map<std::string, int,
+		     hash_string_view_functor,
+		     std::equal_to<std::string_view>> um;
+  um.insert(strlst.begin(), strlst.end());
+  VERIFY( um.size() == 1 );
+
+  VERIFY( __gnu_test::counter::count() == 3 );
+  VERIFY( __gnu_test::counter::get()._M_increments == 3 );
+
+  um.insert(strlst.begin(), strlst.end());
+  VERIFY( um.size() == 1 );
+
+  VERIFY( __gnu_test::counter::count() == 3 );
+  VERIFY( __gnu_test::counter::get()._M_increments == 3 );
+}
+
 int
 main()
 {
@@ -274,5 +296,6 @@  main()
   test21();
   test22();
   test03();
+  test04();
   return 0;
 }