[libc++] Fix insert() calling incorrect constructors by philnik777 · Pull Request #146231 · llvm/llvm-project

philnik777 · 2025-06-28T18:10:09Z

This fixes insert() calling the wrong allocator_traits::construct in the associative containers by removing the special handling that lead to the inconsistencty inside __tree and __hash_table.

EricWF · 2025-06-29T16:54:30Z

Are you at all concerned by the removal of the optimizations here?

Could we add some tests to establish exactly how much performance is being lost?

philnik777 · 2025-06-30T08:14:22Z

Are you at all concerned by the removal of the optimizations here?

Could we add some tests to establish exactly how much performance is being lost?

Could you elaborate which optimizations you are talking about? AFAICT __emplace_unique does the same unwrapping __insert_unique does (though, granted, with a bit more metaprogramming trickery).

EricWF · 2025-06-30T18:32:00Z

Are you at all concerned by the removal of the optimizations here?
Could we add some tests to establish exactly how much performance is being lost?

Could you elaborate which optimizations you are talking about? AFAICT __emplace_unique does the same unwrapping __insert_unique does (though, granted, with a bit more metaprogramming trickery).

You're right. I misread the diff.

llvmbot · 2025-07-08T18:23:45Z

@llvm/pr-subscribers-libcxx

Author: Nikolas Klauser (philnik777)

Changes

This fixes insert() calling the wrong allocator_traits::construct in the associative containers by removing the special handling that lead to the inconsistencty inside __tree and __hash_table.

Patch is 29.35 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/146231.diff

10 Files Affected:

(modified) libcxx/include/__hash_table (+2-25)
(modified) libcxx/include/__tree (+2-44)
(modified) libcxx/include/ext/hash_map (+4-4)
(modified) libcxx/include/ext/hash_set (+4-4)
(modified) libcxx/include/map (+14-14)
(modified) libcxx/include/set (+12-12)
(modified) libcxx/include/unordered_map (+14-14)
(modified) libcxx/include/unordered_set (+12-12)
(modified) libcxx/test/std/containers/map_allocator_requirement_test_templates.h (+5-5)
(modified) libcxx/test/std/containers/set_allocator_requirement_test_templates.h (+1-1)

diff --git a/libcxx/include/__hash_table b/libcxx/include/__hash_table
index c59d8d5bf383a..78f2f3bfd2f4c 100644
--- a/libcxx/include/__hash_table
+++ b/libcxx/include/__hash_table
@@ -852,15 +852,6 @@ public:
   template <class... _Args>
   _LIBCPP_HIDE_FROM_ABI iterator __emplace_hint_multi(const_iterator __p, _Args&&... __args);
 
-  _LIBCPP_HIDE_FROM_ABI pair<iterator, bool> __insert_unique(__container_value_type&& __x) {
-    return __emplace_unique_key_args(_NodeTypes::__get_key(__x), std::move(__x));
-  }
-
-  template <class _Pp, __enable_if_t<!is_same<__remove_cvref_t<_Pp>, __container_value_type>::value, int> = 0>
-  _LIBCPP_HIDE_FROM_ABI pair<iterator, bool> __insert_unique(_Pp&& __x) {
-    return __emplace_unique(std::forward<_Pp>(__x));
-  }
-
   template <class _ValueT = _Tp, __enable_if_t<__is_hash_value_type<_ValueT>::value, int> = 0>
   _LIBCPP_HIDE_FROM_ABI void __insert_unique_from_orphaned_node(value_type&& __value) {
     using __key_type = typename _NodeTypes::key_type;
@@ -877,16 +868,6 @@ public:
     __h.release();
   }
 
-  template <class _Pp>
-  _LIBCPP_HIDE_FROM_ABI iterator __insert_multi(_Pp&& __x) {
-    return __emplace_multi(std::forward<_Pp>(__x));
-  }
-
-  template <class _Pp>
-  _LIBCPP_HIDE_FROM_ABI iterator __insert_multi(const_iterator __p, _Pp&& __x) {
-    return __emplace_hint_multi(__p, std::forward<_Pp>(__x));
-  }
-
   template <class _ValueT = _Tp, __enable_if_t<__is_hash_value_type<_ValueT>::value, int> = 0>
   _LIBCPP_HIDE_FROM_ABI void __insert_multi_from_orphaned_node(value_type&& __value) {
     using __key_type = typename _NodeTypes::key_type;
@@ -903,10 +884,6 @@ public:
     __h.release();
   }
 
