Skip to content

[libc++] Fix input-only range handling for vector #116157

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jan 21, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions libcxx/include/__memory/uninitialized_algorithms.h
Original file line number Diff line number Diff line change
Expand Up @@ -585,9 +585,9 @@ __uninitialized_allocator_copy_impl(_Alloc&, _In* __first1, _In* __last1, _Out*
template <class _Alloc, class _Iter1, class _Sent1, class _Iter2>
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 _Iter2
__uninitialized_allocator_copy(_Alloc& __alloc, _Iter1 __first1, _Sent1 __last1, _Iter2 __first2) {
auto __unwrapped_range = std::__unwrap_range(__first1, __last1);
auto __unwrapped_range = std::__unwrap_range(std::move(__first1), std::move(__last1));
auto __result = std::__uninitialized_allocator_copy_impl(
__alloc, __unwrapped_range.first, __unwrapped_range.second, std::__unwrap_iter(__first2));
__alloc, std::move(__unwrapped_range.first), std::move(__unwrapped_range.second), std::__unwrap_iter(__first2));
return std::__rewrap_iter(__first2, __result);
}

Expand Down
69 changes: 46 additions & 23 deletions libcxx/include/__vector/vector.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,13 @@
#define _LIBCPP___VECTOR_VECTOR_H

