[libc++] Do not call `reserve` in flat containers if underlying container is user defined #140379

huixie90 · 2025-05-17T15:28:38Z

This is brought up in the LWG reflector. We currently call reserve if the underlying container has one. But the spec does not specify what reserve should do for Sequence Container. So in theory if the underlying container is user defined type and it can have a function called reserve which does something completely different.

The fix is to just call reserve for STL containers if it has one

…er defined

llvmbot · 2025-05-17T15:29:12Z

@llvm/pr-subscribers-libcxx

Author: Hui (huixie90)

Changes

Full diff: https://github.com/llvm/llvm-project/pull/140379.diff

18 Files Affected:

(modified) libcxx/include/__flat_map/flat_map.h (+2-2)
(modified) libcxx/include/__flat_map/flat_multimap.h (+2-2)
(modified) libcxx/include/__flat_set/flat_multiset.h (+1-1)
(modified) libcxx/include/__flat_set/flat_set.h (+1-1)
(modified) libcxx/include/__type_traits/container_traits.h (+2)
(modified) libcxx/include/__vector/container_traits.h (+2)
(modified) libcxx/include/deque (+2)
(modified) libcxx/include/forward_list (+2)
(modified) libcxx/include/list (+2)
(modified) libcxx/include/map (+4)
(modified) libcxx/include/set (+4)
(modified) libcxx/include/unordered_map (+4)
(modified) libcxx/include/unordered_set (+4)
(modified) libcxx/test/std/containers/container.adaptors/flat.map/flat.map.modifiers/insert_iter_iter.pass.cpp (+9)
(modified) libcxx/test/std/containers/container.adaptors/flat.multimap/flat.multimap.modifiers/insert_iter_iter.pass.cpp (+7)
(modified) libcxx/test/std/containers/container.adaptors/flat.multiset/flat.multiset.modifiers/insert_iter_iter.pass.cpp (+7)
(modified) libcxx/test/std/containers/container.adaptors/flat.set/flat.set.modifiers/insert_iter_iter.pass.cpp (+7)
(modified) libcxx/test/std/containers/container.adaptors/flat_helpers.h (+9)