-  _LIBCPP_HIDE_FROM_ABI pair<iterator, bool> __insert_unique(const __container_value_type& __x) {
-    return __emplace_unique_key_args(_NodeTypes::__get_key(__x), __x);
-  }
-
 #if _LIBCPP_STD_VER >= 17
   template <class _NodeHandle, class _InsertReturnType>
   _LIBCPP_HIDE_FROM_ABI _InsertReturnType __node_handle_insert_unique(_NodeHandle&& __nh);
@@ -1336,7 +1313,7 @@ void __hash_table<_Tp, _Hash, _Equal, _Alloc>::__assign_unique(_InputIterator __
     __deallocate_node(__cache);
   }
   for (; __first != __last; ++__first)
-    __insert_unique(*__first);
+    __emplace_unique(*__first);
 }
 
 template <class _Tp, class _Hash, class _Equal, class _Alloc>
@@ -1368,7 +1345,7 @@ void __hash_table<_Tp, _Hash, _Equal, _Alloc>::__assign_multi(_InputIterator __f
     __deallocate_node(__cache);
   }
   for (; __first != __last; ++__first)
-    __insert_multi(_NodeTypes::__get_value(*__first));
+    __emplace_multi(_NodeTypes::__get_value(*__first));
 }
 
 template <class _Tp, class _Hash, class _Equal, class _Alloc>
diff --git a/libcxx/include/__tree b/libcxx/include/__tree
index 9d4ba3ad0639c..b3c0ece8e5fdb 100644
--- a/libcxx/include/__tree
+++ b/libcxx/include/__tree
@@ -1015,32 +1015,6 @@ public:
     return __emplace_hint_unique_key_args(__p, __x.first, std::forward<_Pp>(__x)).first;
   }
 
-  _LIBCPP_HIDE_FROM_ABI pair<iterator, bool> __insert_unique(const value_type& __v) {
-    return __emplace_unique_key_args(__v, __v);
-  }
-
-  _LIBCPP_HIDE_FROM_ABI iterator __insert_unique(const_iterator __p, const value_type& __v) {
-    return __emplace_hint_unique_key_args(__p, __v, __v).first;
-  }
-
-  _LIBCPP_HIDE_FROM_ABI pair<iterator, bool> __insert_unique(value_type&& __v) {
-    return __emplace_unique_key_args(__v, std::move(__v));
-  }
-
-  _LIBCPP_HIDE_FROM_ABI iterator __insert_unique(const_iterator __p, value_type&& __v) {
-    return __emplace_hint_unique_key_args(__p, __v, std::move(__v)).first;
-  }
-
-  template <class _Vp, __enable_if_t<!is_same<__remove_const_ref_t<_Vp>, value_type>::value, int> = 0>
-  _LIBCPP_HIDE_FROM_ABI pair<iterator, bool> __insert_unique(_Vp&& __v) {
-    return __emplace_unique(std::forward<_Vp>(__v));
-  }
-
-  template <class _Vp, __enable_if_t<!is_same<__remove_const_ref_t<_Vp>, value_type>::value, int> = 0>
-  _LIBCPP_HIDE_FROM_ABI iterator __insert_unique(const_iterator __p, _Vp&& __v) {
-    return __emplace_hint_unique(__p, std::forward<_Vp>(__v));
-  }
-
   template <class _ValueT = _Tp, __enable_if_t<__is_tree_value_type<_ValueT>::value, int> = 0>
   _LIBCPP_HIDE_FROM_ABI void
   __insert_unique_from_orphaned_node(const_iterator __p, __get_node_value_type_t<_Tp>&& __value) {
@@ -1052,22 +1026,6 @@ public:
     __emplace_hint_unique(__p, std::move(__value));
   }
 
