[libcxx-commits] [libcxx] [libc++] Fix insertion into `deque` from prvalue ranges (PR #160022)
via libcxx-commits
libcxx-commits at lists.llvm.org
Sun Sep 21 19:27:38 PDT 2025
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-libcxx
Author: A. Jiang (frederick-vs-ja)
<details>
<summary>Changes</summary>
When the iterator of the source range in `*_range` functions dereferences to a prvalue, it should be at most _Cpp17InputIterator_ while possibly modeling `random_access_iterator`. When inserting such a range into a container, we should use `ranges::prev`/`ranges::next` instead of `std::prev`/`std::next` internally.
In addition to `deque`, test coverage for prvalue ranges is added for all sequence containers. We can cover other containers and container adaptors later.
Fixes #<!-- -->159943.
---
Full diff: https://github.com/llvm/llvm-project/pull/160022.diff
3 Files Affected:
- (modified) libcxx/include/deque (+9-2)
- (modified) libcxx/test/std/containers/from_range_helpers.h (+35)
- (modified) libcxx/test/std/containers/sequences/insert_range_sequence_containers.h (+84-26)
``````````diff
diff --git a/libcxx/include/deque b/libcxx/include/deque
index 98d1dbbddb7e8..ea83c50bceea0 100644
--- a/libcxx/include/deque
+++ b/libcxx/include/deque
@@ -185,6 +185,7 @@ template <class T, class Allocator, class Predicate>
# include <__algorithm/copy_n.h>
# include <__algorithm/equal.h>
# include <__algorithm/fill_n.h>
+# include <__algorithm/iterator_operations.h>
# include <__algorithm/lexicographical_compare.h>
# include <__algorithm/lexicographical_compare_three_way.h>
# include <__algorithm/max.h>
@@ -1916,6 +1917,12 @@ template <class _Tp, class _Allocator>
template <class _BiIter>
_LIBCPP_HIDE_FROM_ABI typename deque<_Tp, _Allocator>::iterator
deque<_Tp, _Allocator>::__insert_bidirectional(const_iterator __p, _BiIter __f, _BiIter __l, size_type __n) {
+# if _LIBCPP_STD_VER >= 20
+ using _Ops = _IterOps<
+ conditional_t<__has_bidirectional_iterator_category<_BiIter>::value, _ClassicAlgPolicy, _RangeAlgPolicy>>;
+# else
+ using _Ops = _IterOps<_ClassicAlgPolicy>;
+# endif
size_type __pos = __p - begin();
size_type __to_end = size() - __pos;
allocator_type& __a = __alloc();
@@ -1928,7 +1935,7 @@ deque<_Tp, _Allocator>::__insert_bidirectional(const_iterator __p, _BiIter __f,
iterator __i = __old_begin;
_BiIter __m = __f;
if (__n > __pos) {
- __m = __pos < __n / 2 ? std::prev(__l, __pos) : std::next(__f, __n - __pos);
+ __m = __pos < __n / 2 ? _Ops::prev(__l, __pos) : _Ops::next(__f, __n - __pos);
for (_BiIter __j = __m; __j != __f; --__start_, ++__size())
__alloc_traits::construct(__a, std::addressof(*--__i), *--__j);
__n = __pos;
@@ -1955,7 +1962,7 @@ deque<_Tp, _Allocator>::__insert_bidirectional(const_iterator __p, _BiIter __f,
_BiIter __m = __l;
size_type __de = size() - __pos;
if (__n > __de) {
- __m = __de < __n / 2 ? std::next(__f, __de) : std::prev(__l, __n - __de);
+ __m = __de < __n / 2 ? _Ops::next(__f, __de) : _Ops::prev(__l, __n - __de);
for (_BiIter __j = __m; __j != __l; ++__i, (void)++__j, ++__size())
__alloc_traits::construct(__a, std::addressof(*__i), *__j);
__n = __de;
diff --git a/libcxx/test/std/containers/from_range_helpers.h b/libcxx/test/std/containers/from_range_helpers.h
index edf1d6ce528d7..45cbd803cac8c 100644
--- a/libcxx/test/std/containers/from_range_helpers.h
+++ b/libcxx/test/std/containers/from_range_helpers.h
@@ -10,9 +10,12 @@
#define SUPPORT_FROM_RANGE_HELPERS_H
#include <array>
+#include <concepts>
#include <cstddef>
#include <iterator>
+#include <ranges>
#include <type_traits>
+#include <utility>
#include <vector>
#include "min_allocator.h"
@@ -50,6 +53,38 @@ constexpr auto wrap_input(std::vector<T>& input) {
return std::ranges::subrange(std::move(b), std::move(e));
}
+struct DecayCopy {
+ template <class T>
+ requires std::convertible_to<T, std::decay_t<T>>
+ constexpr std::decay_t<T> operator()(T&& t) {
+ return std::forward<T>(t);
+ }
+};
+
+template <class Iter, class Sent, std::ranges::input_range Range>
+constexpr auto wrap_input_decay(Range&& input) {
+ auto b = Iter(std::ranges::begin(input));
+ auto e = Sent(Iter(std::ranges::end(input)));
+ if constexpr (std::is_reference_v<std::iter_reference_t<Iter>>)
+ return std::ranges::subrange(std::move(b), std::move(e)) | std::views::transform(DecayCopy{});
+ else
+ return std::ranges::subrange(std::move(b), std::move(e));
+}
+
+template <class Iter, class Sent, class T, std::size_t N>
+constexpr auto wrap_input_decay(std::array<T, N>& input) {
+ auto b = Iter(input.data());
+ auto e = Sent(Iter(input.data() + input.size()));
+ return std::ranges::subrange(std::move(b), std::move(e)) | std::views::transform(DecayCopy{});
+}
+
+template <class Iter, class Sent, class T>
+constexpr auto wrap_input_decay(std::vector<T>& input) {
+ auto b = Iter(input.data());
+ auto e = Sent(Iter(input.data() + input.size()));
+ return std::ranges::subrange(std::move(b), std::move(e)) | std::views::transform(DecayCopy{});
+}
+
struct KeyValue {
int key; // Only the key is considered for equality comparison.
char value; // Allows distinguishing equivalent instances.
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..07f069e3176ce 100644
--- a/libcxx/test/std/containers/sequences/insert_range_sequence_containers.h
+++ b/libcxx/test/std/containers/sequences/insert_range_sequence_containers.h
@@ -402,14 +402,30 @@ constexpr void test_sequence_insert_range(Validate validate) {
auto get_pos = [](auto& c, auto& test_case) { return std::ranges::next(c.begin(), static_cast<D>(test_case.index)); };
auto test = [&](auto& test_case) {
- Container c(test_case.initial.begin(), test_case.initial.end());
- auto in = wrap_input<Iter, Sent>(test_case.input);
- auto pos = get_pos(c, test_case);
-
- auto result = c.insert_range(pos, in);
- assert(result == get_pos(c, test_case));
- validate(c);
- return std::ranges::equal(c, test_case.expected);
+ {
+ Container c(test_case.initial.begin(), test_case.initial.end());
+ auto in = wrap_input<Iter, Sent>(test_case.input);
+ auto pos = get_pos(c, test_case);
+
+ auto result = c.insert_range(pos, in);
+ assert(result == get_pos(c, test_case));
+ validate(c);
+ if (!std::ranges::equal(c, test_case.expected))
+ return false;
+ }
+ {
+ Container c(test_case.initial.begin(), test_case.initial.end());
+ auto in = wrap_input_decay<Iter, Sent>(test_case.input);
+ auto pos = get_pos(c, test_case);
+
+ auto result = c.insert_range(pos, in);
+ assert(result == get_pos(c, test_case));
+ validate(c);
+ if (!std::ranges::equal(c, test_case.expected))
+ return false;
+ }
+
+ return true;
};
{ // Empty container.
@@ -469,12 +485,26 @@ constexpr void test_sequence_prepend_range(Validate validate) {
using T = typename Container::value_type;
auto test = [&](auto& test_case) {
- Container c(test_case.initial.begin(), test_case.initial.end());
- auto in = wrap_input<Iter, Sent>(test_case.input);
-
- c.prepend_range(in);
- validate(c);
- return std::ranges::equal(c, test_case.expected);
+ {
+ Container c(test_case.initial.begin(), test_case.initial.end());
+ auto in = wrap_input<Iter, Sent>(test_case.input);
+
+ c.prepend_range(in);
+ validate(c);
+ if (!std::ranges::equal(c, test_case.expected))
+ return false;
+ }
+ {
+ Container c(test_case.initial.begin(), test_case.initial.end());
+ auto in = wrap_input_decay<Iter, Sent>(test_case.input);
+
+ c.prepend_range(in);
+ validate(c);
+ if (!std::ranges::equal(c, test_case.expected))
+ return false;
+ }
+
+ return true;
};
{ // Empty container.
@@ -512,12 +542,26 @@ constexpr void test_sequence_append_range(Validate validate) {
using T = typename Container::value_type;
auto test = [&](auto& test_case) {
- Container c(test_case.initial.begin(), test_case.initial.end());
- auto in = wrap_input<Iter, Sent>(test_case.input);
-
- c.append_range(in);
- validate(c);
- return std::ranges::equal(c, test_case.expected);
+ {
+ Container c(test_case.initial.begin(), test_case.initial.end());
+ auto in = wrap_input<Iter, Sent>(test_case.input);
+
+ c.append_range(in);
+ validate(c);
+ if (!std::ranges::equal(c, test_case.expected))
+ return false;
+ }
+ {
+ Container c(test_case.initial.begin(), test_case.initial.end());
+ auto in = wrap_input_decay<Iter, Sent>(test_case.input);
+
+ c.append_range(in);
+ validate(c);
+ if (!std::ranges::equal(c, test_case.expected))
+ return false;
+ }
+
+ return true;
};
{ // Empty container.
@@ -563,12 +607,26 @@ constexpr void test_sequence_assign_range(Validate validate) {
auto& input_long_range = FullContainer_Begin_LongRange<T>.input;
auto test = [&](auto& initial, auto& input) {
- Container c(initial.begin(), initial.end());
- auto in = wrap_input<Iter, Sent>(input);
-
- c.assign_range(in);
- validate(c);
- return std::ranges::equal(c, input);
+ {
+ Container c(initial.begin(), initial.end());
+ auto in = wrap_input<Iter, Sent>(input);
+
+ c.assign_range(in);
+ validate(c);
+ if (!std::ranges::equal(c, input))
+ return false;
+ }
+ {
+ Container c(initial.begin(), initial.end());
+ auto in = wrap_input_decay<Iter, Sent>(input);
+
+ c.assign_range(in);
+ validate(c);
+ if (!std::ranges::equal(c, input))
+ return false;
+ }
+
+ return true;
};
{ // Empty container.
``````````
</details>
https://github.com/llvm/llvm-project/pull/160022
More information about the libcxx-commits
mailing list