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

frederick-vs-ja · frederick-vs-ja · commit cac6fe48ab7c · 2024-11-15T09:19:16.000+08:00
Changes:
- Carve out sized but input-only ranges for C++23.
- Call `std::move` for related functions when the iterator is possibly input-only.
diff --git 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
@@ -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>
@@ -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>
@@ -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,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
@@ -916,7 +922,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 +1029,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 +1311,41 @@ 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);
+            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 {
       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
@@ -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
@@ -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"
@@ -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;
 }
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
@@ -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"
@@ -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());
@@ -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;
 }
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
@@ -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"
@@ -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;
 }
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
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
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