-  _LIBCPP_HIDE_FROM_ABI iterator __insert_multi(value_type&& __v) { return __emplace_multi(std::move(__v)); }
-
-  _LIBCPP_HIDE_FROM_ABI iterator __insert_multi(const_iterator __p, value_type&& __v) {
-    return __emplace_hint_multi(__p, std::move(__v));
-  }
-
-  template <class _Vp>
-  _LIBCPP_HIDE_FROM_ABI iterator __insert_multi(_Vp&& __v) {
-    return __emplace_multi(std::forward<_Vp>(__v));
-  }
-
-  template <class _Vp>
-  _LIBCPP_HIDE_FROM_ABI iterator __insert_multi(const_iterator __p, _Vp&& __v) {
-    return __emplace_hint_multi(__p, std::forward<_Vp>(__v));
-  }
-
   template <class _ValueT = _Tp, __enable_if_t<__is_tree_value_type<_ValueT>::value, int> = 0>
   _LIBCPP_HIDE_FROM_ABI void __insert_multi_from_orphaned_node(const_iterator __p, value_type&& __value) {
     __emplace_hint_multi(__p, const_cast<key_type&&>(__value.first), std::move(__value.second));
@@ -1360,7 +1318,7 @@ void __tree<_Tp, _Compare, _Allocator>::__assign_unique(_ForwardIterator __first
     }
   }
   for (; __first != __last; ++__first)
-    __insert_unique(*__first);
+    __emplace_unique(*__first);
 }
 
 template <class _Tp, class _Compare, class _Allocator>
@@ -1380,7 +1338,7 @@ void __tree<_Tp, _Compare, _Allocator>::__assign_multi(_InputIterator __first, _
   }
   const_iterator __e = end();
   for (; __first != __last; ++__first)
-    __insert_multi(__e, *__first);
+    __emplace_hint_multi(__e, *__first);
 }
 
 template <class _Tp, class _Compare, class _Allocator>
diff --git a/libcxx/include/ext/hash_map b/libcxx/include/ext/hash_map
index da2a34aa56dfb..d6b92204f4376 100644
--- a/libcxx/include/ext/hash_map
+++ b/libcxx/include/ext/hash_map
@@ -520,7 +520,7 @@ public:
   _LIBCPP_HIDE_FROM_ABI const_iterator end() const { return __table_.end(); }
 
   _LIBCPP_HIDE_FROM_ABI std::pair<iterator, bool> insert(const value_type& __x) {
-    return __table_.__insert_unique(__x);
+    return __table_.__emplace_unique(__x);
   }
   _LIBCPP_HIDE_FROM_ABI iterator insert(const_iterator, const value_type& __x) { return insert(__x).first; }
   template <class _InputIterator>
@@ -625,7 +625,7 @@ template <class _Key, class _Tp, class _Hash, class _Pred, class _Alloc>
 template <class _InputIterator>
 inline void hash_map<_Key, _Tp, _Hash, _Pred, _Alloc>::insert(_InputIterator __first, _InputIterator __last) {
   for (; __first != __last; ++__first)
-    __table_.__insert_unique(*__first);
+    __table_.__emplace_unique(*__first);
 }
 
 template <class _Key, class _Tp, class _Hash, class _Pred, class _Alloc>
@@ -744,7 +744,7 @@ public:
   _LIBCPP_HIDE_FROM_ABI const_iterator begin() const { return __table_.begin(); }
   _LIBCPP_HIDE_FROM_ABI const_iterator end() const { return __table_.end(); }
 
-  _LIBCPP_HIDE_FROM_ABI iterator insert(const value_type& __x) { return __table_.__insert_multi(__x); }
+  _LIBCPP_HIDE_FROM_ABI iterator insert(const value_type& __x) { return __table_.__emplace_unique(__x); }
   _LIBCPP_HIDE_FROM_ABI iterator insert(const_iterator, const value_type& __x) { return insert(__x); }
   template <class _InputIterator>
   _LIBCPP_HIDE_FROM_ABI void insert(_InputIterator __first, _InputIterator __last);
