[libcxx-commits] [libcxx] [libc++] Fix {deque, vector}::append_range assuming too much about the types (PR #162438)
Nikolas Klauser via libcxx-commits
libcxx-commits at lists.llvm.org
Wed Dec 10 06:47:12 PST 2025
https://github.com/philnik777 updated https://github.com/llvm/llvm-project/pull/162438
>From 03df819bd0158647b4ca164aaa5f47fbe6d20a8f Mon Sep 17 00:00:00 2001
From: Nikolas Klauser <nikolasklauser at berlin.de>
Date: Wed, 8 Oct 2025 10:20:55 +0200
Subject: [PATCH] [libc++] Fix {deque,vector}::append_range assuming too much
about the types
---
libcxx/docs/ReleaseNotes/22.rst | 1 +
libcxx/include/__vector/vector.h | 17 +++++-
libcxx/include/deque | 41 ++++++--------
.../sequence/sequence_container_benchmarks.h | 49 ++++++++++++++++-
.../std/containers/insert_range_helpers.h | 3 +
.../deque.modifiers/append_range.pass.cpp | 1 +
.../deque.modifiers/prepend_range.pass.cpp | 2 +
.../prepend_range.pass.cpp | 1 +
.../insert_range_sequence_containers.h | 55 +++++++++++++++++++
.../list/list.modifiers/append_range.pass.cpp | 1 +
.../list.modifiers/prepend_range.pass.cpp | 1 +
.../vector.modifiers/append_range.pass.cpp | 1 +
12 files changed, 148 insertions(+), 25 deletions(-)
diff --git a/libcxx/docs/ReleaseNotes/22.rst b/libcxx/docs/ReleaseNotes/22.rst
index 56eb0e588d81d..170d56d420983 100644
--- a/libcxx/docs/ReleaseNotes/22.rst
+++ b/libcxx/docs/ReleaseNotes/22.rst
@@ -71,6 +71,7 @@ Improvements and New Features
in chunks into a buffer.
- Multiple internal types have been refactored to use ``[[no_unique_address]]``, resulting in faster compile times and
reduced debug information.
+- The performance of ``deque::append_range`` has been improved by up to 3.4x
- The performance of ``std::find`` has been improved by up to 2x for integral types
- The ``std::distance`` and ``std::ranges::distance`` algorithms have been optimized for segmented iterators (e.g.,
diff --git a/libcxx/include/__vector/vector.h b/libcxx/include/__vector/vector.h
index 4961a5fcb2067..b6f3cce3d8d0d 100644
--- a/libcxx/include/__vector/vector.h
+++ b/libcxx/include/__vector/vector.h
@@ -42,6 +42,7 @@
#include <__memory/temp_value.h>
#include <__memory/uninitialized_algorithms.h>
#include <__ranges/access.h>
+#include <__ranges/as_rvalue_view.h>
#include <__ranges/concepts.h>
#include <__ranges/container_compatible_range.h>
#include <__ranges/from_range.h>
@@ -484,7 +485,21 @@ class vector {
#if _LIBCPP_STD_VER >= 23
template <_ContainerCompatibleRange<_Tp> _Range>
_LIBCPP_HIDE_FROM_ABI constexpr void append_range(_Range&& __range) {
- insert_range(end(), std::forward<_Range>(__range));
+ if constexpr (ranges::forward_range<_Range> || ranges::sized_range<_Range>) {
+ auto __len = ranges::distance(__range);
+ if (__len < __cap_ - __end_) {
+ __construct_at_end(ranges::begin(__range), ranges::end(__range), __len);
+ } else {
+ __split_buffer<value_type, allocator_type&> __buffer(__recommend(size() + __len), size(), __alloc_);
+ __buffer.__construct_at_end_with_size(ranges::begin(__range), __len);
+ __swap_out_circular_buffer(__buffer);
+ }
+ } else {
+ vector __buffer(__alloc_);
+ for (auto&& __val : __range)
+ __buffer.emplace_back(std::forward<decltype(__val)>(__val));
+ append_range(ranges::as_rvalue_view(__buffer));
+ }
}
#endif
diff --git a/libcxx/include/deque b/libcxx/include/deque
index ad2d759e1fcac..c866970612890 100644
--- a/libcxx/include/deque
+++ b/libcxx/include/deque
@@ -191,6 +191,7 @@ template <class T, class Allocator, class Predicate>
# include <__algorithm/min.h>
# include <__algorithm/move.h>
# include <__algorithm/move_backward.h>
+# include <__algorithm/ranges_copy_n.h>
# include <__algorithm/remove.h>
# include <__algorithm/remove_if.h>
# include <__assert>
@@ -698,8 +699,16 @@ public:
__assign_with_size_random_access(ranges::begin(__range), __n);
} else if constexpr (ranges::forward_range<_Range> || ranges::sized_range<_Range>) {
- auto __n = static_cast<size_type>(ranges::distance(__range));
- __assign_with_size(ranges::begin(__range), __n);
+ auto __n = ranges::distance(__range);
+ auto __first = ranges::begin(__range);
+
+ auto __result = std::ranges::copy_n(std::move(__first), std::min<size_t>(__n, size()), begin());
+
+ if (static_cast<size_type>(__n) > size()) {
+ __append_with_size(std::move(__result.in), __n - size());
+ } else {
+ __erase_to_end(__result.out);
+ }
} else {
__assign_with_sentinel(ranges::begin(__range), ranges::end(__range));
@@ -815,7 +824,11 @@ public:
template <_ContainerCompatibleRange<_Tp> _Range>
_LIBCPP_HIDE_FROM_ABI void append_range(_Range&& __range) {
- insert_range(end(), std::forward<_Range>(__range));
+ if constexpr (ranges::forward_range<_Range> || ranges::sized_range<_Range>) {
+ __append_with_size(ranges::begin(__range), ranges::distance(__range));
+ } else {
+ __append_with_sentinel(ranges::begin(__range), ranges::end(__range));
+ }
}
# endif
@@ -1201,7 +1214,7 @@ private:
template <class _RandomAccessIterator>
_LIBCPP_HIDE_FROM_ABI void __assign_with_size_random_access(_RandomAccessIterator __f, difference_type __n);
- template <class _Iterator>
+ template <class _AlgPolicy, class _Iterator>
_LIBCPP_HIDE_FROM_ABI void __assign_with_size(_Iterator __f, difference_type __n);
template <class _Iterator, class _Sentinel>
@@ -1461,24 +1474,6 @@ deque<_Tp, _Allocator>::__assign_with_size_random_access(_RandomAccessIterator _
__erase_to_end(std::copy_n(__f, __n, begin()));
}
-template <class _Tp, class _Allocator>
-template <class _Iterator>
-_LIBCPP_HIDE_FROM_ABI void deque<_Tp, _Allocator>::__assign_with_size(_Iterator __f, difference_type __n) {
- if (static_cast<size_type>(__n) > size()) {
- auto __added_size = __n - size();
-
- auto __i = begin();
- for (auto __count = size(); __count != 0; --__count) {
- *__i++ = *__f++;
- }
-
- __append_with_size(__f, __added_size);
-
- } else {
- __erase_to_end(std::copy_n(__f, __n, begin()));
- }
-}
-
template <class _Tp, class _Allocator>
void deque<_Tp, _Allocator>::assign(size_type __n, const value_type& __v) {
if (__n > size()) {
@@ -1803,7 +1798,7 @@ template <class _Iterator>
_LIBCPP_HIDE_FROM_ABI typename deque<_Tp, _Allocator>::iterator
deque<_Tp, _Allocator>::__insert_with_size(const_iterator __p, _Iterator __f, size_type __n) {
__split_buffer<value_type, allocator_type&> __buf(__n, 0, __alloc());
- __buf.__construct_at_end_with_size(__f, __n);
+ __buf.__construct_at_end_with_size(std::move(__f), __n);
typedef typename __split_buffer<value_type, allocator_type&>::iterator __fwd;
return insert(__p, move_iterator<__fwd>(__buf.begin()), move_iterator<__fwd>(__buf.end()));
}
diff --git a/libcxx/test/benchmarks/containers/sequence/sequence_container_benchmarks.h b/libcxx/test/benchmarks/containers/sequence/sequence_container_benchmarks.h
index dcd251d6997dd..6e83e6b0c94e2 100644
--- a/libcxx/test/benchmarks/containers/sequence/sequence_container_benchmarks.h
+++ b/libcxx/test/benchmarks/containers/sequence/sequence_container_benchmarks.h
@@ -309,7 +309,7 @@ void sequence_container_benchmarks(std::string container) {
}
/////////////////////////
- // Variations of push_back
+ // Appending elements
/////////////////////////
static constexpr bool has_push_back = requires(Container c, ValueType v) { c.push_back(v); };
static constexpr bool has_capacity = requires(Container c) { c.capacity(); };
@@ -399,6 +399,53 @@ void sequence_container_benchmarks(std::string container) {
st.ResumeTiming();
}
});
+
+#if TEST_STD_VER >= 23
+ for (auto gen : generators)
+ bench("append_range() (into empty container)" + tostr(gen), [gen](auto& state) {
+ auto const size = state.range(0);
+ std::vector<ValueType> in;
+ std::generate_n(std::back_inserter(in), size, gen);
+ DoNotOptimizeData(in);
+
+ Container c;
+ DoNotOptimizeData(c);
+ for (auto _ : state) {
+ c.append_range(in);
+ DoNotOptimizeData(c);
+
+ state.PauseTiming();
+ c.clear();
+ state.ResumeTiming();
+ }
+ });
+#endif
+ }
+
+ /////////////////////////
+ // Prepending elements
+ /////////////////////////
+ static constexpr bool has_prepend_range = requires(Container c, std::vector<ValueType> v) { c.prepend_range(v); };
+
+ if constexpr (has_prepend_range) {
+ for (auto gen : generators)
+ bench("prepend_range() (into empty container)" + tostr(gen), [gen](auto& state) {
+ auto const size = state.range(0);
+ std::vector<ValueType> in;
+ std::generate_n(std::back_inserter(in), size, gen);
+ DoNotOptimizeData(in);
+
+ Container c;
+ DoNotOptimizeData(c);
+ for (auto _ : state) {
+ c.prepend_range(in);
+ DoNotOptimizeData(c);
+
+ state.PauseTiming();
+ c.clear();
+ state.ResumeTiming();
+ }
+ });
}
/////////////////////////
diff --git a/libcxx/test/std/containers/insert_range_helpers.h b/libcxx/test/std/containers/insert_range_helpers.h
index d7d89f5ea6503..4fbb128d91dc8 100644
--- a/libcxx/test/std/containers/insert_range_helpers.h
+++ b/libcxx/test/std/containers/insert_range_helpers.h
@@ -94,6 +94,9 @@ constexpr void for_all_iterators_and_allocators(Func f) {
f.template operator()<Iter, Iter, safe_allocator<T>>();
}
});
+ // This is added because otherwise there would be no input-only sized range, which has fewer guarantees than a
+ // forward and sized range. We don't want to put it in the for_each above to avoid a combinatorial explosion.
+ f.template operator()<cpp20_input_iterator<PtrT>, sized_sentinel<cpp20_input_iterator<PtrT>>, std::allocator<T>>();
}
// Uses a shorter list of iterator types for use in `constexpr` mode for cases when running the full set in would take
diff --git a/libcxx/test/std/containers/sequences/deque/deque.modifiers/append_range.pass.cpp b/libcxx/test/std/containers/sequences/deque/deque.modifiers/append_range.pass.cpp
index 56a1d226db46f..be91242b3d292 100644
--- a/libcxx/test/std/containers/sequences/deque/deque.modifiers/append_range.pass.cpp
+++ b/libcxx/test/std/containers/sequences/deque/deque.modifiers/append_range.pass.cpp
@@ -31,6 +31,7 @@ int main(int, char**) {
});
});
test_sequence_append_range_move_only<std::deque>();
+ test_sequence_append_range_emplace_constructible<std::deque>();
test_append_range_exception_safety_throwing_copy<std::deque>();
test_append_range_exception_safety_throwing_allocator<std::deque, int>();
diff --git a/libcxx/test/std/containers/sequences/deque/deque.modifiers/prepend_range.pass.cpp b/libcxx/test/std/containers/sequences/deque/deque.modifiers/prepend_range.pass.cpp
index 3154cd389d2f0..5ff572c79e512 100644
--- a/libcxx/test/std/containers/sequences/deque/deque.modifiers/prepend_range.pass.cpp
+++ b/libcxx/test/std/containers/sequences/deque/deque.modifiers/prepend_range.pass.cpp
@@ -31,6 +31,8 @@ int main(int, char**) {
});
});
test_sequence_prepend_range_move_only<std::deque>();
+ // FIXME: This should work - see https://llvm.org/PR162605
+ // test_sequence_prepend_range_emplace_constructible<std::deque>();
test_prepend_range_exception_safety_throwing_copy<std::deque>();
test_prepend_range_exception_safety_throwing_allocator<std::deque, int>();
diff --git a/libcxx/test/std/containers/sequences/forwardlist/forwardlist.modifiers/prepend_range.pass.cpp b/libcxx/test/std/containers/sequences/forwardlist/forwardlist.modifiers/prepend_range.pass.cpp
index c4b9cd9bdfc41..6b601bbd03712 100644
--- a/libcxx/test/std/containers/sequences/forwardlist/forwardlist.modifiers/prepend_range.pass.cpp
+++ b/libcxx/test/std/containers/sequences/forwardlist/forwardlist.modifiers/prepend_range.pass.cpp
@@ -30,6 +30,7 @@ TEST_CONSTEXPR_CXX26 bool test() {
});
});
test_sequence_prepend_range_move_only<std::forward_list>();
+ test_sequence_prepend_range_emplace_constructible<std::forward_list>();
if (!TEST_IS_CONSTANT_EVALUATED) {
test_prepend_range_exception_safety_throwing_copy<std::forward_list>();
diff --git a/libcxx/test/std/containers/sequences/insert_range_sequence_containers.h b/libcxx/test/std/containers/sequences/insert_range_sequence_containers.h
index 9f404c46df778..a3fbd7960b824 100644
--- a/libcxx/test/std/containers/sequences/insert_range_sequence_containers.h
+++ b/libcxx/test/std/containers/sequences/insert_range_sequence_containers.h
@@ -643,6 +643,61 @@ constexpr void test_sequence_assign_range_move_only() {
c.assign_range(in);
}
+struct InPlaceOnly {
+ constexpr InPlaceOnly() {}
+ InPlaceOnly(const InPlaceOnly&) = delete;
+ InPlaceOnly(InPlaceOnly&&) = delete;
+ InPlaceOnly& operator=(const InPlaceOnly&) = delete;
+ InPlaceOnly& operator=(InPlaceOnly&&) = delete;
+};
+
+struct EmplaceConstructible {
+ EmplaceConstructible(const EmplaceConstructible&) = delete;
+ EmplaceConstructible& operator=(const EmplaceConstructible&) = delete;
+ EmplaceConstructible& operator=(EmplaceConstructible&&) = delete;
+ EmplaceConstructible(EmplaceConstructible&&) = delete;
+ constexpr EmplaceConstructible(const InPlaceOnly&) {}
+};
+
+template <template <class...> class Container>
+constexpr void test_sequence_append_range_emplace_constructible() {
+ InPlaceOnly input[5];
+ types::for_each(types::cpp20_input_iterator_list<InPlaceOnly*>{}, [&]<class Iter> {
+ std::ranges::subrange in(Iter(input), sentinel_wrapper<Iter>(Iter(input + 5)));
+ Container<EmplaceConstructible> c;
+ c.append_range(in);
+ });
+}
+
+template <template <class...> class Container>
+constexpr void test_sequence_prepend_range_emplace_constructible() {
+ InPlaceOnly input[5];
+ types::for_each(types::cpp20_input_iterator_list<InPlaceOnly*>{}, [&]<class Iter> {
+ std::ranges::subrange in(Iter(input), sentinel_wrapper<Iter>(Iter(input + 5)));
+ Container<EmplaceConstructible> c;
+ c.prepend_range(in);
+ });
+}
+
+// vector has a special requirement that the type also has to be Cpp17MoveInsertable
+struct EmplaceConstructibleAndMoveInsertable {
+ EmplaceConstructibleAndMoveInsertable(const EmplaceConstructibleAndMoveInsertable&) = delete;
+ EmplaceConstructibleAndMoveInsertable& operator=(const EmplaceConstructibleAndMoveInsertable&) = delete;
+ EmplaceConstructibleAndMoveInsertable& operator=(EmplaceConstructibleAndMoveInsertable&&) = delete;
+ constexpr EmplaceConstructibleAndMoveInsertable(EmplaceConstructibleAndMoveInsertable&&) {}
+ constexpr EmplaceConstructibleAndMoveInsertable(const InPlaceOnly&) {}
+};
+
+template <template <class...> class Container>
+constexpr void test_sequence_append_range_emplace_constructible_and_move_insertable() {
+ InPlaceOnly input[5];
+ types::for_each(types::cpp20_input_iterator_list<InPlaceOnly*>{}, [&]<class Iter> {
+ std::ranges::subrange in(Iter(input), sentinel_wrapper<Iter>(Iter(input + 5)));
+ Container<EmplaceConstructibleAndMoveInsertable> c;
+ c.append_range(in);
+ });
+}
+
// Exception safety.
template <template <class...> class Container>
diff --git a/libcxx/test/std/containers/sequences/list/list.modifiers/append_range.pass.cpp b/libcxx/test/std/containers/sequences/list/list.modifiers/append_range.pass.cpp
index 4b47a8738e525..75dbb5f7a323f 100644
--- a/libcxx/test/std/containers/sequences/list/list.modifiers/append_range.pass.cpp
+++ b/libcxx/test/std/containers/sequences/list/list.modifiers/append_range.pass.cpp
@@ -31,6 +31,7 @@ TEST_CONSTEXPR_CXX26 bool test() {
});
});
test_sequence_append_range_move_only<std::list>();
+ test_sequence_append_range_emplace_constructible<std::list>();
if (!TEST_IS_CONSTANT_EVALUATED) {
test_append_range_exception_safety_throwing_copy<std::list>();
diff --git a/libcxx/test/std/containers/sequences/list/list.modifiers/prepend_range.pass.cpp b/libcxx/test/std/containers/sequences/list/list.modifiers/prepend_range.pass.cpp
index 41f7061c09d28..c163a01d99478 100644
--- a/libcxx/test/std/containers/sequences/list/list.modifiers/prepend_range.pass.cpp
+++ b/libcxx/test/std/containers/sequences/list/list.modifiers/prepend_range.pass.cpp
@@ -31,6 +31,7 @@ TEST_CONSTEXPR_CXX26 bool test() {
});
});
test_sequence_prepend_range_move_only<std::list>();
+ test_sequence_prepend_range_emplace_constructible<std::list>();
if (!TEST_IS_CONSTANT_EVALUATED) {
test_prepend_range_exception_safety_throwing_copy<std::list>();
diff --git a/libcxx/test/std/containers/sequences/vector/vector.modifiers/append_range.pass.cpp b/libcxx/test/std/containers/sequences/vector/vector.modifiers/append_range.pass.cpp
index 7a5031e4676f2..3b4bbf642809a 100644
--- a/libcxx/test/std/containers/sequences/vector/vector.modifiers/append_range.pass.cpp
+++ b/libcxx/test/std/containers/sequences/vector/vector.modifiers/append_range.pass.cpp
@@ -33,6 +33,7 @@ constexpr bool test() {
});
});
test_sequence_append_range_move_only<std::vector>();
+ test_sequence_append_range_emplace_constructible_and_move_insertable<std::vector>();
{ // Vector may or may not need to reallocate because of the insertion -- make sure to test both cases.
{ // Ensure reallocation happens.
More information about the libcxx-commits
mailing list