Skip to content

Commit 7abcb4f

Browse files
[libc++] Fix insertion and input-only range handling for vector
Changes: - Carve out sized but input-only ranges for C++23. - Call `std::move` for related functions when the iterator is possibly input-only. - Avoid direct assignment in iterator-pair `insert` overload and `insert_range`, except when the assignment is move assignment.
1 parent edfa75d commit 7abcb4f

File tree

10 files changed

+181
-36
lines changed

10 files changed

+181
-36
lines changed

libcxx/include/__memory/uninitialized_algorithms.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -585,9 +585,9 @@ __uninitialized_allocator_copy_impl(_Alloc&, _In* __first1, _In* __last1, _Out*
585585
template <class _Alloc, class _Iter1, class _Sent1, class _Iter2>
586586
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 _Iter2
587587
__uninitialized_allocator_copy(_Alloc& __alloc, _Iter1 __first1, _Sent1 __last1, _Iter2 __first2) {
588-
auto __unwrapped_range = std::__unwrap_range(__first1, __last1);
588+
auto __unwrapped_range = std::__unwrap_range(std::move(__first1), std::move(__last1));
589589
auto __result = std::__uninitialized_allocator_copy_impl(
590-
__alloc, __unwrapped_range.first, __unwrapped_range.second, std::__unwrap_iter(__first2));
590+
__alloc, std::move(__unwrapped_range.first), std::move(__unwrapped_range.second), std::__unwrap_iter(__first2));
591591
return std::__rewrap_iter(__first2, __result);
592592
}
593593

libcxx/include/__vector/vector.h