@@ -831,7 +831,7 @@ template <class _Key, class _Tp, class _Hash, class _Pred, class _Alloc>
 template <class _InputIterator>
 inline void hash_multimap<_Key, _Tp, _Hash, _Pred, _Alloc>::insert(_InputIterator __first, _InputIterator __last) {
   for (; __first != __last; ++__first)
-    __table_.__insert_multi(*__first);
+    __table_.__emplace_unique(*__first);
 }
 
 template <class _Key, class _Tp, class _Hash, class _Pred, class _Alloc>
diff --git a/libcxx/include/ext/hash_set b/libcxx/include/ext/hash_set
index 1c94130246b63..7fd5df24ed3a8 100644
--- a/libcxx/include/ext/hash_set
+++ b/libcxx/include/ext/hash_set
@@ -279,7 +279,7 @@ public:
   _LIBCPP_HIDE_FROM_ABI const_iterator end() const { return __table_.end(); }
 
   _LIBCPP_HIDE_FROM_ABI std::pair<iterator, bool> insert(const value_type& __x) {
-    return __table_.__insert_unique(__x);
+    return __table_.__emplace_unique(__x);
   }
   _LIBCPP_HIDE_FROM_ABI iterator insert(const_iterator, const value_type& __x) { return insert(__x).first; }
   template <class _InputIterator>
@@ -365,7 +365,7 @@ template <class _Value, class _Hash, class _Pred, class _Alloc>
 template <class _InputIterator>
 inline void hash_set<_Value, _Hash, _Pred, _Alloc>::insert(_InputIterator __first, _InputIterator __last) {
   for (; __first != __last; ++__first)
-    __table_.__insert_unique(*__first);
+    __table_.__emplace_unique(*__first);
 }
 
 template <class _Value, class _Hash, class _Pred, class _Alloc>
@@ -458,7 +458,7 @@ public:
   _LIBCPP_HIDE_FROM_ABI const_iterator begin() const { return __table_.begin(); }
   _LIBCPP_HIDE_FROM_ABI const_iterator end() const { return __table_.end(); }
 
-  _LIBCPP_HIDE_FROM_ABI iterator insert(const value_type& __x) { return __table_.__insert_multi(__x); }
+  _LIBCPP_HIDE_FROM_ABI iterator insert(const value_type& __x) { return __table_.__emplace_unique(__x); }
   _LIBCPP_HIDE_FROM_ABI iterator insert(const_iterator, const value_type& __x) { return insert(__x); }
   template <class _InputIterator>
   _LIBCPP_HIDE_FROM_ABI void insert(_InputIterator __first, _InputIterator __last);
@@ -543,7 +543,7 @@ template <class _Value, class _Hash, class _Pred, class _Alloc>
 template <class _InputIterator>
 inline void hash_multiset<_Value, _Hash, _Pred, _Alloc>::insert(_InputIterator __first, _InputIterator __last) {
   for (; __first != __last; ++__first)
-    __table_.__insert_multi(*__first);
+    __table_.__emplace_unique(*__first);
 }
 
 template <class _Value, class _Hash, class _Pred, class _Alloc>
diff --git a/libcxx/include/map b/libcxx/include/map
index 6c271392911dc..f441768f2f8e4 100644
--- a/libcxx/include/map
+++ b/libcxx/include/map
@@ -1073,29 +1073,29 @@ public:
 
   template <class _Pp, __enable_if_t<is_constructible<value_type, _Pp>::value, int> = 0>
   _LIBCPP_HIDE_FROM_ABI pair<iterator, bool> insert(_Pp&& __p) {
-    return __tree_.__insert_unique(std::forward<_Pp>(__p));
+    return __tree_.__emplace_unique(std::forward<_Pp>(__p));
   }
 
   template <class _Pp, __enable_if_t<is_constructible<value_type, _Pp>::value, int> = 0>
   _LIBCPP_HIDE_FROM_ABI iterator insert(const_iterator __pos, _Pp&& __p) {
-    return __tree_.__insert_unique(__pos.__i_, std::forward<_Pp>(__p));
+    return __tree_.__emplace_hint_unique(__pos.__i_, std::forward<_Pp>(__p));
   }
 
 #  endif // _LIBCPP_CXX03_LANG
 
