Skip to content

Commit 1116f84

Browse files
committed
[libc++] Fix {deque,vector}::append_range assuming too much about the types
1 parent 4154c18 commit 1116f84

File tree

9 files changed

+64
-2
lines changed

9 files changed

+64
-2
lines changed

libcxx/include/__vector/vector.h

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@
4242
#include <__memory/temp_value.h>
4343
#include <__memory/uninitialized_algorithms.h>
4444
#include <__ranges/access.h>
45+
#include <__ranges/as_rvalue_view.h>
4546
#include <__ranges/concepts.h>
4647
#include <__ranges/container_compatible_range.h>
4748
#include <__ranges/from_range.h>
@@ -489,7 +490,21 @@ class vector {
489490
#if _LIBCPP_STD_VER >= 23
490491
template <_ContainerCompatibleRange<_Tp> _Range>
491492
_LIBCPP_HIDE_FROM_ABI constexpr void append_range(_Range&& __range) {
492-
insert_range(end(), std::forward<_Range>(__range));
493+
if constexpr (ranges::forward_range<_Range> || ranges::sized_range<_Range>) {
494+
auto __size = ranges::distance(__range);
495+
if (__size < __cap_ - __end_) {
496+
__construct_at_end(ranges::begin(__range), ranges::end(__range), __size);
497+
} else {
498+
__split_buffer<value_type, allocator_type&> __buffer(__recommend(size() + __size), size(), __alloc_);
499+
__buffer.__construct_at_end_with_size(ranges::begin(__range), __size);
500+
__swap_out_circular_buffer(__buffer);
501+
}
502+
} else {
503+
vector __buffer(__alloc_);
504+
for (auto&& __val : __range)
505+
__buffer.emplace_back(std::forward<decltype(__val)>(__val));
506+
append_range(ranges::as_rvalue_view(__buffer));
507+
}
493508
}
494509
#endif
495510

libcxx/include/deque

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -807,7 +807,11 @@ public:
807807

808808
template <_ContainerCompatibleRange<_Tp> _Range>
809809
_LIBCPP_HIDE_FROM_ABI void append_range(_Range&& __range) {
810-
insert_range(end(), std::forward<_Range>(__range));
810+
if constexpr (ranges::forward_range<_Range> || ranges::sized_range<_Range>) {
811+
__append_with_size(ranges::begin(__range), ranges::distance(__range));
812+
} else {
813+
__append_with_sentinel(ranges::begin(__range), ranges::end(__range));
814+
}
811815
}
812816
# endif
813817

libcxx/test/std/containers/sequences/deque/deque.modifiers/append_range.pass.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ int main(int, char**) {
3131
});
3232
});
3333
test_sequence_append_range_move_only<std::deque>();
34+
test_sequence_append_range_only_move_construct<std::deque>();
3435

3536
test_append_range_exception_safety_throwing_copy<std::deque>();
3637
test_append_range_exception_safety_throwing_allocator<std::deque, int>();

libcxx/test/std/containers/sequences/deque/deque.modifiers/prepend_range.pass.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,8 @@ int main(int, char**) {
3131
});
3232
});
3333
test_sequence_prepend_range_move_only<std::deque>();
34+
// FIXME: This should work
35+
// test_sequence_prepend_range_only_move_construct<std::deque>();
3436

3537
test_prepend_range_exception_safety_throwing_copy<std::deque>();
3638
test_prepend_range_exception_safety_throwing_allocator<std::deque, int>();

libcxx/test/std/containers/sequences/forwardlist/forwardlist.modifiers/prepend_range.pass.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ TEST_CONSTEXPR_CXX26 bool test() {
3030
});
3131
});
3232
test_sequence_prepend_range_move_only<std::forward_list>();
33+
test_sequence_prepend_range_only_move_construct<std::forward_list>();
3334

3435
if (!TEST_IS_CONSTANT_EVALUATED) {
3536
test_prepend_range_exception_safety_throwing_copy<std::forward_list>();

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

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -643,6 +643,42 @@ constexpr void test_sequence_assign_range_move_only() {
643643
c.assign_range(in);
644644
}
645645

646+
struct InPlaceOnly {
647+
InPlaceOnly(const InPlaceOnly&) = delete;
648+
InPlaceOnly(InPlaceOnly&&) = delete;
649+
InPlaceOnly& operator=(const InPlaceOnly&) = delete;
650+
InPlaceOnly& operator=(InPlaceOnly&&) = delete;
651+
constexpr InPlaceOnly() {}
652+
};
653+
654+
struct MoveConstructOnly {
655+
MoveConstructOnly(const MoveConstructOnly&) = delete;
656+
MoveConstructOnly& operator=(const MoveConstructOnly&) = delete;
657+
MoveConstructOnly& operator=(MoveConstructOnly&&) = delete;
658+
constexpr MoveConstructOnly(MoveConstructOnly&&) {}
659+
constexpr MoveConstructOnly(const InPlaceOnly&) {}
660+
};
661+
662+
template <template <class...> class Container>
663+
constexpr void test_sequence_append_range_only_move_construct() {
664+
InPlaceOnly input[5];
665+
types::for_each(types::cpp20_input_iterator_list<InPlaceOnly*>{}, [&]<class Iter> {
666+
std::ranges::subrange in(Iter(input), sentinel_wrapper<Iter>(Iter(input + 5)));
667+
Container<MoveConstructOnly> c;
668+
c.append_range(in);
669+
});
670+
}
671+
672+
template <template <class...> class Container>
673+
constexpr void test_sequence_prepend_range_only_move_construct() {
674+
InPlaceOnly input[5];
675+
types::for_each(types::cpp20_input_iterator_list<InPlaceOnly*>{}, [&]<class Iter> {
676+
std::ranges::subrange in(Iter(input), sentinel_wrapper<Iter>(Iter(input + 5)));
677+
Container<MoveConstructOnly> c;
678+
c.prepend_range(in);
679+
});
680+
}
681+
646682
// Exception safety.
647683

648684
template <template <class...> class Container>

libcxx/test/std/containers/sequences/list/list.modifiers/append_range.pass.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ TEST_CONSTEXPR_CXX26 bool test() {
3131
});
3232
});
3333
test_sequence_append_range_move_only<std::list>();
34+
test_sequence_append_range_only_move_construct<std::list>();
3435

3536
if (!TEST_IS_CONSTANT_EVALUATED) {
3637
test_append_range_exception_safety_throwing_copy<std::list>();

libcxx/test/std/containers/sequences/list/list.modifiers/prepend_range.pass.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ TEST_CONSTEXPR_CXX26 bool test() {
3131
});
3232
});
3333
test_sequence_prepend_range_move_only<std::list>();
34+
test_sequence_prepend_range_only_move_construct<std::list>();
3435

3536
if (!TEST_IS_CONSTANT_EVALUATED) {
3637
test_prepend_range_exception_safety_throwing_copy<std::list>();

libcxx/test/std/containers/sequences/vector/vector.modifiers/append_range.pass.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ constexpr bool test() {
3333
});
3434
});
3535
test_sequence_append_range_move_only<std::vector>();
36+
test_sequence_append_range_only_move_construct<std::vector>();
3637

3738
{ // Vector may or may not need to reallocate because of the insertion -- make sure to test both cases.
3839
{ // Ensure reallocation happens.

0 commit comments

Comments
 (0)