Skip to content

Commit 733a98d

Browse files
frederick-vs-jafrederick-vs-ja
authored
[libc++] Fix input-only range handling for vector (#116157)
Changes: - Carve out sized but input-only ranges for C++23. - Call `std::move` for related functions when the iterator is possibly input-only. Fixes #115727
1 parent 80097a1 commit 733a98d

File tree

9 files changed

+194
-34
lines changed

9 files changed

+194
-34
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: 46 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,13 @@
1010
#define _LIBCPP___VECTOR_VECTOR_H
1111

1212
#include <__algorithm/copy.h>
13+
#include <__algorithm/copy_n.h>
1314
#include <__algorithm/fill_n.h>
1415
#include <__algorithm/max.h>
1516
#include <__algorithm/min.h>
1617
#include <__algorithm/move.h>
1718
#include <__algorithm/move_backward.h>
19+
#include <__algorithm/ranges_copy_n.h>
1820
#include <__algorithm/rotate.h>
1921
#include <__assert>
2022
#include <__config>
@@ -23,6 +25,7 @@
2325
#include <__fwd/vector.h>
2426
#include <__iterator/advance.h>
2527
#include <__iterator/bounded_iter.h>
28+
#include <__iterator/concepts.h>
2629
#include <__iterator/distance.h>
2730
#include <__iterator/iterator_traits.h>
2831
#include <__iterator/move_iterator.h>
@@ -575,7 +578,7 @@ class _LIBCPP_TEMPLATE_VIS vector {
575578

576579
if (__n > 0) {
577580
__vallocate(__n);
578-
__construct_at_end(__first, __last, __n);
581+
__construct_at_end(std::move(__first), std::move(__last), __n);
579582
}
580583

581584
__guard.__complete();
@@ -595,9 +598,12 @@ class _LIBCPP_TEMPLATE_VIS vector {
595598
template <class _Iterator, class _Sentinel>
596599
_LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI void __assign_with_sentinel(_Iterator __first, _Sentinel __last);
597600

598-
template <class _ForwardIterator, class _Sentinel>
601+
// The `_Iterator` in `*_with_size` functions can be input-only only if called from `*_range` (since C++23).
602+
// Otherwise, `_Iterator` is a forward iterator.
603+
604+
template <class _Iterator, class _Sentinel>
599605
_LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI void
600-
__assign_with_size(_ForwardIterator __first, _Sentinel __last, difference_type __n);
606+
__assign_with_size(_Iterator __first, _Sentinel __last, difference_type __n);
601607

602608
template <class _InputIterator, class _Sentinel>
603609
_LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI iterator
@@ -918,7 +924,7 @@ template <class _InputIterator, class _Sentinel>
918924
_LIBCPP_CONSTEXPR_SINCE_CXX20 void
919925
vector<_Tp, _Allocator>::__construct_at_end(_InputIterator __first, _Sentinel __last, size_type __n) {
920926
_ConstructTransaction __tx(*this, __n);
921-
__tx.__pos_ = std::__uninitialized_allocator_copy(this->__alloc_, __first, __last, __tx.__pos_);
927+
__tx.__pos_ = std::__uninitialized_allocator_copy(this->__alloc_, std::move(__first), std::move(__last), __tx.__pos_);
922928
}
923929

924930
// Default constructs __n objects starting at __end_
@@ -1027,23 +1033,28 @@ vector<_Tp, _Allocator>::__assign_with_sentinel(_Iterator __first, _Sentinel __l
10271033
}
10281034

10291035
template <class _Tp, class _Allocator>
1030-
template <class _ForwardIterator, class _Sentinel>
1036+
template <class _Iterator, class _Sentinel>
10311037
_LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI void
1032-
vector<_Tp, _Allocator>::__assign_with_size(_ForwardIterator __first, _Sentinel __last, difference_type __n) {
1038+
vector<_Tp, _Allocator>::__assign_with_size(_Iterator __first, _Sentinel __last, difference_type __n) {
10331039
size_type __new_size = static_cast<size_type>(__n);
10341040
if (__new_size <= capacity()) {
10351041
if (__new_size > size()) {
1036-
_ForwardIterator __mid = std::next(__first, size());
1042+
#if _LIBCPP_STD_VER >= 23
1043+
auto __mid = ranges::copy_n(std::move(__first), size(), this->__begin_).in;
1044+
__construct_at_end(std::move(__mid), std::move(__last), __new_size - size());
1045+
#else
1046+
_Iterator __mid = std::next(__first, size());
10371047
std::copy(__first, __mid, this->__begin_);
10381048
__construct_at_end(__mid, __last, __new_size - size());
1049+
#endif
10391050
} else {
1040-
pointer __m = std::__copy(__first, __last, this->__begin_).second;
1051+
pointer __m = std::__copy(std::move(__first), __last, this->__begin_).second;
10411052
this->__destruct_at_end(__m);
10421053
}
10431054
} else {
10441055
__vdeallocate();
10451056
__vallocate(__recommend(__new_size));
1046-
__construct_at_end(__first, __last, __new_size);
1057+
__construct_at_end(std::move(__first), std::move(__last), __new_size);
10471058
}
10481059
}
10491060

@@ -1293,28 +1304,40 @@ template <class _Iterator, class _Sentinel>
12931304
_LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI typename vector<_Tp, _Allocator>::iterator
12941305
vector<_Tp, _Allocator>::__insert_with_size(
12951306
const_iterator __position, _Iterator __first, _Sentinel __last, difference_type __n) {
1296-
auto __insertion_size = __n;
1297-
pointer __p = this->__begin_ + (__position - begin());
1307+
pointer __p = this->__begin_ + (__position - begin());
12981308
if (__n > 0) {
12991309
if (__n <= this->__cap_ - this->__end_) {
1300-
size_type __old_n = __n;
13011310
pointer __old_last = this->__end_;
1302-
_Iterator __m = std::next(__first, __n);
13031311
difference_type __dx = this->__end_ - __p;
13041312
if (__n > __dx) {
1305-
__m = __first;
1306-
difference_type __diff = this->__end_ - __p;
1307-
std::advance(__m, __diff);
1308-
__construct_at_end(__m, __last, __n - __diff);
1309-
__n = __dx;
1310-
}
1311-
if (__n > 0) {
1312-
__move_range(__p, __old_last, __p + __old_n);
1313-
std::copy(__first, __m, __p);
1313+
#if _LIBCPP_STD_VER >= 23
1314+
if constexpr (!forward_iterator<_Iterator>) {
1315+
__construct_at_end(std::move(__first), std::move(__last), __n);
1316+
std::rotate(__p, __old_last, this->__end_);
1317+
} else
1318+
#endif
1319+
{
1320+
_Iterator __m = std::next(__first, __dx);
1321+
__construct_at_end(__m, __last, __n - __dx);
1322+
if (__dx > 0) {
1323+
__move_range(__p, __old_last, __p + __n);
1324+
std::copy(__first, __m, __p);
1325+
}
1326+
}
1327+
} else {
1328+
__move_range(__p, __old_last, __p + __n);
1329+
#if _LIBCPP_STD_VER >= 23
1330+
if constexpr (!forward_iterator<_Iterator>) {
1331+
ranges::copy_n(std::move(__first), __n, __p);
1332+
} else
1333+
#endif
1334+
{
1335+
std::copy_n(__first, __n, __p);
1336+
}
13141337
}
13151338
} else {
13161339
__split_buffer<value_type, allocator_type&> __v(__recommend(size() + __n), __p - this->__begin_, this->__alloc_);
1317-
__v.__construct_at_end_with_size(__first, __insertion_size);
1340+
__v.__construct_at_end_with_size(std::move(__first), __n);
13181341
__p = __swap_out_circular_buffer(__v, __p);
13191342
}
13201343
}

‎libcxx/include/__vector/vector_bool.h‎

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

423-
template <class _ForwardIterator, class _Sentinel>
423+
// The `_Iterator` in `*_with_size` functions can be input-only only if called from `*_range` (since C++23).
424+
// Otherwise, `_Iterator` is a forward iterator.
425+
426+
template <class _Iterator, class _Sentinel>
424427
_LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI void
425-
__assign_with_size(_ForwardIterator __first, _Sentinel __last, difference_type __ns);
428+
__assign_with_size(_Iterator __first, _Sentinel __last, difference_type __ns);
426429

427430
template <class _InputIterator, class _Sentinel>
428431
_LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI iterator
@@ -578,7 +581,7 @@ vector<bool, _Allocator>::__construct_at_end(_InputIterator __first, _Sentinel _
578581
else
579582
this->__begin_[(this->__size_ - 1) / __bits_per_word] = __storage_type(0);
580583
}
581-
std::__copy(__first, __last, __make_iter(__old_size));
584+
std::__copy(std::move(__first), std::move(__last), __make_iter(__old_size));
582585
}
583586

584587
template <class _Allocator>
@@ -828,9 +831,9 @@ _LIBCPP_CONSTEXPR_SINCE_CXX20 void vector<bool, _Allocator>::assign(_ForwardIter
828831
}
829832

830833
template <class _Allocator>
831-
template <class _ForwardIterator, class _Sentinel>
834+
template <class _Iterator, class _Sentinel>
832835
_LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI void
833-
vector<bool, _Allocator>::__assign_with_size(_ForwardIterator __first, _Sentinel __last, difference_type __ns) {
836+
vector<bool, _Allocator>::__assign_with_size(_Iterator __first, _Sentinel __last, difference_type __ns) {
834837
_LIBCPP_ASSERT_VALID_INPUT_RANGE(__ns >= 0, "invalid range specified");
835838

836839
clear();
@@ -841,7 +844,7 @@ vector<bool, _Allocator>::__assign_with_size(_ForwardIterator __first, _Sentinel
841844
__vdeallocate();
842845
__vallocate(__n);
843846
}
844-
__construct_at_end(__first, __last, __n);
847+
__construct_at_end(std::move(__first), std::move(__last), __n);
845848
}
846849
}
847850

@@ -988,10 +991,10 @@ vector<bool, _Allocator>::insert(const_iterator __position, _ForwardIterator __f
988991
}
989992

990993
template <class _Allocator>
991-
template <class _ForwardIterator, class _Sentinel>
994+
template <class _Iterator, class _Sentinel>
992995
_LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI typename vector<bool, _Allocator>::iterator
993996
vector<bool, _Allocator>::__insert_with_size(
994-
const_iterator __position, _ForwardIterator __first, _Sentinel __last, difference_type __n_signed) {
997+
const_iterator __position, _Iterator __first, _Sentinel __last, difference_type __n_signed) {
995998
_LIBCPP_ASSERT_VALID_INPUT_RANGE(__n_signed >= 0, "invalid range specified");
996999
const size_type __n = static_cast<size_type>(__n_signed);
9971000
iterator __r;
@@ -1009,7 +1012,7 @@ vector<bool, _Allocator>::__insert_with_size(
10091012
std::copy_backward(__position, cend(), __v.end());
10101013
swap(__v);
10111014
}
1012-
std::__copy(__first, __last, __r);
1015+
std::__copy(std::move(__first), std::move(__last), __r);
10131016
return __r;
10141017
}
10151018

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

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

15+
#include <sstream>
1516
#include <vector>
1617

1718
#include "../insert_range_sequence_containers.h"
@@ -49,17 +50,39 @@ constexpr bool test() {
4950
v.assign_range(in);
5051
assert(std::ranges::equal(v, in));
5152
}
53+
54+
{ // Ensure input-only sized ranges are accepted.
55+
using input_iter = cpp20_input_iterator<const bool*>;
56+
const bool in[]{true, true, false, true};
57+
std::vector<bool> v;
58+
v.assign_range(std::views::counted(input_iter{std::ranges::begin(in)}, std::ranges::ssize(in)));
59+
assert(std::ranges::equal(v, std::vector<bool>{true, true, false, true}));
60+
}
5261
}
5362

5463
return true;
5564
}
5665

66+
#ifndef TEST_HAS_NO_LOCALIZATION
67+
void test_counted_istream_view() {
68+
std::istringstream is{"1 1 0 1"};
69+
auto vals = std::views::istream<bool>(is);
70+
std::vector<bool> v;
71+
v.assign_range(std::views::counted(vals.begin(), 3));
72+
assert(v == (std::vector{true, true, false}));
73+
}
74+
#endif
75+
5776
int main(int, char**) {
5877
test();
5978
static_assert(test());
6079

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

83+
#ifndef TEST_HAS_NO_LOCALIZATION
84+
test_counted_istream_view();
85+
#endif
86+
6487
return 0;
6588
}

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

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

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

11+
#include <sstream>
1112
#include <vector>
1213

1314
#include "../from_range_sequence_containers.h"
@@ -24,9 +25,25 @@ constexpr bool test() {
2425
});
2526
});
2627