-  _LIBCPP_HIDE_FROM_ABI pair<iterator, bool> insert(const value_type& __v) { return __tree_.__insert_unique(__v); }
+  _LIBCPP_HIDE_FROM_ABI pair<iterator, bool> insert(const value_type& __v) { return __tree_.__emplace_unique(__v); }
 
   _LIBCPP_HIDE_FROM_ABI iterator insert(const_iterator __p, const value_type& __v) {
-    return __tree_.__insert_unique(__p.__i_, __v);
+    return __tree_.__emplace_hint_unique(__p.__i_, __v);
   }
 
 #  ifndef _LIBCPP_CXX03_LANG
   _LIBCPP_HIDE_FROM_ABI pair<iterator, bool> insert(value_type&& __v) {
-    return __tree_.__insert_unique(std::move(__v));
+    return __tree_.__emplace_unique(std::move(__v));
   }
 
   _LIBCPP_HIDE_FROM_ABI iterator insert(const_iterator __p, value_type&& __v) {
-    return __tree_.__insert_unique(__p.__i_, std::move(__v));
+    return __tree_.__emplace_hint_unique(__p.__i_, std::move(__v));
   }
 
   _LIBCPP_HIDE_FROM_ABI void insert(initializer_list<value_type> __il) { insert(__il.begin(), __il.end()); }
@@ -1756,34 +1756,34 @@ public:
 
   template <class _Pp, __enable_if_t<is_constructible<value_type, _Pp>::value, int> = 0>
   _LIBCPP_HIDE_FROM_ABI iterator insert(_Pp&& __p) {
-    return __tree_.__insert_multi(std::forward<_Pp>(__p));
+    return __tree_.__emplace_multi(std::forward<_Pp>(__p));
   }
 
   template <class _Pp, __enable_if_t<is_constructible<value_type, _Pp>::value, int> = 0>
   _LIBCPP_HIDE_FROM_ABI iterator insert(const_iterator __pos, _Pp&& __p) {
-    return __tree_.__insert_multi(__pos.__i_, std::forward<_Pp>(__p));
+    return __tree_.__emplace_hint_multi(__pos.__i_, std::forward<_Pp>(__p));
   }
 
-  _LIBCPP_HIDE_FROM_ABI iterator insert(value_type&& __v) { return __tree_.__insert_multi(std::move(__v)); }
+  _LIBCPP_HIDE_FROM_ABI iterator insert(value_type&& __v) { return __tree_.__emplace_multi(std::move(__v)); }
 
   _LIBCPP_HIDE_FROM_ABI iterator insert(const_iterator __p, value_type&& __v) {
-    return __tree_.__insert_multi(__p.__i_, std::move(__v));
+    return __tree_.__emplace_hint_multi(__p.__i_, std::move(__v));
   }
 
   _LIBCPP_HIDE_FROM_ABI void insert(initializer_list<value_type> __il) { insert(__il.begin(), __il.end()); }
 
 #  endif // _LIBCPP_CXX03_LANG
 
-  _LIBCPP_HIDE_FROM_ABI iterator insert(const value_type& __v) { return __tree_.__insert_multi(__v); }
+  _LIBCPP_HIDE_FROM_ABI iterator insert(const value_type& __v) { return __tree_.__emplace_multi(__v); }
 
   _LIBCPP_HIDE_FROM_ABI iterator insert(const_iterator __p, const value_type& __v) {
-    return __tree_.__insert_multi(__p.__i_, __v);
+    return __tree_.__emplace_hint_multi(__p.__i_, __v);
   }
 
   template <class _InputIterator>
   _LIBCPP_HIDE_FROM_ABI void insert(_InputIterator __f, _InputIterator __l) {
     for (const_iterator __e = cend(); __f != __l; ++__f)
-      __tree_.__insert_multi(__e.__i_, *__f);
+      __tree_.__emplace_hint_multi(__e.__i_, *__f);
   }
 
 #  if _LIBCPP_STD_VER >= 23
@@ -1791,7 +1791,7 @@ public:
   _LIBCPP_HIDE_FROM_ABI void insert_range(_Range&& __range) {
     const_iterator __end = cend();
     for (auto&& __element : __range) {
-      __tree_.__insert_multi(__end.__i_, std::forward<decltype(__element)>(__element));
+      __tree_.__emplace_hint_multi(__end.__i_, std::forward<decltype(__element)>(__element));
     }
   }
 #  endif
