[libcxx-commits] [libcxx] [libc++] Fix input-only range handling for `vector` (PR #116157)

[Louis Dionne via libcxx-commits]

================
@@ -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
----------------
ldionne 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());
```

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.

https://github.com/llvm/llvm-project/pull/116157