[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

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.

ldionne · 2025-05-21T15:14:53Z

@Michael137 We've been seeing the LLDB tests that we run as part of libc++'s bootstrapping build fail for a few days (~5 days roughly). This is an example error: https://github.com/llvm/llvm-project/actions/runs/15086636371/job/42430861883?pr=140379. Does this ring a bell? @philnik777 tried reproducing locally (outside of the Docker image) and couldn't reproduce. Have you seen this kind of error in your CI setup before?

frederick-vs-ja · 2025-05-27T04:12:22Z

@Michael137 We've been seeing the LLDB tests that we run as part of libc++'s bootstrapping build fail for a few days (~5 days roughly). This is an example error: https://github.com/llvm/llvm-project/actions/runs/15086636371/job/42430861883?pr=140379. Does this ring a bell? @philnik777 tried reproducing locally (outside of the Docker image) and couldn't reproduce. Have you seen this kind of error in your CI setup before?

I'm attempting to fix this now.

Michael137 · 2025-05-27T08:32:48Z

Thanks for looking at that. I was out for couple of days. Let me comment on that PR

ldionne

LGTM with documentation change!

libcxx/include/__type_traits/container_traits.h

…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

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

ldionne approved these changes May 27, 2025

View reviewed changes

libcxx/include/__type_traits/container_traits.h Show resolved Hide resolved

huixie90 added 3 commits May 31, 2025 12:04

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

253c1b9

…er defined

clang format

f661248

documentation

842dd4a

huixie90 force-pushed the hxie/fix_reserve branch from 245481a to 842dd4a Compare May 31, 2025 11:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[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

Uh oh!

huixie90 commented May 17, 2025 •

edited

Loading

Uh oh!

llvmbot commented May 17, 2025

Uh oh!

github-actions bot commented May 17, 2025 •

edited

Loading

Uh oh!

ldionne commented May 21, 2025

Uh oh!

frederick-vs-ja commented May 27, 2025

Uh oh!

Michael137 commented May 27, 2025

Uh oh!

ldionne left a comment

Uh oh!

Uh oh!

Uh oh!

[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

Uh oh!

Conversation

huixie90 commented May 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented May 17, 2025

Uh oh!

github-actions bot commented May 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ldionne commented May 21, 2025

Uh oh!

frederick-vs-ja commented May 27, 2025

Uh oh!

Michael137 commented May 27, 2025

Uh oh!

ldionne left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

[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