diff --git a/libcxx/include/set b/libcxx/include/set
index aeea98adf582b..b4272172de5d7 100644
--- a/libcxx/include/set
+++ b/libcxx/include/set
@@ -740,15 +740,15 @@ public:
   }
 #  endif // _LIBCPP_CXX03_LANG
 
-  _LIBCPP_HIDE_FROM_ABI pair<iterator, bool> insert(const value_type& __v) { return __tree_.__insert_unique(__v); }
+  _LIBCPP_HIDE_FROM_ABI pair<iterator, bool> insert(const value_type& __v) { return __tree_.__emplace_unique(__v); }
   _LIBCPP_HIDE_FROM_ABI iterator insert(const_iterator __p, const value_type& __v) {
-    return __tree_.__insert_unique(__p, __v);
+    return __tree_.__emplace_hint_unique(__p, __v);
   }
 
   template <class _InputIterator>
   _LIBCPP_HIDE_FROM_ABI void insert(_InputIterator __f, _InputIterator __l) {
     for (const_iterator __e = cend(); __f != __l; ++__f)
-      __tree_.__insert_unique(__e, *__f);
+      __tree_.__emplace_hint_unique(__e, *__f);
   }
 
 #  if _LIBCPP_STD_VER >= 23
@@ -756,18 +756,18 @@ public:
   _LIBCPP_HIDE_FROM_ABI void insert_range(_Range&& __range) {
     const_iterator __end = cend();
     for (auto&& __element : __range) {
-      __tree_.__insert_unique(__end, std::forward<decltype(__element)>(__element));
+      __tree_.__emplace_hint_unique(__end, std::forward<decltype(__element)>(__element));
     }
   }
 #  endif
 
 #  ifndef _LIBCPP_CXX03_LANG
   _LIBCPP_HIDE_FROM_ABI pair<iterator, bool> insert(value_type&& __v) {
-    return __tree_.__insert_unique(std::move(__v));
+    return __tree_.__emplace_unique(std::move(__v));
   }
 
   _LIBCPP_HIDE_FROM_ABI iterator insert(const_iterator __p, value_type&& __v) {
-    return __tree_.__insert_unique(__p, std::move(__v));
+    return __tree_.__emplace_hint_unique(__p, std::move(__v));
   }
 
   _LIBCPP_HIDE_FROM_ABI void insert(initializer_list<value_type> __il) { insert(__il.begin(), __il.end()); }
@@ -1209,15 +1209,15 @@ public:
   }
 #  endif // _LIBCPP_CXX03_LANG
 
-  _LIBCPP_HIDE_FROM_ABI iterator insert(const value_type& __v) { return __tree_.__insert_multi(__v); }
+  _LIBCPP_HIDE_FROM_ABI iterator insert(const value_type& __v) { return __tree_.__emplace_multi(__v); }
   _LIBCPP_HIDE_FROM_ABI iterator insert(const_iterator __p, const value_type& __v) {
-    return __tree_.__insert_multi(__p, __v);
+    return __tree_.__emplace_hint_multi(__p, __v);
   }
 
   template <class _InputIterator>
   _LIBCPP_HIDE_FROM_ABI void insert(_InputIterator __f, _InputIterator __l) {
     for (const_iterator __e = cend(); __f != __l; ++__f)
-      __tree_.__insert_multi(__e, *__f);
+      __tree_.__emplace_hint_multi(__e, *__f);
   }
 
 #  if _LIBCPP_STD_VER >= 23
@@ -1225,16 +1225,16 @@ public:
   _LIBCPP_HIDE_FROM_ABI void insert_range(_Range&& __range) {
     const_iterator __end = cend();
     for (auto&& __element : __range) {
-      __tree_.__insert_multi(__end, std::forward<decltype(__element)>(__element));
+      __tree_.__emplace_hint_multi(__end, std::forward<decltype(__element)>(__element));
     }
   }
 #  endif
 
 #  ifndef _LIBCPP_CXX03_LANG