diff --git a/libcxx/include/__flat_map/flat_map.h b/libcxx/include/__flat_map/flat_map.h
index f5e9756ff6a60..12540a53a5ca0 100644
--- a/libcxx/include/__flat_map/flat_map.h
+++ b/libcxx/include/__flat_map/flat_map.h
@@ -1017,11 +1017,11 @@ class flat_map {
   }
 
   _LIBCPP_HIDE_FROM_ABI void __reserve(size_t __size) {
-    if constexpr (requires { __containers_.keys.reserve(__size); }) {
+    if constexpr (__container_traits<_KeyContainer>::__reservable) {
       __containers_.keys.reserve(__size);
     }
 
-    if constexpr (requires { __containers_.values.reserve(__size); }) {
+    if constexpr (__container_traits<_MappedContainer>::__reservable) {
       __containers_.values.reserve(__size);
     }
   }
diff --git a/libcxx/include/__flat_map/flat_multimap.h b/libcxx/include/__flat_map/flat_multimap.h
index 15fcd7995ad0a..b6f14f7ce1bc0 100644
--- a/libcxx/include/__flat_map/flat_multimap.h
+++ b/libcxx/include/__flat_map/flat_multimap.h
@@ -826,11 +826,11 @@ class flat_multimap {
   }
 
   _LIBCPP_HIDE_FROM_ABI void __reserve(size_t __size) {
-    if constexpr (requires { __containers_.keys.reserve(__size); }) {
+    if constexpr (__container_traits<_KeyContainer>::__reservable) {
       __containers_.keys.reserve(__size);
     }
 
-    if constexpr (requires { __containers_.values.reserve(__size); }) {
+    if constexpr (__container_traits<_MappedContainer>::__reservable) {
       __containers_.values.reserve(__size);
     }
   }
diff --git a/libcxx/include/__flat_set/flat_multiset.h b/libcxx/include/__flat_set/flat_multiset.h
index 0fed377b25e5a..44d8af05a56af 100644
--- a/libcxx/include/__flat_set/flat_multiset.h
+++ b/libcxx/include/__flat_set/flat_multiset.h
@@ -667,7 +667,7 @@ class flat_multiset {
   }
 
   _LIBCPP_HIDE_FROM_ABI void __reserve(size_t __size) {
-    if constexpr (requires { __keys_.reserve(__size); }) {
+    if constexpr (__container_traits<_KeyContainer>::__reservable) {
       __keys_.reserve(__size);
     }
   }
diff --git a/libcxx/include/__flat_set/flat_set.h b/libcxx/include/__flat_set/flat_set.h
index a87496bb9916e..1efcaef87733c 100644
--- a/libcxx/include/__flat_set/flat_set.h
+++ b/libcxx/include/__flat_set/flat_set.h
@@ -699,7 +699,7 @@ class flat_set {
   }
 
   _LIBCPP_HIDE_FROM_ABI void __reserve(size_t __size) {
-    if constexpr (requires { __keys_.reserve(__size); }) {
+    if constexpr (__container_traits<_KeyContainer>::__reservable) {
       __keys_.reserve(__size);
     }
   }
diff --git a/libcxx/include/__type_traits/container_traits.h b/libcxx/include/__type_traits/container_traits.h
index 5262cef94c61e..f0c3697a3943c 100644
--- a/libcxx/include/__type_traits/container_traits.h
+++ b/libcxx/include/__type_traits/container_traits.h
@@ -36,6 +36,8 @@ struct __container_traits {
   // `insert(...)` or `emplace(...)` has strong exception guarantee, that is, if the function
   // exits via an exception, the original container is unaffected
   static _LIBCPP_CONSTEXPR const bool __emplacement_has_strong_exception_safety_guarantee = false;
+
+  static _LIBCPP_CONSTEXPR const bool __reservable = false;
 };
 
 _LIBCPP_END_NAMESPACE_STD
diff --git a/libcxx/include/__vector/container_traits.h b/libcxx/include/__vector/container_traits.h
index 337b9c5c83687..20f2b9ef042cb 100644
--- a/libcxx/include/__vector/container_traits.h
+++ b/libcxx/include/__vector/container_traits.h
@@ -32,6 +32,8 @@ struct __container_traits<vector<_Tp, _Allocator> > {
   //  the effects are unspecified.
   static _LIBCPP_CONSTEXPR const bool __emplacement_has_strong_exception_safety_guarantee =
       is_nothrow_move_constructible<_Tp>::value || __is_cpp17_copy_insertable_v<_Allocator>;
+
+  static _LIBCPP_CONSTEXPR const bool __reservable = true;
 };
 
 _LIBCPP_END_NAMESPACE_STD
diff --git a/libcxx/include/deque b/libcxx/include/deque
index d8645d06ae59e..5b9aaf8788a63 100644
--- a/libcxx/include/deque
+++ b/libcxx/include/deque
@@ -2631,6 +2631,8 @@ struct __container_traits<deque<_Tp, _Allocator> > {
   //  non-Cpp17CopyInsertable T, the effects are unspecified.
   static _LIBCPP_CONSTEXPR const bool __emplacement_has_strong_exception_safety_guarantee =
       is_nothrow_move_constructible<_Tp>::value || __is_cpp17_copy_insertable_v<_Allocator>;
+
+  static _LIBCPP_CONSTEXPR const bool __reservable = false;
 };
 
 _LIBCPP_END_NAMESPACE_STD
diff --git a/libcxx/include/forward_list b/libcxx/include/forward_list
index f264f567c9bb3..844f860af44cc 100644
--- a/libcxx/include/forward_list
+++ b/libcxx/include/forward_list
@@ -1567,6 +1567,8 @@ struct __container_traits<forward_list<_Tp, _Allocator> > {
   // - If an exception is thrown by an insert() or emplace() function while inserting a single element, that
   // function has no effects.
   static _LIBCPP_CONSTEXPR const bool __emplacement_has_strong_exception_safety_guarantee = true;
+
+  static _LIBCPP_CONSTEXPR const bool __reservable = false;
 };
 
 _LIBCPP_END_NAMESPACE_STD
diff --git a/libcxx/include/list b/libcxx/include/list
index d1da347b9bd13..98610f59ed74a 100644
--- a/libcxx/include/list
+++ b/libcxx/include/list
@@ -1718,6 +1718,8 @@ struct __container_traits<list<_Tp, _Allocator> > {
   // - If an exception is thrown by an insert() or emplace() function while inserting a single element, that
   // function has no effects.
   static _LIBCPP_CONSTEXPR const bool __emplacement_has_strong_exception_safety_guarantee = true;
+
+  static _LIBCPP_CONSTEXPR const bool __reservable = false;
 };
 
 _LIBCPP_END_NAMESPACE_STD
diff --git a/libcxx/include/map b/libcxx/include/map
index 039ed86dc756f..8f266d29816b7 100644
--- a/libcxx/include/map
+++ b/libcxx/include/map
@@ -1561,6 +1561,8 @@ struct __container_traits<map<_Key, _Tp, _Compare, _Allocator> > {
   // For associative containers, if an exception is thrown by any operation from within
   // an insert or emplace function inserting a single element, the insertion has no effect.
   static _LIBCPP_CONSTEXPR const bool __emplacement_has_strong_exception_safety_guarantee = true;
+
+  static _LIBCPP_CONSTEXPR const bool __reservable = false;
 };
 
 template <class _Key, class _Tp, class _Compare, class _Allocator>
@@ -2085,6 +2087,8 @@ struct __container_traits<multimap<_Key, _Tp, _Compare, _Allocator> > {
   // For associative containers, if an exception is thrown by any operation from within
   // an insert or emplace function inserting a single element, the insertion has no effect.
   static _LIBCPP_CONSTEXPR const bool __emplacement_has_strong_exception_safety_guarantee = true;
+
+  static _LIBCPP_CONSTEXPR const bool __reservable = false;
 };
 
 _LIBCPP_END_NAMESPACE_STD
diff --git a/libcxx/include/set b/libcxx/include/set
index 83c8ed59065c9..aeea98adf582b 100644
--- a/libcxx/include/set
+++ b/libcxx/include/set
@@ -1030,6 +1030,8 @@ struct __container_traits<set<_Key, _Compare, _Allocator> > {
   // For associative containers, if an exception is thrown by any operation from within
   // an insert or emplace function inserting a single element, the insertion has no effect.
   static _LIBCPP_CONSTEXPR const bool __emplacement_has_strong_exception_safety_guarantee = true;
+
+  static _LIBCPP_CONSTEXPR const bool __reservable = false;
 };
 
 template <class _Key, class _Compare, class _Allocator>
@@ -1499,6 +1501,8 @@ struct __container_traits<multiset<_Key, _Compare, _Allocator> > {
   // For associative containers, if an exception is thrown by any operation from within
   // an insert or emplace function inserting a single element, the insertion has no effect.
   static _LIBCPP_CONSTEXPR const bool __emplacement_has_strong_exception_safety_guarantee = true;
+
+  static _LIBCPP_CONSTEXPR const bool __reservable = false;
 };
 
 _LIBCPP_END_NAMESPACE_STD
diff --git a/libcxx/include/unordered_map b/libcxx/include/unordered_map
index 61c89a0ca73bb..b7f333e5f1178 100644
--- a/libcxx/include/unordered_map
+++ b/libcxx/include/unordered_map
@@ -1843,6 +1843,8 @@ struct __container_traits<unordered_map<_Key, _Tp, _Hash, _Pred, _Alloc> > {
   //  inserting a single element, the insertion has no effect.
   static _LIBCPP_CONSTEXPR const bool __emplacement_has_strong_exception_safety_guarantee =
       __is_nothrow_invocable_v<_Hash, const _Key&>;
+
+  static _LIBCPP_CONSTEXPR const bool __reservable = true;
 };
 
 template <class _Key,
@@ -2543,6 +2545,8 @@ struct __container_traits<unordered_multimap<_Key, _Tp, _Hash, _Pred, _Alloc> >
   //  inserting a single element, the insertion has no effect.
   static _LIBCPP_CONSTEXPR const bool __emplacement_has_strong_exception_safety_guarantee =
       __is_nothrow_invocable_v<_Hash, const _Key&>;
+
+  static _LIBCPP_CONSTEXPR const bool __reservable = true;
 };
 
 _LIBCPP_END_NAMESPACE_STD
diff --git a/libcxx/include/unordered_set b/libcxx/include/unordered_set
index e97e6e8042e60..1412badbe37f8 100644
--- a/libcxx/include/unordered_set
+++ b/libcxx/include/unordered_set
@@ -1196,6 +1196,8 @@ struct __container_traits<unordered_set<_Value, _Hash, _Pred, _Alloc> > {
   //  inserting a single element, the insertion has no effect.
   static _LIBCPP_CONSTEXPR const bool __emplacement_has_strong_exception_safety_guarantee =
       __is_nothrow_invocable_v<_Hash, const _Value&>;
+
+  static _LIBCPP_CONSTEXPR const bool __reservable = true;
 };
 
 template <class _Value, class _Hash = hash<_Value>, class _Pred = equal_to<_Value>, class _Alloc = allocator<_Value> >
@@ -1816,6 +1818,8 @@ struct __container_traits<unordered_multiset<_Value, _Hash, _Pred, _Alloc> > {
   //  inserting a single element, the insertion has no effect.
   static _LIBCPP_CONSTEXPR const bool __emplacement_has_strong_exception_safety_guarantee =
       __is_nothrow_invocable_v<_Hash, const _Value&>;
+
+  static _LIBCPP_CONSTEXPR const bool __reservable = true;
 };
 
 _LIBCPP_END_NAMESPACE_STD
diff --git a/libcxx/test/std/containers/container.adaptors/flat.map/flat.map.modifiers/insert_iter_iter.pass.cpp b/libcxx/test/std/containers/container.adaptors/flat.map/flat.map.modifiers/insert_iter_iter.pass.cpp
index 8455b19475fe4..ccce117c90fca 100644
--- a/libcxx/test/std/containers/container.adaptors/flat.map/flat.map.modifiers/insert_iter_iter.pass.cpp
+++ b/libcxx/test/std/containers/container.adaptors/flat.map/flat.map.modifiers/insert_iter_iter.pass.cpp
@@ -13,6 +13,7 @@
 // template <class InputIterator>
 //   void insert(InputIterator first, InputIterator last);
 
+#include <algorithm>
 #include <flat_map>
 #include <cassert>
 #include <functional>
@@ -75,6 +76,7 @@ void test() {
   M expected2{{0, 1}, {1, 1}, {2, 1}, {3, 1}, {4, 1}};
   assert(m == expected2);
 }
+
 int main(int, char**) {
   test<std::vector<int>, std::vector<double>>();
   test<std::deque<int>, std::vector<double>>();
@@ -85,5 +87,12 @@ int main(int, char**) {
     auto insert_func = [](auto& m, const auto& newValues) { m.insert(newValues.begin(), newValues.end()); };
     test_insert_range_exception_guarantee(insert_func);
   }
+  {
+    std::flat_map<int, int, std::less<int>, SillyReserveVector<int>, SillyReserveVector<int>> m{{1, 1}, {2, 2}};
+    std::vector<std::pair<int, int>> v{{3, 3}, {4, 4}};
+    m.insert(v.begin(), v.end());
+    assert(std::ranges::equal(m, std::vector<std::pair<int, int>>{{1, 1}, {2, 2}, {3, 3}, {4, 4}}));
+  }
+
   return 0;
 }
diff --git a/libcxx/test/std/containers/container.adaptors/flat.multimap/flat.multimap.modifiers/insert_iter_iter.pass.cpp b/libcxx/test/std/containers/container.adaptors/flat.multimap/flat.multimap.modifiers/insert_iter_iter.pass.cpp
index ae031bd010f76..30cb89dadbfe0 100644
--- a/libcxx/test/std/containers/container.adaptors/flat.multimap/flat.multimap.modifiers/insert_iter_iter.pass.cpp
+++ b/libcxx/test/std/containers/container.adaptors/flat.multimap/flat.multimap.modifiers/insert_iter_iter.pass.cpp
@@ -16,6 +16,7 @@
 //   void insert(InputIterator first, InputIterator last);
 
 #include <flat_map>
+#include <algorithm>
 #include <cassert>
 #include <functional>
 #include <deque>
@@ -105,5 +106,11 @@ int main(int, char**) {
     auto insert_func = [](auto& m, const auto& newValues) { m.insert(newValues.begin(), newValues.end()); };
     test_insert_range_exception_guarantee(insert_func);
   }
+  {
+    std::flat_multimap<int, int, std::less<int>, SillyReserveVector<int>, SillyReserveVector<int>> m{{1, 1}, {2, 2}};
+    std::vector<std::pair<int, int>> v{{3, 3}, {4, 4}};
+    m.insert(v.begin(), v.end());
+    assert(std::ranges::equal(m, std::vector<std::pair<int, int>>{{1, 1}, {2, 2}, {3, 3}, {4, 4}}));
+  }
   return 0;
 }
diff --git a/libcxx/test/std/containers/container.adaptors/flat.multiset/flat.multiset.modifiers/insert_iter_iter.pass.cpp b/libcxx/test/std/containers/container.adaptors/flat.multiset/flat.multiset.modifiers/insert_iter_iter.pass.cpp
index 3505e097cca69..93815686787c4 100644
--- a/libcxx/test/std/containers/container.adaptors/flat.multiset/flat.multiset.modifiers/insert_iter_iter.pass.cpp
+++ b/libcxx/test/std/containers/container.adaptors/flat.multiset/flat.multiset.modifiers/insert_iter_iter.pass.cpp
@@ -14,6 +14,7 @@
 //   void insert(InputIterator first, InputIterator last);
 
 #include <flat_set>
+#include <algorithm>
 #include <cassert>
 #include <functional>
 #include <deque>
@@ -79,6 +80,12 @@ void test() {
   test_one<std::deque<int>>();
   test_one<MinSequenceContainer<int>>();
   test_one<std::vector<int, min_allocator<int>>>();
+  {
+    std::flat_multiset<int, std::less<int>, SillyReserveVector<int>> m{1, 2};
+    std::vector<int> v{3, 4};
+    m.insert(v.begin(), v.end());
+    assert(std::ranges::equal(m, std::vector<int>{1, 2, 3, 4}));
+  }
 }
 
 void test_exception() {
diff --git a/libcxx/test/std/containers/container.adaptors/flat.set/flat.set.modifiers/insert_iter_iter.pass.cpp b/libcxx/test/std/containers/container.adaptors/flat.set/flat.set.modifiers/insert_iter_iter.pass.cpp
index 8063686c960ed..18bdb798de01f 100644
--- a/libcxx/test/std/containers/container.adaptors/flat.set/flat.set.modifiers/insert_iter_iter.pass.cpp
+++ b/libcxx/test/std/containers/container.adaptors/flat.set/flat.set.modifiers/insert_iter_iter.pass.cpp
@@ -14,6 +14,7 @@
 //   void insert(InputIterator first, InputIterator last);
 
 #include <flat_set>
+#include <algorithm>
 #include <cassert>
 #include <functional>
 #include <deque>
@@ -79,6 +80,12 @@ void test() {
   test_one<std::deque<int>>();
   test_one<MinSequenceContainer<int>>();
   test_one<std::vector<int, min_allocator<int>>>();
+  {
+    std::flat_set<int, std::less<int>, SillyReserveVector<int>> m{1, 2};
+    std::vector<int> v{3, 4};
+    m.insert(v.begin(), v.end());
+    assert(std::ranges::equal(m, std::vector<int>{1, 2, 3, 4}));
+  }
 }
 
 void test_exception() {
diff --git a/libcxx/test/std/containers/container.adaptors/flat_helpers.h b/libcxx/test/std/containers/container.adaptors/flat_helpers.h
index 9cd408ef960a9..458ad24e47932 100644
--- a/libcxx/test/std/containers/container.adaptors/flat_helpers.h
+++ b/libcxx/test/std/containers/container.adaptors/flat_helpers.h
@@ -25,6 +25,15 @@ struct CopyOnlyVector : std::vector<T> {
   CopyOnlyVector& operator=(CopyOnlyVector& other) { return this->operator=(other); }
 };
 
+template <class T>
+struct SillyReserveVector : std::vector<T> {
+  using std::vector<T>::vector;
+
+  void reserve(size_t ) {
+    this->clear();
+  }
+};
+
 template <class T, bool ConvertibleToT = false>
 struct Transparent {
   T t;

github-actions · 2025-05-17T15:30:53Z

✅ With the latest revision this PR passed the C/C++ code formatter.

[libc++] Do not call in flat containers if underlying container is us…

860639f

…er defined

huixie90 requested a review from a team as a code owner May 17, 2025 15:28

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

clang format

d5355cf

huixie90 changed the title ~~[libc++] Do not call in flat containers if underlying container is user defined~~ [libc++] Do not call reserve in flat containers if underlying container is user defined May 17, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[libc++] Do not call `reserve` in flat containers if underlying container is user defined #140379

[libc++] Do not call `reserve` in flat containers if underlying container is user defined #140379

huixie90 commented May 17, 2025 •

edited

Loading

llvmbot commented May 17, 2025

github-actions bot commented May 17, 2025 •

edited

Loading

[libc++] Do not call reserve in flat containers if underlying container is user defined #140379

Are you sure you want to change the base?

[libc++] Do not call reserve in flat containers if underlying container is user defined #140379

Conversation

huixie90 commented May 17, 2025 • edited Loading

llvmbot commented May 17, 2025

github-actions bot commented May 17, 2025 • edited Loading

[libc++] Do not call `reserve` in flat containers if underlying container is user defined #140379

[libc++] Do not call `reserve` in flat containers if underlying container is user defined #140379

huixie90 commented May 17, 2025 •

edited

Loading

github-actions bot commented May 17, 2025 •

edited

Loading