[libcxx-commits] [libcxx] [libc++] Fix assignment in insertion into `vector` (PR #116001)
A. Jiang via libcxx-commits
libcxx-commits at lists.llvm.org
Tue Feb 18 02:57:57 PST 2025
https://github.com/frederick-vs-ja updated https://github.com/llvm/llvm-project/pull/116001
>From 902653c55c9242ac2d634e9bded0cd3ead53a827 Mon Sep 17 00:00:00 2001
From: "A. Jiang" <de34 at live.cn>
Date: Fri, 15 Nov 2024 09:59:39 +0800
Subject: [PATCH 1/3] [libc++] Fix assignment in insertion into `vector`
Changes:
- Avoid direct assignment in iterator-pair `insert` overload and
`insert_range`, except when the assignment is move assignment.
---
libcxx/include/__vector/vector.h | 36 ++++++++++++++-----
.../insert_iter_iter_iter.pass.cpp | 18 ++++++++++
.../vector.modifiers/insert_range.pass.cpp | 16 +++++++++
3 files changed, 61 insertions(+), 9 deletions(-)
diff --git a/libcxx/include/__vector/vector.h b/libcxx/include/__vector/vector.h
index 244506065db61..5b2e29504d324 100644
--- a/libcxx/include/__vector/vector.h
+++ b/libcxx/include/__vector/vector.h
@@ -58,6 +58,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>
@@ -602,6 +603,30 @@ class _LIBCPP_TEMPLATE_VIS vector {
_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
+ __insert_assign_n_unchecked(_Iterator __first, difference_type __n, pointer __position) {
+#if _LIBCPP_STD_VER >= 23
+ if constexpr (!forward_iterator<_Iterator>) {
+ ranges::copy_n(std::move(__first), __n, __position);
+ } else
+#endif
+ {
+ std::copy_n(__first, __n, __position);
+ }
+ }
+
template <class _InputIterator, class _Sentinel>
_LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI iterator
__insert_with_sentinel(const_iterator __position, _InputIterator __first, _Sentinel __last);
@@ -1322,19 +1347,12 @@ vector<_Tp, _Allocator>::__insert_with_size(
__construct_at_end(__m, __last, __n - __dx);
if (__dx > 0) {
__move_range(__p, __old_last, __p + __n);
- std::copy(__first, __m, __p);
+ __insert_assign_n_unchecked(__first, __dx, __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);
- }
+ __insert_assign_n_unchecked(std::move(__first), __n, __p);
}
} else {
__split_buffer<value_type, allocator_type&> __v(__recommend(size() + __n), __p - this->__begin_, this->__alloc_);
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 e4625e4865c4d..34b13a5c7d826 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
@@ -161,6 +161,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/insert_range.pass.cpp b/libcxx/test/std/containers/sequences/vector/vector.modifiers/insert_range.pass.cpp
index ea0573df73751..ef63db083a998 100644
--- a/libcxx/test/std/containers/sequences/vector/vector.modifiers/insert_range.pass.cpp
+++ b/libcxx/test/std/containers/sequences/vector/vector.modifiers/insert_range.pass.cpp
@@ -64,6 +64,22 @@ constexpr bool test() {
}
}
+ { // Ensure that insert_range doesn't use unexpected assignment.
+ struct Wrapper {
+ constexpr Wrapper(int n) : n_(n) {}
+ void operator=(int) = delete;
+
+ int n_;
+ };
+
+ int a[]{1, 2, 3, 4, 5};
+ std::vector<Wrapper> v;
+ v.insert_range(v.end(), a);
+ assert(v.size() == std::size(a));
+ for (std::size_t i = 0; i != std::size(a); ++i)
+ assert(v[i].n_ == a[i]);
+ }
+
return true;
}
>From aa6607fb88eb28fb62326538b9db993657a3cd05 Mon Sep 17 00:00:00 2001
From: "A. Jiang" <de34 at live.cn>
Date: Tue, 18 Feb 2025 18:57:10 +0800
Subject: [PATCH 2/3] Clang-format
`vector.modifiers/insert_iter_iter_iter.pass.cpp`
---
.../insert_iter_iter_iter.pass.cpp | 30 +++++++++----------
1 file changed, 15 insertions(+), 15 deletions(-)
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 34b13a5c7d826..efd553c4e93dd 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
@@ -161,24 +161,24 @@ TEST_CONSTEXPR_CXX20 bool tests() {
for (; j < 105; ++j)
assert(v[j] == 0);
}
- {
- struct Wrapper {
- TEST_CONSTEXPR Wrapper(int n) : n_(n) {}
+ {
+ struct Wrapper {
+ TEST_CONSTEXPR Wrapper(int n) : n_(n) {}
- int n_;
+ int n_;
- private:
- void operator=(int);
- };
+ 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]);
- }
+ 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;
>From a0af3eba2b06701c971168059d519860cb33e7da Mon Sep 17 00:00:00 2001
From: "A. Jiang" <de34 at live.cn>
Date: Tue, 18 Feb 2025 18:57:41 +0800
Subject: [PATCH 3/3] Adopt @winner245's review comments
---
libcxx/include/__vector/vector.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libcxx/include/__vector/vector.h b/libcxx/include/__vector/vector.h
index 5b2e29504d324..f3d12a6e2e4a9 100644
--- a/libcxx/include/__vector/vector.h
+++ b/libcxx/include/__vector/vector.h
@@ -607,7 +607,7 @@ class _LIBCPP_TEMPLATE_VIS vector {
__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) {
+ for (pointer __end_position = __position + __n; __position != __end_position; ++__position, (void)++__first) {
__temp_value<value_type, _Allocator> __tmp(this->__alloc(), *__first);
*__position = std::move(__tmp.get());
}
More information about the libcxx-commits
mailing list