-  _LIBCPP_HIDE_FROM_ABI iterator insert(value_type&& __v) { return __tree_.__insert_multi(std::move(__v)); }
+  _LIBCPP_HIDE_FROM_ABI iterator insert(value_type&& __v) { return __tree_.__emplace_multi(std::move(__v)); }
 
   _LIBCPP_HIDE_FROM_ABI iterator insert(const_iterator __p, value_type&& __v) {
-    return __tree_.__insert_multi(__p, std::move(__v));
+    return __tree_.__emplace_hint_multi(__p, std::move(__v));
   }
 
   _LIBCPP_HIDE_FROM_ABI void insert(initializer_list<value_type> __il) { insert(__il.begin(), __il.end()); }
diff --git a/libcxx/include/unordered_map b/libcxx/include/unordered_map
index 484f22ce5d72d..30d7385673adf 100644
--- a/libcxx/include/unordered_map
+++ b/libcxx/include/unordered_map
@@ -1136,7 +1136,7 @@ public:
   _LIBCPP_HIDE_FROM_ABI const_iterator cbegin() const _NOEXCEPT { return __table_.begin(); }
   _LIBCPP_HIDE_FROM_ABI const_iterator cend() const _NOEXCEPT { return __table_.end(); }
 
-  _LIBCPP_HIDE_FROM_ABI pair<iterator, bool> insert(const value_type& __x) { return __table_.__insert_unique(__x); }
+  _LIBCPP_HIDE_FROM_ABI pair<iterator, bool> insert(const value_type& __x) { return __table_.__emplace_unique(__x); }
 
   _LIBCPP_HIDE_FROM_ABI iterator insert(const_iterator, const value_type& __x) { return insert(__x).first; }
 
@@ -1147,7 +1147,7 @@ public:
   template <_ContainerCompatibleRange<value_type> _Range>
   _LIBCPP_HIDE_FROM_ABI void insert_range(_Range&& __range) {
     for (auto&& __element : __range) {
-      __table_.__insert_unique(std::forward<decltype(__element)>(__element));
+      __table_.__emplace_unique(std::forward<decltype(__element)>(__element));
     }
   }
 #  endif
@@ -1156,16 +1156,16 @@ public:
   _LIBCPP_HIDE_FROM_ABI void insert(initializer_list<value_type> __il) { insert(__il.begin(), __il.end()); }
 
   _LIBCPP_HIDE_FROM_ABI pair<iterator, bool> insert(value_type&& __x) {
-    return __table_.__insert_unique(std::move(__x));
+    return __table_.__emplace_unique(std::move(__x));
   }
 
   _LIBCPP_HIDE_FROM_ABI iterator insert(const_iterator, value_type&& __x) {
-    return __table_.__insert_unique(std::move(__x)).first;
+    return __table_.__emplace_unique(std::move(__x)).first;
   }
 
   template <class _Pp, __enable_if_t<is_constructible<value_type, _Pp>::value, int> = 0>
   _LIBCPP_HIDE_FROM_ABI pair<iterator, bool> insert(_Pp&& __x) {
-    return __table_.__insert_unique(std::forward<_Pp>(__x));
+    return __table_.__emplace_unique(std::forward<_Pp>(__x));
   }
 
   template <class _Pp, __enable_if_t<is_constructible<value_type, _Pp>::value, int> = 0>
@@ -1640,7 +1640,7 @@ template <class _Key, class _Tp, class _Hash, class _Pred, class _Alloc>
 template <class _InputIterator>
 inline void unordered_map<_Key, _Tp, _Hash, _Pred, _Alloc>::insert(_InputIterator __first, _InputIterator __last) {
   for (; __first != __last; ++__first)
-    __table_.__insert_unique(*__first);
+    __table_.__emplace_unique(*__first);
 }
 
 #  ifndef _LIBCPP_CXX03_LANG
@@ -1944,10 +1944,10 @@ public:
   _LIBCPP_HIDE_FROM_ABI const_iterator cbegin() const _NOEXCEPT { return __table_.begin(); }
   _LIBCPP_HIDE_FROM_ABI const_iterator cend() const _NOEXCEPT { return __table_.end(); }
 
