[libcxx-commits] [libcxx] [libc++] Fix input-only range handling for `vector` (PR #116157)
A. Jiang via libcxx-commits
libcxx-commits at lists.llvm.org
Sun Dec 22 07:35:58 PST 2024
================
@@ -1011,23 +1017,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
----------------
frederick-vs-ja wrote:
> This trailing `else` is really confusing IMO! I think you can do this instead:
>
> ```c++
> #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());
> return;
> }
> #endif
>
> // not under any else
> _Iterator __mid = std::next(__first, size());
> std::copy(__first, __mid, this->__begin_);
> __construct_at_end(__mid, __last, __new_size - size());
> ```
Oh, we can't, because the "else" branch needs to be discarded for input-only iterators.
----
> Actually, is there a reason why we don't do this instead?
>
> ```c++
> #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
> ```
>
> IOW, I don't see why we can't use `ranges::copy_n` regardless of the iterator category in C++23.
Thanks! I think this should work.
https://github.com/llvm/llvm-project/pull/116157
More information about the libcxx-commits
mailing list