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

huixie90 · tomtor · commit 4b835434f856 · 2025-06-14T19:47:28.000+02:00
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
diff --git a/libcxx/include/__flat_map/flat_map.h b/libcxx/include/__flat_map/flat_map.h
@@ -1012,11 +1012,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
@@ -827,11 +827,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
@@ -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
@@ -698,7 +698,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
@@ -36,6 +36,9 @@ 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;
+
+  // A trait that tells whether a container supports `reserve(n)` member function.
+  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
@@ -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
@@ -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
@@ -1561,6 +1561,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
@@ -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
@@ -1548,6 +1548,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>
@@ -2071,6 +2073,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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -25,6 +25,13 @@ 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;

Original file line number	Diff line number	Diff line change
`@@ -1012,11 +1012,11 @@ class flat_map {`
`1012`	`1012`	`}`
`1013`	`1013`
`1014`	`1014`	`_LIBCPP_HIDE_FROM_ABI void __reserve(size_t __size) {`
`1015`		`- if constexpr (requires { __containers_.keys.reserve(__size); }) {`
	`1015`	`+ if constexpr (__container_traits<_KeyContainer>::__reservable) {`
`1016`	`1016`	`__containers_.keys.reserve(__size);`
`1017`	`1017`	`}`
`1018`	`1018`
`1019`		`- if constexpr (requires { __containers_.values.reserve(__size); }) {`
	`1019`	`+ if constexpr (__container_traits<_MappedContainer>::__reservable) {`
`1020`	`1020`	`__containers_.values.reserve(__size);`
`1021`	`1021`	`}`
`1022`	`1022`	`}`
Original file line number	Diff line number	Diff line change
`@@ -827,11 +827,11 @@ class flat_multimap {`
`827`	`827`	`}`
`828`	`828`
`829`	`829`	`_LIBCPP_HIDE_FROM_ABI void __reserve(size_t __size) {`
`830`		`- if constexpr (requires { __containers_.keys.reserve(__size); }) {`
	`830`	`+ if constexpr (__container_traits<_KeyContainer>::__reservable) {`
`831`	`831`	`__containers_.keys.reserve(__size);`
`832`	`832`	`}`
`833`	`833`
`834`		`- if constexpr (requires { __containers_.values.reserve(__size); }) {`
	`834`	`+ if constexpr (__container_traits<_MappedContainer>::__reservable) {`
`835`	`835`	`__containers_.values.reserve(__size);`
`836`	`836`	`}`
`837`	`837`	`}`
Original file line number	Diff line number	Diff line change
`@@ -667,7 +667,7 @@ class flat_multiset {`
`667`	`667`	`}`
`668`	`668`
`669`	`669`	`_LIBCPP_HIDE_FROM_ABI void __reserve(size_t __size) {`
`670`		`- if constexpr (requires { __keys_.reserve(__size); }) {`
	`670`	`+ if constexpr (__container_traits<_KeyContainer>::__reservable) {`
`671`	`671`	`__keys_.reserve(__size);`
`672`	`672`	`}`
`673`	`673`	`}`
Original file line number	Diff line number	Diff line change
`@@ -698,7 +698,7 @@ class flat_set {`
`698`	`698`	`}`
`699`	`699`
`700`	`700`	`_LIBCPP_HIDE_FROM_ABI void __reserve(size_t __size) {`
`701`		`- if constexpr (requires { __keys_.reserve(__size); }) {`
	`701`	`+ if constexpr (__container_traits<_KeyContainer>::__reservable) {`
`702`	`702`	`__keys_.reserve(__size);`
`703`	`703`	`}`
`704`	`704`	`}`