-  _LIB...
[truncated]

ldionne

Looks good, thanks for the fix!

ldionne · 2025-07-09T14:43:49Z

@StephanTLavavej Apparently this patch fixes an XFAIL you have in your copy of our test suite, FYI!

zygoloid

We're seeing build failures after this change.

zygoloid · 2025-07-15T13:15:57Z

  _LIBCPP_HIDE_FROM_ABI const_iterator end() const { return __table_.end(); }

-  _LIBCPP_HIDE_FROM_ABI iterator insert(const value_type& __x) { return __table_.__insert_multi(__x); }
+  _LIBCPP_HIDE_FROM_ABI iterator insert(const value_type& __x) { return __table_.__emplace_unique(__x); }


Should this be emplace_multi not emplace_unique?

zygoloid · 2025-07-15T13:16:03Z

 inline void hash_multimap<_Key, _Tp, _Hash, _Pred, _Alloc>::insert(_InputIterator __first, _InputIterator __last) {
  for (; __first != __last; ++__first)
-    __table_.__insert_multi(*__first);
+    __table_.__emplace_unique(*__first);


zygoloid · 2025-07-15T13:16:15Z

  _LIBCPP_HIDE_FROM_ABI const_iterator end() const { return __table_.end(); }

-  _LIBCPP_HIDE_FROM_ABI iterator insert(const value_type& __x) { return __table_.__insert_multi(__x); }
+  _LIBCPP_HIDE_FROM_ABI iterator insert(const value_type& __x) { return __table_.__emplace_unique(__x); }


zygoloid · 2025-07-15T13:16:22Z

 inline void hash_multiset<_Value, _Hash, _Pred, _Alloc>::insert(_InputIterator __first, _InputIterator __last) {
  for (; __first != __last; ++__first)
-    __table_.__insert_multi(*__first);
+    __table_.__emplace_unique(*__first);


alexfh · 2025-07-15T17:27:27Z

An example of a compilation error after this commit: https://godbolt.org/z/orqG681hc

alexfh · 2025-07-17T11:09:52Z

An example of a compilation error after this commit: https://godbolt.org/z/orqG681hc

@philnik777 please take a look at this and Richard's comments above.

philnik777 · 2025-07-17T11:52:17Z

@alexfh @zygoloid #149290 should fix it.

EricWF self-assigned this Jun 29, 2025

[libc++] Fix insert() calling incorrect constructors

714e28e

philnik777 force-pushed the tree_fix_insert branch from 5b68b3b to 714e28e Compare July 4, 2025 12:53

philnik777 marked this pull request as ready for review July 8, 2025 18:23

philnik777 requested a review from a team as a code owner July 8, 2025 18:23

llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Jul 8, 2025

ldionne approved these changes Jul 9, 2025

View reviewed changes

philnik777 merged commit afcf76b into llvm:main Jul 10, 2025
126 of 135 checks passed

philnik777 deleted the tree_fix_insert branch July 10, 2025 07:24

zygoloid reviewed Jul 15, 2025

View reviewed changes

Conversation

philnik777 commented Jun 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

EricWF commented Jun 29, 2025

Uh oh!

philnik777 commented Jun 30, 2025

Uh oh!

EricWF commented Jun 30, 2025

Uh oh!

llvmbot commented Jul 8, 2025

Uh oh!

ldionne left a comment

Choose a reason for hiding this comment

Uh oh!

ldionne Jul 9, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

zygoloid left a comment

Choose a reason for hiding this comment

Uh oh!

zygoloid Jul 15, 2025

Choose a reason for hiding this comment

Uh oh!

zygoloid Jul 15, 2025

Choose a reason for hiding this comment

Uh oh!

zygoloid Jul 15, 2025

Choose a reason for hiding this comment

Uh oh!

zygoloid Jul 15, 2025

Choose a reason for hiding this comment

Uh oh!

alexfh commented Jul 15, 2025

Uh oh!

alexfh commented Jul 17, 2025

Uh oh!

philnik777 commented Jul 17, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

philnik777 commented Jun 28, 2025 •

edited

Loading