28+
{ // Ensure input-only sized ranges are accepted.
29+
using input_iter = cpp20_input_iterator<const bool*>;
30+
const bool in[]{true, true, false, true};
31+
std::vector v(std::from_range, std::views::counted(input_iter{std::ranges::begin(in)}, std::ranges::ssize(in)));
32+
assert(std::ranges::equal(v, std::vector<bool>{true, true, false, true}));
33+
}
34+
2735
return true;
2836
}
2937

38+
#ifndef TEST_HAS_NO_LOCALIZATION
39+
void test_counted_istream_view() {
40+
std::istringstream is{"1 1 0 1"};
41+
auto vals = std::views::istream<bool>(is);
42+
std::vector v(std::from_range, std::views::counted(vals.begin(), 3));
43+
assert(v == (std::vector{true, true, false}));
44+
}
45+
#endif
46+
3047
int main(int, char**) {
3148
test();
3249
static_assert(test());
@@ -36,5 +53,9 @@ int main(int, char**) {
3653
// Note: test_exception_safety_throwing_copy doesn't apply because copying a boolean cannot throw.
3754
test_exception_safety_throwing_allocator<std::vector, bool>();
3855

56+
#ifndef TEST_HAS_NO_LOCALIZATION
57+
test_counted_istream_view();
58+
#endif
59+
3960
return 0;
4061
}

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

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

15+
#include <sstream>
1516
#include <vector>
1617

1718
#include "../insert_range_sequence_containers.h"
@@ -56,17 +57,39 @@ constexpr bool test() {
5657
v.insert_range(v.end(), in);
5758
assert(std::ranges::equal(v, std::vector<bool>{0, 0, 0, 1, 1, 0, 0, 0, 1, 1, 1, 1, 0, 0, 1, 1, 1, 1}));
5859
}
60+
61+
{ // Ensure input-only sized ranges are accepted.
62+
using input_iter = cpp20_input_iterator<const bool*>;
63+
const bool in[]{true, true, false, true};
64+
std::vector<bool> v{true, false};
65+
v.insert_range(v.begin(), std::views::counted(input_iter{std::ranges::begin(in)}, std::ranges::ssize(in)));
66+
assert(std::ranges::equal(v, std::vector<bool>{true, true, false, true, true, false}));
67+
}
5968
}
6069

6170
return true;
6271
}
6372

73+
#ifndef TEST_HAS_NO_LOCALIZATION
74+
void test_counted_istream_view() {
75+
std::istringstream is{"1 1 0 1"};
76+
auto vals = std::views::istream<bool>(is);
77+
std::vector<bool> v;
78+
v.insert_range(v.end(), std::views::counted(vals.begin(), 3));
79+
assert(v == (std::vector{true, true, false}));
80+
}
81+
#endif
82+
6483
int main(int, char**) {
6584
test();
6685
static_assert(test());
6786

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

90+
#ifndef TEST_HAS_NO_LOCALIZATION
91+
test_counted_istream_view();
92+
#endif
93+
7194
return 0;
7295
}

0 commit comments

Comments
 (0)