#include <__algorithm/copy.h>
#include <__algorithm/copy_n.h>
#include <__algorithm/fill_n.h>
#include <__algorithm/max.h>
#include <__algorithm/min.h>
#include <__algorithm/move.h>
#include <__algorithm/move_backward.h>
#include <__algorithm/ranges_copy_n.h>
#include <__algorithm/rotate.h>
#include <__assert>
#include <__config>
Expand All @@ -23,6 +25,7 @@
#include <__fwd/vector.h>
#include <__iterator/advance.h>
#include <__iterator/bounded_iter.h>
#include <__iterator/concepts.h>
#include <__iterator/distance.h>
#include <__iterator/iterator_traits.h>
#include <__iterator/move_iterator.h>
Expand Down Expand Up @@ -575,7 +578,7 @@ class _LIBCPP_TEMPLATE_VIS vector {

if (__n > 0) {
__vallocate(__n);
__construct_at_end(__first, __last, __n);
__construct_at_end(std::move(__first), std::move(__last), __n);
}

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

template <class _ForwardIterator, class _Sentinel>
// The `_Iterator` in `*_with_size` functions can be input-only only if called from `*_range` (since C++23).
// Otherwise, `_Iterator` is a forward iterator.

template <class _Iterator, class _Sentinel>
_LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI void
__assign_with_size(_ForwardIterator __first, _Sentinel __last, difference_type __n);
__assign_with_size(_Iterator __first, _Sentinel __last, difference_type __n);

template <class _InputIterator, class _Sentinel>
_LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI iterator
Expand Down Expand Up @@ -918,7 +924,7 @@ template <class _InputIterator, class _Sentinel>
_LIBCPP_CONSTEXPR_SINCE_CXX20 void
vector<_Tp, _Allocator>::__construct_at_end(_InputIterator __first, _Sentinel __last, size_type __n) {
_ConstructTransaction __tx(*this, __n);
__tx.__pos_ = std::__uninitialized_allocator_copy(this->__alloc_, __first, __last, __tx.__pos_);
__tx.__pos_ = std::__uninitialized_allocator_copy(this->__alloc_, std::move(__first), std::move(__last), __tx.__pos_);
}

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

template <class _Tp, class _Allocator>
template <class _ForwardIterator, class _Sentinel>
template <class _Iterator, class _Sentinel>
_LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI void
vector<_Tp, _Allocator>::__assign_with_size(_ForwardIterator __first, _Sentinel __last, difference_type __n) {
vector<_Tp, _Allocator>::__assign_with_size(_Iterator __first, _Sentinel __last, difference_type __n) {
size_type __new_size = static_cast<size_type>(__n);
if (__new_size <= capacity()) {
if (__new_size > size()) {
_ForwardIterator __mid = std::next(__first, size());
#if _LIBCPP_STD_VER >= 23
auto __mid = ranges::copy_n(std::move(__first), size(), this->__begin_).in;
__construct_at_end(std::move(__mid), std::move(__last), __new_size - size());
#else
_Iterator __mid = std::next(__first, size());
std::copy(__first, __mid, this->__begin_);
__construct_at_end(__mid, __last, __new_size - size());
#endif
} else {
pointer __m = std::__copy(__first, __last, this->__begin_).second;
pointer __m = std::__copy(std::move(__first), __last, this->__begin_).second;
this->__destruct_at_end(__m);
}
} else {
__vdeallocate();
__vallocate(__recommend(__new_size));
__construct_at_end(__first, __last, __new_size);
__construct_at_end(std::move(__first), std::move(__last), __new_size);
}
}

Expand Down Expand Up @@ -1293,28 +1304,40 @@ template <class _Iterator, class _Sentinel>
_LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI typename vector<_Tp, _Allocator>::iterator
vector<_Tp, _Allocator>::__insert_with_size(
const_iterator __position, _Iterator __first, _Sentinel __last, difference_type __n) {
auto __insertion_size = __n;
pointer __p = this->__begin_ + (__position - begin());
pointer __p = this->__begin_ + (__position - begin());
if (__n > 0) {
if (__n <= this->__cap_ - this->__end_) {
size_type __old_n = __n;
pointer __old_last = this->__end_;
_Iterator __m = std::next(__first, __n);
difference_type __dx = this->__end_ - __p;
if (__n > __dx) {
__m = __first;
difference_type __diff = this->__end_ - __p;
std::advance(__m, __diff);
__construct_at_end(__m, __last, __n - __diff);
__n = __dx;
}
if (__n > 0) {
__move_range(__p, __old_last, __p + __old_n);
std::copy(__first, __m, __p);
#if _LIBCPP_STD_VER >= 23
if constexpr (!forward_iterator<_Iterator>) {
__construct_at_end(std::move(__first), std::move(__last), __n);
std::rotate(__p, __old_last, this->__end_);
} else
#endif
{
_Iterator __m = std::next(__first, __dx);
__construct_at_end(__m, __last, __n - __dx);
if (__dx > 0) {
__move_range(__p, __old_last, __p + __n);
std::copy(__first, __m, __p);
}
}
} else {
__move_range(__p, __old_last, __p + __n);
#if _LIBCPP_STD_VER >= 23
if constexpr (!forward_iterator<_Iterator>) {
ranges::copy_n(std::move(__first), __n, __p);
} else
#endif
{
std::copy_n(__first, __n, __p);
}
}
} else {
__split_buffer<value_type, allocator_type&> __v(__recommend(size() + __n), __p - this->__begin_, this->__alloc_);
__v.__construct_at_end_with_size(__first, __insertion_size);
__v.__construct_at_end_with_size(std::move(__first), __n);
__p = __swap_out_circular_buffer(__v, __p);
}
}
Expand Down
21 changes: 12 additions & 9 deletions libcxx/include/__vector/vector_bool.h
Original file line number Diff line number Diff line change
Expand Up @@ -420,9 +420,12 @@ class _LIBCPP_TEMPLATE_VIS vector<bool, _Allocator> {
template <class _Iterator, class _Sentinel>
_LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI void __assign_with_sentinel(_Iterator __first, _Sentinel __last);

template <class _ForwardIterator, class _Sentinel>
// The `_Iterator` in `*_with_size` functions can be input-only only if called from `*_range` (since C++23).
// Otherwise, `_Iterator` is a forward iterator.

template <class _Iterator, class _Sentinel>
_LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI void
__assign_with_size(_ForwardIterator __first, _Sentinel __last, difference_type __ns);
__assign_with_size(_Iterator __first, _Sentinel __last, difference_type __ns);

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

template <class _Allocator>
Expand Down Expand Up @@ -828,9 +831,9 @@ _LIBCPP_CONSTEXPR_SINCE_CXX20 void vector<bool, _Allocator>::assign(_ForwardIter
}

template <class _Allocator>
template <class _ForwardIterator, class _Sentinel>
template <class _Iterator, class _Sentinel>
_LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI void
vector<bool, _Allocator>::__assign_with_size(_ForwardIterator __first, _Sentinel __last, difference_type __ns) {
vector<bool, _Allocator>::__assign_with_size(_Iterator __first, _Sentinel __last, difference_type __ns) {
_LIBCPP_ASSERT_VALID_INPUT_RANGE(__ns >= 0, "invalid range specified");

clear();
Expand All @@ -841,7 +844,7 @@ vector<bool, _Allocator>::__assign_with_size(_ForwardIterator __first, _Sentinel
__vdeallocate();
__vallocate(__n);
}
__construct_at_end(__first, __last, __n);
__construct_at_end(std::move(__first), std::move(__last), __n);
}
}

Expand Down Expand Up @@ -986,10 +989,10 @@ vector<bool, _Allocator>::insert(const_iterator __position, _ForwardIterator __f
}

template <class _Allocator>
template <class _ForwardIterator, class _Sentinel>
template <class _Iterator, class _Sentinel>
_LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI typename vector<bool, _Allocator>::iterator
vector<bool, _Allocator>::__insert_with_size(
const_iterator __position, _ForwardIterator __first, _Sentinel __last, difference_type __n_signed) {
const_iterator __position, _Iterator __first, _Sentinel __last, difference_type __n_signed) {
_LIBCPP_ASSERT_VALID_INPUT_RANGE(__n_signed >= 0, "invalid range specified");
const size_type __n = static_cast<size_type>(__n_signed);
iterator __r;
Expand All @@ -1007,7 +1010,7 @@ vector<bool, _Allocator>::__insert_with_size(
std::copy_backward(__position, cend(), __v.end());
swap(__v);
}
std::__copy(__first, __last, __r);
std::__copy(std::move(__first), std::move(__last), __r);
return __r;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
// template<container-compatible-range<bool> R>
// constexpr void assign_range(R&& rg); // C++23

#include <sstream>
#include <vector>

#include "../insert_range_sequence_containers.h"
Expand Down Expand Up @@ -49,17 +50,39 @@ constexpr bool test() {
v.assign_range(in);
assert(std::ranges::equal(v, in));
}

{ // Ensure input-only sized ranges are accepted.
using input_iter = cpp20_input_iterator<const bool*>;
const bool in[]{true, true, false, true};
std::vector<bool> v;
v.assign_range(std::views::counted(input_iter{std::ranges::begin(in)}, std::ranges::ssize(in)));
assert(std::ranges::equal(v, std::vector<bool>{true, true, false, true}));
}
}

return true;
}

#ifndef TEST_HAS_NO_LOCALIZATION
void test_counted_istream_view() {
std::istringstream is{"1 1 0 1"};
auto vals = std::views::istream<bool>(is);
std::vector<bool> v;
v.assign_range(std::views::counted(vals.begin(), 3));
assert(v == (std::vector{true, true, false}));
}
#endif

int main(int, char**) {
test();
static_assert(test());

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

#ifndef TEST_HAS_NO_LOCALIZATION
test_counted_istream_view();
#endif

return 0;
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

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

#include <sstream>
#include <vector>

#include "../from_range_sequence_containers.h"
Expand All @@ -24,9 +25,25 @@ constexpr bool test() {
});
});

{ // Ensure input-only sized ranges are accepted.
using input_iter = cpp20_input_iterator<const bool*>;
const bool in[]{true, true, false, true};
std::vector v(std::from_range, std::views::counted(input_iter{std::ranges::begin(in)}, std::ranges::ssize(in)));
assert(std::ranges::equal(v, std::vector<bool>{true, true, false, true}));
}

return true;
}

