Skip to content

Commit 78ae244

Browse files
[libc++] Fix insertion into deque from prvalue ranges
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.
1 parent 84a796d commit 78ae244

File tree

3 files changed

+128
-28
lines changed

3 files changed

+128
-28
lines changed

libcxx/include/deque

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,7 @@ template <class T, class Allocator, class Predicate>
185185
# include <__algorithm/copy_n.h>
186186
# include <__algorithm/equal.h>
187187
# include <__algorithm/fill_n.h>
188+
# include <__algorithm/iterator_operations.h>
188189
# include <__algorithm/lexicographical_compare.h>
189190
# include <__algorithm/lexicographical_compare_three_way.h>
190191
# include <__algorithm/max.h>
@@ -1916,6 +1917,12 @@ template <class _Tp, class _Allocator>
19161917
template <class _BiIter>
19171918
_LIBCPP_HIDE_FROM_ABI typename deque<_Tp, _Allocator>::iterator
19181919
deque<_Tp, _Allocator>::__insert_bidirectional(const_iterator __p, _BiIter __f, _BiIter __l, size_type __n) {
1920+
# if _LIBCPP_STD_VER >= 20
1921+
using _Ops = _IterOps<
1922+
conditional_t<__has_bidirectional_iterator_category<_BiIter>::value, _ClassicAlgPolicy, _RangeAlgPolicy>>;
1923+
# else
1924+
using _Ops = _IterOps<_ClassicAlgPolicy>;
1925+
# endif
19191926
size_type __pos = __p - begin();
19201927
size_type __to_end = size() - __pos;
19211928
allocator_type& __a = __alloc();
@@ -1928,7 +1935,7 @@ deque<_Tp, _Allocator>::__insert_bidirectional(const_iterator __p, _BiIter __f,
19281935
iterator __i = __old_begin;
19291936
_BiIter __m = __f;
19301937
if (__n > __pos) {
1931-
__m = __pos < __n / 2 ? std::prev(__l, __pos) : std::next(__f, __n - __pos);
1938+
__m = __pos < __n / 2 ? _Ops::prev(__l, __pos) : _Ops::next(__f, __n - __pos);
19321939
for (_BiIter __j = __m; __j != __f; --__start_, ++__size())
19331940
__alloc_traits::construct(__a, std::addressof(*--__i), *--__j);
19341941
__n = __pos;
@@ -1955,7 +1962,7 @@ deque<_Tp, _Allocator>::__insert_bidirectional(const_iterator __p, _BiIter __f,
19551962
_BiIter __m = __l;
19561963
size_type __de = size() - __pos;
19571964
if (__n > __de) {
1958-
__m = __de < __n / 2 ? std::next(__f, __de) : std::prev(__l, __n - __de);
1965+
__m = __de < __n / 2 ? _Ops::next(__f, __de) : _Ops::prev(__l, __n - __de);
19591966
for (_BiIter __j = __m; __j != __l; ++__i, (void)++__j, ++__size())
19601967
__alloc_traits::construct(__a, std::addressof(*__i), *__j);
19611968
__n = __de;

libcxx/test/std/containers/from_range_helpers.h

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,12 @@
1010
#define SUPPORT_FROM_RANGE_HELPERS_H
1111

1212
#include <array>
13+
#include <concepts>
1314
#include <cstddef>
1415
#include <iterator>
16+
#include <ranges>
1517
#include <type_traits>
18+
#include <utility>
1619
#include <vector>
1720

1821
#include "min_allocator.h"
@@ -50,6 +53,38 @@ constexpr auto wrap_input(std::vector<T>& input) {
5053
return std::ranges::subrange(std::move(b), std::move(e));
5154
}
5255

56+
struct DecayCopy {
57+
template <class T>
58+
requires std::convertible_to<T, std::decay_t<T>>
59+
static constexpr std::decay_t<T> operator()(T&& t) {
60+
return std::forward<T>(t);
61+
}
62+
};
63+
64+
template <class Iter, class Sent, std::ranges::input_range Range>
65+
constexpr auto wrap_input_decay(Range&& input) {
66+
auto b = Iter(std::ranges::begin(input));
67+
auto e = Sent(Iter(std::ranges::end(input)));
68+
if constexpr (std::is_reference_v<std::iter_reference_t<Iter>>)
69+
return std::ranges::subrange(std::move(b), std::move(e)) | std::views::transform(DecayCopy{});
70+
else
71+
return std::ranges::subrange(std::move(b), std::move(e));
72+
}
73+
74+
template <class Iter, class Sent, class T, std::size_t N>
75+
constexpr auto wrap_input_decay(std::array<T, N>& input) {
76+
auto b = Iter(input.data());
77+
auto e = Sent(Iter(input.data() + input.size()));
78+
return std::ranges::subrange(std::move(b), std::move(e)) | std::views::transform(DecayCopy{});
79+
}
80+
81+
template <class Iter, class Sent, class T>
82+
constexpr auto wrap_input_decay(std::vector<T>& input) {
83+
auto b = Iter(input.data());
84+
auto e = Sent(Iter(input.data() + input.size()));
85+
return std::ranges::subrange(std::move(b), std::move(e)) | std::views::transform(DecayCopy{});
86+
}
87+
5388
struct KeyValue {
5489
int key; // Only the key is considered for equality comparison.
5590
char value; // Allows distinguishing equivalent instances.

libcxx/test/std/containers/sequences/insert_range_sequence_containers.h

Lines changed: 84 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -402,14 +402,30 @@ constexpr void test_sequence_insert_range(Validate validate) {
402402
auto get_pos = [](auto& c, auto& test_case) { return std::ranges::next(c.begin(), static_cast<D>(test_case.index)); };
403403

404404
auto test = [&](auto& test_case) {
405-
Container c(test_case.initial.begin(), test_case.initial.end());
406-
auto in = wrap_input<Iter, Sent>(test_case.input);
407-
auto pos = get_pos(c, test_case);
408-
409-
auto result = c.insert_range(pos, in);
410-
assert(result == get_pos(c, test_case));
411-
validate(c);
412-
return std::ranges::equal(c, test_case.expected);
405+
{
406+
Container c(test_case.initial.begin(), test_case.initial.end());
407+
auto in = wrap_input<Iter, Sent>(test_case.input);
408+
auto pos = get_pos(c, test_case);
409+
410+
auto result = c.insert_range(pos, in);
411+
assert(result == get_pos(c, test_case));
412+
validate(c);
413+
if (!std::ranges::equal(c, test_case.expected))
414+
return false;
415+
}
416+
{
417+
Container c(test_case.initial.begin(), test_case.initial.end());
418+
auto in = wrap_input_decay<Iter, Sent>(test_case.input);
419+
auto pos = get_pos(c, test_case);
420+
421+
auto result = c.insert_range(pos, in);
422+
assert(result == get_pos(c, test_case));
423+
validate(c);
424+
if (!std::ranges::equal(c, test_case.expected))
425+
return false;
426+
}
427+
428+
return true;
413429
};
414430

415431
{ // Empty container.
@@ -469,12 +485,26 @@ constexpr void test_sequence_prepend_range(Validate validate) {
469485
using T = typename Container::value_type;
470486

471487
auto test = [&](auto& test_case) {
472-
Container c(test_case.initial.begin(), test_case.initial.end());
473-
auto in = wrap_input<Iter, Sent>(test_case.input);
474-
475-
c.prepend_range(in);
476-
validate(c);
477-
return std::ranges::equal(c, test_case.expected);
488+
{
489+
Container c(test_case.initial.begin(), test_case.initial.end());
490+
auto in = wrap_input<Iter, Sent>(test_case.input);
491+
492+
c.prepend_range(in);
493+
validate(c);
494+
if (!std::ranges::equal(c, test_case.expected))
495+
return false;
496+
}
497+
{
498+
Container c(test_case.initial.begin(), test_case.initial.end());
499+
auto in = wrap_input_decay<Iter, Sent>(test_case.input);
500+
501+
c.prepend_range(in);
502+
validate(c);
503+
if (!std::ranges::equal(c, test_case.expected))
504+
return false;
505+
}
506+
507+
return true;
478508
};
479509

480510
{ // Empty container.
@@ -512,12 +542,26 @@ constexpr void test_sequence_append_range(Validate validate) {
512542
using T = typename Container::value_type;
513543

514544
auto test = [&](auto& test_case) {
515-
Container c(test_case.initial.begin(), test_case.initial.end());
516-
auto in = wrap_input<Iter, Sent>(test_case.input);
517-
518-
c.append_range(in);
519-
validate(c);
520-
return std::ranges::equal(c, test_case.expected);
545+
{
546+
Container c(test_case.initial.begin(), test_case.initial.end());
547+
auto in = wrap_input<Iter, Sent>(test_case.input);
548+
549+
c.append_range(in);
550+
validate(c);
551+
if (!std::ranges::equal(c, test_case.expected))
552+
return false;
553+
}
554+
{
555+
Container c(test_case.initial.begin(), test_case.initial.end());
556+
auto in = wrap_input_decay<Iter, Sent>(test_case.input);
557+
558+
c.append_range(in);
559+
validate(c);
560+
if (!std::ranges::equal(c, test_case.expected))
561+
return false;
562+
}
563+
564+
return true;
521565
};
522566

523567
{ // Empty container.
@@ -563,12 +607,26 @@ constexpr void test_sequence_assign_range(Validate validate) {
563607
auto& input_long_range = FullContainer_Begin_LongRange<T>.input;
564608

565609
auto test = [&](auto& initial, auto& input) {
566-
Container c(initial.begin(), initial.end());
567-
auto in = wrap_input<Iter, Sent>(input);
568-
569-
c.assign_range(in);
570-
validate(c);
571-
return std::ranges::equal(c, input);
610+
{
611+
Container c(initial.begin(), initial.end());
612+
auto in = wrap_input<Iter, Sent>(input);
613+
614+
c.assign_range(in);
615+
validate(c);
616+
if (!std::ranges::equal(c, input))
617+
return false;
618+
}
619+
{
620+
Container c(initial.begin(), initial.end());
621+
auto in = wrap_input_decay<Iter, Sent>(input);
622+
623+
c.assign_range(in);
624+
validate(c);
625+
if (!std::ranges::equal(c, input))
626+
return false;
627+
}
628+
629+
return true;
572630
};
573631

574632
{ // Empty container.

0 commit comments

Comments
 (0)