Lines changed: 63 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
#include <__algorithm/min.h>
1616
#include <__algorithm/move.h>
1717
#include <__algorithm/move_backward.h>
18+
#include <__algorithm/ranges_copy_n.h>
1819
#include <__algorithm/rotate.h>
1920
#include <__assert>
2021
#include <__config>
@@ -23,6 +24,7 @@
2324
#include <__fwd/vector.h>
2425
#include <__iterator/advance.h>
2526
#include <__iterator/bounded_iter.h>
27+
#include <__iterator/concepts.h>
2628
#include <__iterator/distance.h>
2729
#include <__iterator/iterator_traits.h>
2830
#include <__iterator/move_iterator.h>
@@ -54,6 +56,7 @@
5456
#include <__type_traits/is_same.h>
5557
#include <__type_traits/is_trivially_relocatable.h>
5658
#include <__type_traits/type_identity.h>
59+
#include <__utility/declval.h>
5760
#include <__utility/exception_guard.h>
5861
#include <__utility/forward.h>
5962
#include <__utility/is_pointer_in_range.h>
@@ -570,7 +573,7 @@ class _LIBCPP_TEMPLATE_VIS vector {
570573

571574
if (__n > 0) {
572575
__vallocate(__n);
573-
__construct_at_end(__first, __last, __n);
576+
__construct_at_end(std::move(__first), std::move(__last), __n);
574577
}
575578

576579
__guard.__complete();
@@ -590,9 +593,31 @@ class _LIBCPP_TEMPLATE_VIS vector {
590593
template <class _Iterator, class _Sentinel>
591594
_LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI void __assign_with_sentinel(_Iterator __first, _Sentinel __last);
592595

593-
template <class _ForwardIterator, class _Sentinel>
596+
// The `_Iterator` in `*_with_size` functions can be input-only only if called from `*_range` (since C++23).
597+
// Otherwise, `_Iterator` is a forward iterator.
598+
599+
template <class _Iterator, class _Sentinel>
600+
_LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI void
601+
__assign_with_size(_Iterator __first, _Sentinel __last, difference_type __n);
602+
603+
template <class _Iterator,
604+
__enable_if_t<!is_same<decltype(*std::declval<_Iterator&>())&&, value_type&&>::value, int> = 0>
605+
_LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI void
606+
__insert_assign_n_unchecked(_Iterator __first, difference_type __n, pointer __position) {
607+
for (pointer __end_position = __position + __n; __position != __end_position; (void)++__position, ++__first) {
608+
__temp_value<value_type, _Allocator> __tmp(this->__alloc(), *__first);
609+
*__position = std::move(__tmp.get());
610+
}
611+
}
612+
613+
template <class _Iterator,
614+
__enable_if_t<is_same<decltype(*std::declval<_Iterator&>())&&, value_type&&>::value, int> = 0>
594615
_LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI void
595-
__assign_with_size(_ForwardIterator __first, _Sentinel __last, difference_type __n);
616+
__insert_assign_n_unchecked(_Iterator __first, difference_type __n, pointer __position) {
617+
for (pointer __end_position = __position + __n; __position != __end_position; (void)++__position, ++__first) {
618+
*__position = *__first;
619+
}
620+
}
596621

597622
template <class _InputIterator, class _Sentinel>
598623
_LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI iterator
@@ -916,7 +941,7 @@ template <class _InputIterator, class _Sentinel>
916941
_LIBCPP_CONSTEXPR_SINCE_CXX20 void
917942
vector<_Tp, _Allocator>::__construct_at_end(_InputIterator __first, _Sentinel __last, size_type __n) {
918943
_ConstructTransaction __tx(*this, __n);
919-
__tx.__pos_ = std::__uninitialized_allocator_copy(__alloc(), __first, __last, __tx.__pos_);
944+
__tx.__pos_ = std::__uninitialized_allocator_copy(__alloc(), std::move(__first), std::move(__last), __tx.__pos_);
920945
}
921946

922947
// Default constructs __n objects starting at __end_
@@ -1023,23 +1048,31 @@ vector<_Tp, _Allocator>::__assign_with_sentinel(_Iterator __first, _Sentinel __l
10231048
}
10241049

10251050
template <class _Tp, class _Allocator>
1026-
template <class _ForwardIterator, class _Sentinel>
1051+
template <class _Iterator, class _Sentinel>
10271052
_LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI void
1028-
vector<_Tp, _Allocator>::__assign_with_size(_ForwardIterator __first, _Sentinel __last, difference_type __n) {
1053+
vector<_Tp, _Allocator>::__assign_with_size(_Iterator __first, _Sentinel __last, difference_type __n) {
10291054
size_type __new_size = static_cast<size_type>(__n);
10301055
if (__new_size <= capacity()) {
10311056
if (__new_size > size()) {
1032-
_ForwardIterator __mid = std::next(__first, size());
1033-
std::copy(__first, __mid, this->__begin_);
1034-
__construct_at_end(__mid, __last, __new_size - size());
1057+
#if _LIBCPP_STD_VER >= 23
1058+
if constexpr (!forward_iterator<_Iterator>) {
1059+
auto __mid = ranges::copy_n(std::move(__first), size(), this->__begin_).in;
1060+
__construct_at_end(std::move(__mid), std::move(__last), __new_size - size());
1061+
} else
1062+
#endif
1063+
{
1064+
_Iterator __mid = std::next(__first, size());
1065+
std::copy(__first, __mid, this->__begin_);
1066+
__construct_at_end(__mid, __last, __new_size - size());
1067+
}
10351068
} else {
1036-
pointer __m = std::__copy(__first, __last, this->__begin_).second;
1069+
pointer __m = std::__copy(std::move(__first), __last, this->__begin_).second;
10371070
this->__destruct_at_end(__m);
10381071
}
10391072
} else {
10401073
__vdeallocate();
10411074
__vallocate(__recommend(__new_size));
1042-
__construct_at_end(__first, __last, __new_size);
1075+
__construct_at_end(std::move(__first), std::move(__last), __new_size);
10431076
}
10441077
}
10451078

@@ -1297,29 +1330,34 @@ template <class _Iterator, class _Sentinel>
12971330
_LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI typename vector<_Tp, _Allocator>::iterator
12981331
vector<_Tp, _Allocator>::__insert_with_size(
12991332
const_iterator __position, _Iterator __first, _Sentinel __last, difference_type __n) {
1300-
auto __insertion_size = __n;
1301-
pointer __p = this->__begin_ + (__position - begin());
1333+
pointer __p = this->__begin_ + (__position - begin());
13021334
if (__n > 0) {
13031335
if (__n <= this->__end_cap() - this->__end_) {
1304-
size_type __old_n = __n;
13051336
pointer __old_last = this->__end_;
1306-
_Iterator __m = std::next(__first, __n);
13071337
difference_type __dx = this->__end_ - __p;
13081338
if (__n > __dx) {
1309-
__m = __first;
1310-
difference_type __diff = this->__end_ - __p;
1311-
std::advance(__m, __diff);
1312-
__construct_at_end(__m, __last, __n - __diff);
1313-
__n = __dx;
1314-
}
1315-
if (__n > 0) {
1316-
__move_range(__p, __old_last, __p + __old_n);
1317-
std::copy(__first, __m, __p);
1339+
#if _LIBCPP_STD_VER >= 23
1340+
if constexpr (!forward_iterator<_Iterator>) {
1341+
__construct_at_end(std::move(__first), std::move(__last), __n);
1342+
std::rotate(__p, __old_last, this->__end_);
1343+
} else
1344+
#endif
1345+
{
1346+
_Iterator __m = std::next(__first, __dx);
1347+
__construct_at_end(__m, __last, __n - __dx);
1348+
if (__dx > 0) {
1349+
__move_range(__p, __old_last, __p + __n);
1350+
__insert_assign_n_unchecked(__first, __dx, __p);
1351+
}
1352+
}
1353+
} else {
1354+
__move_range(__p, __old_last, __p + __n);
1355+
__insert_assign_n_unchecked(std::move(__first), __n, __p);
13181356
}
13191357
} else {
13201358
allocator_type& __a = this->__alloc();
13211359
__split_buffer<value_type, allocator_type&> __v(__recommend(size() + __n), __p - this->__begin_, __a);
1322-
__v.__construct_at_end_with_size(__first, __insertion_size);
1360+
__v.__construct_at_end_with_size(std::move(__first), __n);
13231361
__p = __swap_out_circular_buffer(__v, __p);
13241362
}
13251363
}

libcxx/include/__vector/vector_bool.h

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -415,9 +415,12 @@ class _LIBCPP_TEMPLATE_VIS vector<bool, _Allocator> {
415415
template <class _Iterator, class _Sentinel>
416416
_LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI void __assign_with_sentinel(_Iterator __first, _Sentinel __last);
417417

418-
template <class _ForwardIterator, class _Sentinel>
418+
// The `_Iterator` in `*_with_size` functions can be input-only only if called from `*_range` (since C++23).
419+
// Otherwise, `_Iterator` is a forward iterator.
420+
421+
template <class _Iterator, class _Sentinel>
419422
_LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI void
420-
__assign_with_size(_ForwardIterator __first, _Sentinel __last, difference_type __ns);
423+
__assign_with_size(_Iterator __first, _Sentinel __last, difference_type __ns);
421424

422425
template <class _InputIterator, class _Sentinel>
423426
_LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI iterator
@@ -574,7 +577,7 @@ vector<bool, _Allocator>::__construct_at_end(_InputIterator __first, _Sentinel _
574577
else
575578
this->__begin_[(this->__size_ - 1) / __bits_per_word] = __storage_type(0);
576579
}
577-
std::__copy(__first, __last, __make_iter(__old_size));
580+
std::__copy(std::move(__first), std::move(__last), __make_iter(__old_size));
578581
}
579582

580583
template <class _Allocator>
@@ -824,9 +827,9 @@ _LIBCPP_CONSTEXPR_SINCE_CXX20 void vector<bool, _Allocator>::assign(_ForwardIter
824827
}
825828

826829
template <class _Allocator>
827-
template <class _ForwardIterator, class _Sentinel>
830+
template <class _Iterator, class _Sentinel>
828831
_LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI void
829-
vector<bool, _Allocator>::__assign_with_size(_ForwardIterator __first, _Sentinel __last, difference_type __ns) {
832+
vector<bool, _Allocator>::__assign_with_size(_Iterator __first, _Sentinel __last, difference_type __ns) {
830833
_LIBCPP_ASSERT_VALID_INPUT_RANGE(__ns >= 0, "invalid range specified");
831834

832835
clear();
@@ -837,7 +840,7 @@ vector<bool, _Allocator>::__assign_with_size(_ForwardIterator __first, _Sentinel
837840
__vdeallocate();
838841
__vallocate(__n);
839842
}
840-
__construct_at_end(__first, __last, __n);
843+
__construct_at_end(std::move(__first), std::move(__last), __n);
841844
}
842845
}
843846

@@ -981,10 +984,10 @@ vector<bool, _Allocator>::insert(const_iterator __position, _ForwardIterator __f
981984
}
982985

983986
template <class _Allocator>
984-
template <class _ForwardIterator, class _Sentinel>
987+
template <class _Iterator, class _Sentinel>
985988
_LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI typename vector<bool, _Allocator>::iterator
986989
vector<bool, _Allocator>::__insert_with_size(
987-
const_iterator __position, _ForwardIterator __first, _Sentinel __last, difference_type __n_signed) {
990+
const_iterator __position, _Iterator __first, _Sentinel __last, difference_type __n_signed) {
988991
_LIBCPP_ASSERT_VALID_INPUT_RANGE(__n_signed >= 0, "invalid range specified");
989992
const size_type __n = static_cast<size_type>(__n_signed);
990993
iterator __r;
@@ -1002,7 +1005,7 @@ vector<bool, _Allocator>::__insert_with_size(
10021005
std::copy_backward(__position, cend(), __v.end());
10031006
swap(__v);
10041007
}
1005-
std::__copy(__first, __last, __r);
1008+
std::__copy(std::move(__first), std::move(__last), __r);
10061009
return __r;
10071010
}
10081011

libcxx/test/std/containers/sequences/vector.bool/assign_range.pass.cpp

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@
1212
// template<container-compatible-range<bool> R>
1313
// constexpr void assign_range(R&& rg); // C++23
1414

15+
#include <cassert>
16+
#include <sstream>
1517
#include <vector>
1618

1719
#include "../insert_range_sequence_containers.h"
@@ -54,12 +56,22 @@ constexpr bool test() {
5456
return true;
5557
}
5658

59+
void test_sized_input_only_range() {
60+
std::istringstream is{"1 1 0 1"};
61+
auto vals = std::views::istream<bool>(is);
62+
std::vector<bool> v;
63+
v.assign_range(std::views::counted(vals.begin(), 3));
64+
assert(v == (std::vector{true, true, false}));
65+
}
66+
5767
int main(int, char**) {
5868
test();
5969
static_assert(test());
6070

6171
// Note: `test_assign_range_exception_safety_throwing_copy` doesn't apply because copying booleans cannot throw.
6272
test_assign_range_exception_safety_throwing_allocator<std::vector, bool>();
6373

74+
test_sized_input_only_range();
75+
6476
return 0;
6577
}

libcxx/test/std/containers/sequences/vector.bool/construct_from_range.pass.cpp

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@
88

99
// UNSUPPORTED: c++03, c++11, c++14, c++17, c++20
1010

11+
#include <cassert>
12+
#include <sstream>
1113
#include <vector>
1214

1315
#include "../from_range_sequence_containers.h"
@@ -27,6 +29,13 @@ constexpr bool test() {
2729
return true;
2830
}
2931

32+
void test_sized_input_only_range() {
33+
std::istringstream is{"1 1 0 1"};
34+
auto vals = std::views::istream<bool>(is);
35+
std::vector v(std::from_range, std::views::counted(vals.begin(), 3));
36+
assert(v == (std::vector{true, true, false}));
37+
}
38+
3039
int main(int, char**) {
3140
test();
3241
static_assert(test());
@@ -36,5 +45,7 @@ int main(int, char**) {
3645
// Note: test_exception_safety_throwing_copy doesn't apply because copying a boolean cannot throw.
3746
test_exception_safety_throwing_allocator<std::vector, bool>();
3847

48+
test_sized_input_only_range();
49+
3950
return 0;
4051
}

libcxx/test/std/containers/sequences/vector.bool/insert_range.pass.cpp

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@
1212
// template<container-compatible-range<bool> R>
1313
// constexpr iterator insert_range(const_iterator position, R&& rg); // C++23
1414

15+
#include <cassert>
16+
#include <sstream>
1517
#include <vector>
1618

1719
#include "../insert_range_sequence_containers.h"
@@ -61,12 +63,22 @@ constexpr bool test() {
6163
return true;
6264
}
6365

66+
void test_sized_input_only_range() {
67+
std::istringstream is{"1 1 0 1"};
68+
auto vals = std::views::istream<bool>(is);
69+
std::vector<bool> v;
70+
v.insert_range(v.end(), std::views::counted(vals.begin(), 3));
71+
assert(v == (std::vector{true, true, false}));
72+
}
73+
6474
int main(int, char**) {
6575
test();
6676
static_assert(test());
6777

6878
// Note: `test_insert_range_exception_safety_throwing_copy` doesn't apply because copying booleans cannot throw.
6979
test_insert_range_exception_safety_throwing_allocator<std::vector, bool>();
7080

81+
test_sized_input_only_range();
82+
7183
return 0;
7284
}

libcxx/test/std/containers/sequences/vector/vector.cons/construct_from_range.pass.cpp

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@
1111
// template<container-compatible-range<T> R>
1212
// vector(from_range_t, R&& rg, const Allocator& = Allocator()); // C++23
1313

14+
#include <cassert>
15+
#include <sstream>
1416
#include <vector>
1517

1618
#include "../../from_range_sequence_containers.h"
@@ -29,6 +31,13 @@ constexpr bool test() {
2931
return true;
3032
}
3133

34+
void test_sized_input_only_range() {
35+
std::istringstream is{"1 2 3 4"};
36+
auto vals = std::views::istream<int>(is);
37+
std::vector v(std::from_range, std::views::counted(vals.begin(), 3));
38+
assert(v == (std::vector{1, 2, 3}));
39+
}
40+
3241
int main(int, char**) {
3342
static_assert(test_constraints<std::vector, int, double>());
3443
test();
@@ -38,5 +47,7 @@ int main(int, char**) {
3847
test_exception_safety_throwing_copy<std::vector>();
3948
test_exception_safety_throwing_allocator<std::vector, int>();
4049

50+
test_sized_input_only_range();
51+
4152
return 0;
4253
}

0 commit comments

Comments
 (0)