#ifndef TEST_HAS_NO_LOCALIZATION
void test_counted_istream_view() {
std::istringstream is{"1 1 0 1"};
auto vals = std::views::istream<bool>(is);
std::vector v(std::from_range, std::views::counted(vals.begin(), 3));
assert(v == (std::vector{true, true, false}));
}
#endif

int main(int, char**) {
test();
static_assert(test());
Expand All @@ -36,5 +53,9 @@ int main(int, char**) {
// Note: test_exception_safety_throwing_copy doesn't apply because copying a boolean cannot throw.
test_exception_safety_throwing_allocator<std::vector, bool>();

#ifndef TEST_HAS_NO_LOCALIZATION
test_counted_istream_view();
#endif

return 0;
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
// template<container-compatible-range<bool> R>
// constexpr iterator insert_range(const_iterator position, R&& rg); // C++23

#include <sstream>
#include <vector>

#include "../insert_range_sequence_containers.h"
Expand Down Expand Up @@ -56,17 +57,39 @@ constexpr bool test() {
v.insert_range(v.end(), in);
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}));
}

{ // Ensure input-only sized ranges are accepted.
using input_iter = cpp20_input_iterator<const bool*>;
const bool in[]{true, true, false, true};
std::vector<bool> v{true, false};
v.insert_range(v.begin(), std::views::counted(input_iter{std::ranges::begin(in)}, std::ranges::ssize(in)));
assert(std::ranges::equal(v, std::vector<bool>{true, true, false, true, true, false}));
}
}

return true;
}

#ifndef TEST_HAS_NO_LOCALIZATION
void test_counted_istream_view() {
std::istringstream is{"1 1 0 1"};
auto vals = std::views::istream<bool>(is);
std::vector<bool> v;
v.insert_range(v.end(), std::views::counted(vals.begin(), 3));
assert(v == (std::vector{true, true, false}));
}
#endif

int main(int, char**) {
test();
static_assert(test());

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

#ifndef TEST_HAS_NO_LOCALIZATION
test_counted_istream_view();
#endif

return 0;
}
Loading
Loading