-
Notifications
You must be signed in to change notification settings - Fork 12k
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
[libc++] Fix assignment in insertion into vector
#116001
base: main
Are you sure you want to change the base?
Conversation
@llvm/pr-subscribers-libcxx Author: A. Jiang (frederick-vs-ja) ChangesChanges:
Fixes #47963. Fixes #115727. Fixes #115801. Patch is 21.60 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/116001.diff 11 Files Affected:
diff --git a/libcxx/include/__memory/uninitialized_algorithms.h b/libcxx/include/__memory/uninitialized_algorithms.h
index 627ee44e808d9c..16318d6c76f9e7 100644
--- a/libcxx/include/__memory/uninitialized_algorithms.h
+++ b/libcxx/include/__memory/uninitialized_algorithms.h
@@ -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);
}
diff --git a/libcxx/include/__vector/vector.h b/libcxx/include/__vector/vector.h
index d6bf58d02c414c..2a58d6a80a56e2 100644
--- a/libcxx/include/__vector/vector.h
+++ b/libcxx/include/__vector/vector.h
@@ -15,6 +15,7 @@
#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>
@@ -23,6 +24,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>
@@ -54,6 +56,7 @@
#include <__type_traits/is_same.h>
#include <__type_traits/is_trivially_relocatable.h>
#include <__type_traits/type_identity.h>
+#include <__utility/declval.h>
#include <__utility/exception_guard.h>
#include <__utility/forward.h>
#include <__utility/is_pointer_in_range.h>
@@ -570,7 +573,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();
@@ -590,9 +593,31 @@ 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(_Iterator __first, _Sentinel __last, difference_type __n);
+
+ template <class _Iterator,
+ __enable_if_t<!is_same<decltype(*std::declval<_Iterator&>())&&, value_type&&>::value, int> = 0>
+ _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI void
+ __insert_assign_n_unchecked(_Iterator __first, difference_type __n, pointer __position) {
+ for (pointer __end_position = __position + __n; __position != __end_position; (void)++__position, ++__first) {
+ __temp_value<value_type, _Allocator> __tmp(this->__alloc(), *__first);
+ *__position = std::move(__tmp.get());
+ }
+ }
+
+ template <class _Iterator,
+ __enable_if_t<is_same<decltype(*std::declval<_Iterator&>())&&, value_type&&>::value, int> = 0>
_LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI void
- __assign_with_size(_ForwardIterator __first, _Sentinel __last, difference_type __n);
+ __insert_assign_n_unchecked(_Iterator __first, difference_type __n, pointer __position) {
+ for (pointer __end_position = __position + __n; __position != __end_position; (void)++__position, ++__first) {
+ *__position = *__first;
+ }
+ }
template <class _InputIterator, class _Sentinel>
_LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI iterator
@@ -916,7 +941,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(__alloc(), __first, __last, __tx.__pos_);
+ __tx.__pos_ = std::__uninitialized_allocator_copy(__alloc(), std::move(__first), std::move(__last), __tx.__pos_);
}
// Default constructs __n objects starting at __end_
@@ -1023,23 +1048,31 @@ 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());
- std::copy(__first, __mid, this->__begin_);
- __construct_at_end(__mid, __last, __new_size - size());
+#if _LIBCPP_STD_VER >= 23
+ if constexpr (!forward_iterator<_Iterator>) {
+ 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
+#endif
+ {
+ _Iterator __mid = std::next(__first, size());
+ std::copy(__first, __mid, this->__begin_);
+ __construct_at_end(__mid, __last, __new_size - size());
+ }
} 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);
}
}
@@ -1297,29 +1330,34 @@ 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->__end_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);
+ __insert_assign_n_unchecked(__first, __dx, __p);
+ }
+ }
+ } else {
+ __move_range(__p, __old_last, __p + __n);
+ __insert_assign_n_unchecked(std::move(__first), __n, __p, __p);
}
} else {
allocator_type& __a = this->__alloc();
__split_buffer<value_type, allocator_type&> __v(__recommend(size() + __n), __p - this->__begin_, __a);
- __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);
}
}
diff --git a/libcxx/include/__vector/vector_bool.h b/libcxx/include/__vector/vector_bool.h
index bc6a61ad3215fb..13207df1a8cff2 100644
--- a/libcxx/include/__vector/vector_bool.h
+++ b/libcxx/include/__vector/vector_bool.h
@@ -415,9 +415,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
@@ -574,7 +577,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>
@@ -824,9 +827,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();
@@ -837,7 +840,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);
}
}
@@ -981,10 +984,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;
@@ -1002,7 +1005,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;
}
diff --git a/libcxx/test/std/containers/sequences/vector.bool/assign_range.pass.cpp b/libcxx/test/std/containers/sequences/vector.bool/assign_range.pass.cpp
index e5d0454a844d5a..c1315379da48c1 100644
--- a/libcxx/test/std/containers/sequences/vector.bool/assign_range.pass.cpp
+++ b/libcxx/test/std/containers/sequences/vector.bool/assign_range.pass.cpp
@@ -12,6 +12,8 @@
// template<container-compatible-range<bool> R>
// constexpr void assign_range(R&& rg); // C++23
+#include <cassert>
+#include <sstream>
#include <vector>
#include "../insert_range_sequence_containers.h"
@@ -54,6 +56,14 @@ constexpr bool test() {
return true;
}
+void test_sized_input_only_range() {
+ 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}));
+}
+
int main(int, char**) {
test();
static_assert(test());
@@ -61,5 +71,7 @@ int main(int, char**) {
// 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>();
+ test_sized_input_only_range();
+
return 0;
}
diff --git a/libcxx/test/std/containers/sequences/vector.bool/construct_from_range.pass.cpp b/libcxx/test/std/containers/sequences/vector.bool/construct_from_range.pass.cpp
index 03f3100b928833..1a46b3027d72bc 100644
--- a/libcxx/test/std/containers/sequences/vector.bool/construct_from_range.pass.cpp
+++ b/libcxx/test/std/containers/sequences/vector.bool/construct_from_range.pass.cpp
@@ -8,6 +8,8 @@
// UNSUPPORTED: c++03, c++11, c++14, c++17, c++20
+#include <cassert>
+#include <sstream>
#include <vector>
#include "../from_range_sequence_containers.h"
@@ -27,6 +29,13 @@ constexpr bool test() {
return true;
}
+void test_sized_input_only_range() {
+ 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}));
+}
+
int main(int, char**) {
test();
static_assert(test());
@@ -36,5 +45,7 @@ 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>();
+ test_sized_input_only_range();
+
return 0;
}
diff --git a/libcxx/test/std/containers/sequences/vector.bool/insert_iter_iter_iter.pass.cpp b/libcxx/test/std/containers/sequences/vector.bool/insert_iter_iter_iter.pass.cpp
index d3e1297aeec928..0caa435574d4eb 100644
--- a/libcxx/test/std/containers/sequences/vector.bool/insert_iter_iter_iter.pass.cpp
+++ b/libcxx/test/std/containers/sequences/vector.bool/insert_iter_iter_iter.pass.cpp
@@ -91,6 +91,24 @@ TEST_CONSTEXPR_CXX20 bool tests()
for (; j < v.size(); ++j)
assert(v[j] == 0);
}
+ {
+ struct Wrapper {
+ TEST_CONSTEXPR Wrapper(bool b) : b_(b) {}
+
+ bool b_;
+
+ private:
+ void operator=(bool);
+ };
+
+ int a[] = {true, true, false, true, false};
+ const std::size_t count = sizeof(a) / sizeof(a[0]);
+ std::vector<Wrapper> v;
+ v.insert(v.end(), a, a + count);
+ assert(v.size() == count);
+ for (std::size_t i = 0; i != count; ++i)
+ assert(v[i].b_ == a[i]);
+ }
#if TEST_STD_VER >= 11
{
std::vector<bool, explicit_allocator<bool>> v(100);
diff --git a/libcxx/test/std/containers/sequences/vector.bool/insert_range.pass.cpp b/libcxx/test/std/containers/sequences/vector.bool/insert_range.pass.cpp
index 65d085fa1f0832..d31c49d4add6f3 100644
--- a/libcxx/test/std/containers/sequences/vector.bool/insert_range.pass.cpp
+++ b/libcxx/test/std/containers/sequences/vector.bool/insert_range.pass.cpp
@@ -12,6 +12,8 @@
// template<container-compatible-range<bool> R>
// constexpr iterator insert_range(const_iterator position, R&& rg); // C++23
+#include <cassert>
+#include <sstream>
#include <vector>
#include "../insert_range_sequence_containers.h"
@@ -61,6 +63,14 @@ constexpr bool test() {
return true;
}
+void test_sized_input_only_range() {
+ 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}));
+}
+
int main(int, char**) {
test();
static_assert(test());
@@ -68,5 +78,7 @@ int main(int, char**) {
// 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>();
+ test_sized_input_only_range();
+
return 0;
}
diff --git a/libcxx/test/std/containers/sequences/vector/vector.cons/construct_from_range.pass.cpp b/libcxx/test/std/containers/sequences/vector/vector.cons/construct_from_range.pass.cpp
index 5fb2b46f7e9420..b912e3f2eae518 100644
--- a/libcxx/test/std/containers/sequences/vector/vector.cons/construct_from_range.pass.cpp
+++ b/libcxx/test/std/containers/sequences/vector/vector.cons/construct_from_range.pass.cpp
@@ -11,6 +11,8 @@
// template<container-compatible-range<T> R>
// vector(from_range_t, R&& rg, const Allocator& = Allocator()); // C++23
+#include <cassert>
+#include <sstream>
#include <vector>
#include "../../from_range_sequence_containers.h"
@@ -29,6 +31,13 @@ constexpr bool test() {
return true;
}
+void test_sized_input_only_range() {
+ std::istringstream is{"1 2 3 4"};
+ auto vals = std::views::istream<int>(is);
+ std::vector v(std::from_range, std::views::counted(vals.begin(), 3));
+ assert(v == (std::vector{1, 2, 3}));
+}
+
int main(int, char**) {
static_assert(test_constraints<std::vector, int, double>());
test();
@@ -38,5 +47,7 @@ int main(int, char**) {
test_exception_safety_throwing_copy<std::vector>();
test_exception_safety_throwing_allocator<std::vector, int>();
+ test_sized_input_only_range();
+
return 0;
}
diff --git a/libcxx/test/std/containers/sequences/vector/vector.modifiers/assign_range.pass.cpp b/libcxx/test/std/containers/sequences/vector/vector.modifiers/assign_range.pass.cpp
index 8ab3dc10aed990..053e4402381141 100644
--- a/libcxx/test/std/containers/sequences/vector/vector.modifiers/assign_range.pass.cpp
+++ b/libcxx/test/std/containers/sequences/vector/vector.modifiers/assign_range.pass.cpp
@@ -12,6 +12,8 @@
// template<container-compatible-range<T> R>
// constexpr void assign_range(R&& rg); // C++23
+#include <cassert>
+#include <sstream>
#include <vector>
#include "../../insert_range_sequence_containers.h"
@@ -67,6 +69,14 @@ constexpr bool test() {
return true;
}
+void test_sized_input_only_range() {
+ std::istringstream is{"1 2 3 4"};
+ auto vals = std::views::istream<int>(is);
+ std::vector<int> v;
+ v.assign_range(std::views::counted(vals.begin(), 3));
+ assert(v == (std::vector{1, 2, 3}));
+}
+
int main(int, char**) {
test();
static_assert(test());
@@ -74,5 +84,7 @@ int main(int, char**) {
test_assign_range_exception_safety_throwing_copy<std::vector>();
test_assign_range_exception_safety_throwing_allocator<std::vector, int>();
+ test_sized_input_only_range();
+
return 0;
}
diff --git a/libcxx/test/std/containers/sequences/vector/vector.modifiers/insert_iter_iter_iter.pass.cpp b/libcxx/test/std/containers/sequences/vector/vector.modifiers/insert_iter_iter_iter.pass.cpp
index 934b85ce01c67b..4b7c47cc6388d9 100644
--- a/libcxx/test/std/containers/sequences/vector/vector.modifiers/insert_iter_iter_iter.pass.cpp
+++ b/libcxx/test/std/containers/sequences/vector/vector.modifiers/insert_iter_iter_iter.pass.cpp
@@ -138,6 +138,24 @@ TEST_CONSTEXPR_CXX20 bool tests()
for (; j < 105; ++j)
assert(v[j] == 0);
}
+ {
+ struct Wrapper {
+ TEST_CONSTEXPR Wrapper(int n) : n_(n) {}
+
+ int n_;
+
+ private:
+ void operator=(int);
+ };
+
+ int a[] = {1, 2, 3, 4, 5};
+ const std::size_t count = sizeof(a) / sizeof(a[0]);
+ std::vector<Wrapper> v;
+ v.insert(v.end(), a, a + count);
+ assert(v.size() == count);
+ for (std::size_t i = 0; i != count; ++i)
+ assert(v[i].n_ == a[i]);
+ }
#if TEST_STD_VER >= 11
{
typedef std::vector<int, min_allocator<int> > V;
diff --git a/libcxx/test/std/containers/sequences/vector/vector.modifiers/in...
[truncated]
|
5afcb30
to
7abcb4f
Compare
Please split adding |
I think I should split out the handling for input-only iterators, which is more than adding |
Sure. Which way you split is up to you. I only care that it is split. |
Split to #116157. I sadly realized why I placed the changes in one PR - |
vector
vector
f89d701
to
426462c
Compare
Changes: - Avoid direct assignment in iterator-pair `insert` overload and `insert_range`, except when the assignment is move assignment.
426462c
to
7ea886b
Compare
Changes:
insert
overload andinsert_range
, except when the assignment is move assignment.Fixes #47963. Fixes #115801.