[libc++] Fix {deque,vector}::append_range assuming too much about the types

Author: philnik777

libcxx/docs/ReleaseNotes/22.rst

@@ -65,6 +65,7 @@ Improvements and New Features
   in chunks into a buffer.
 - Multiple internal types have been refactored to use ``[[no_unique_address]]``, resulting in faster compile times and
   reduced debug information.
+- The performance of ``deque::append_range`` has been improved by up to 3.4x
 
 - The performance of ``std::find`` has been improved by up to 2x for integral types
 

libcxx/include/__vector/vector.h

@@ -42,6 +42,7 @@
 #include <__memory/temp_value.h>
 #include <__memory/uninitialized_algorithms.h>
 #include <__ranges/access.h>
+#include <__ranges/as_rvalue_view.h>
 #include <__ranges/concepts.h>
 #include <__ranges/container_compatible_range.h>
 #include <__ranges/from_range.h>
@@ -489,7 +490,21 @@ class vector {
 #if _LIBCPP_STD_VER >= 23
   template <_ContainerCompatibleRange<_Tp> _Range>
   _LIBCPP_HIDE_FROM_ABI constexpr void append_range(_Range&& __range) {
-    insert_range(end(), std::forward<_Range>(__range));
+    if constexpr (ranges::forward_range<_Range> || ranges::sized_range<_Range>) {
+      auto __len = ranges::distance(__range);
+      if (__len < __cap_ - __end_) {
+        __construct_at_end(ranges::begin(__range), ranges::end(__range), __len);
+      } else {
+        __split_buffer<value_type, allocator_type&> __buffer(__recommend(size() + __len), size(), __alloc_);
+        __buffer.__construct_at_end_with_size(ranges::begin(__range), __len);
+        __swap_out_circular_buffer(__buffer);
+      }
+    } else {
+      vector __buffer(__alloc_);
+      for (auto&& __val : __range)
+        __buffer.emplace_back(std::forward<decltype(__val)>(__val));
+      append_range(ranges::as_rvalue_view(__buffer));
+    }
   }
 #endif
 

libcxx/include/deque

@@ -807,7 +807,11 @@ public:
 
   template <_ContainerCompatibleRange<_Tp> _Range>
   _LIBCPP_HIDE_FROM_ABI void append_range(_Range&& __range) {
-    insert_range(end(), std::forward<_Range>(__range));
+    if constexpr (ranges::forward_range<_Range> || ranges::sized_range<_Range>) {
+      __append_with_size(ranges::begin(__range), ranges::distance(__range));
+    } else {
+      __append_with_sentinel(ranges::begin(__range), ranges::end(__range));
+    }
   }
 #    endif
 

libcxx/test/benchmarks/containers/sequence/sequence_container_benchmarks.h

@@ -309,7 +309,7 @@ void sequence_container_benchmarks(std::string container) {
   }
 
   /////////////////////////
-  // Variations of push_back
+  // Appending elements
   /////////////////////////
   static constexpr bool has_push_back = requires(Container c, ValueType v) { c.push_back(v); };
   static constexpr bool has_capacity  = requires(Container c) { c.capacity(); };
@@ -399,6 +399,53 @@ void sequence_container_benchmarks(std::string container) {
           st.ResumeTiming();
         }
       });
+
+#if TEST_STD_VER >= 23
+    for (auto gen : generators)
+      bench("append_range()" + tostr(gen), [gen](auto& state) {
+        auto const size = state.range(0);
+        std::vector<ValueType> in;
+        std::generate_n(std::back_inserter(in), size, gen);
+        DoNotOptimizeData(in);
+
+        Container c;
+        DoNotOptimizeData(c);
+        for (auto _ : state) {
+          c.append_range(in);
+          DoNotOptimizeData(c);
+
+          state.PauseTiming();
+          c.clear();
+          state.ResumeTiming();
+        }
+      });
+#endif
+  }
+
+  /////////////////////////
+  // Prepending elements
+  /////////////////////////
+  static constexpr bool has_prepend_range = requires(Container c, std::vector<ValueType> v) { c.prepend_range(v); };
+
+  if constexpr (has_prepend_range) {
+    for (auto gen : generators)
+      bench("prepend_range()" + tostr(gen), [gen](auto& state) {
+        auto const size = state.range(0);
+        std::vector<ValueType> in;
+        std::generate_n(std::back_inserter(in), size, gen);
+        DoNotOptimizeData(in);
+
+        Container c;
+        DoNotOptimizeData(c);
+        for (auto _ : state) {
+          c.prepend_range(in);
+          DoNotOptimizeData(c);
+
+          state.PauseTiming();
+          c.clear();
+          state.ResumeTiming();
+        }
+      });
   }
 
   /////////////////////////

libcxx/test/std/containers/insert_range_helpers.h

@@ -87,6 +87,11 @@ constexpr void for_all_iterators_and_allocators(Func f) {
     f.template operator()<Iter, sentinel_wrapper<Iter>, min_allocator<T>>();
     f.template operator()<Iter, sentinel_wrapper<Iter>, safe_allocator<T>>();
 
+    f.template operator()<Iter, sized_sentinel<Iter>, std::allocator<T>>();
+    f.template operator()<Iter, sized_sentinel<Iter>, test_allocator<T>>();
+    f.template operator()<Iter, sized_sentinel<Iter>, min_allocator<T>>();
+    f.template operator()<Iter, sized_sentinel<Iter>, safe_allocator<T>>();
+
     if constexpr (std::sentinel_for<Iter, Iter>) {
       f.template operator()<Iter, Iter, std::allocator<T>>();
       f.template operator()<Iter, Iter, test_allocator<T>>();

libcxx/test/std/containers/sequences/deque/deque.modifiers/append_range.pass.cpp

@@ -31,6 +31,7 @@ int main(int, char**) {
     });
   });
   test_sequence_append_range_move_only<std::deque>();
+  test_sequence_append_range_emplace_constructible<std::deque>();
 
   test_append_range_exception_safety_throwing_copy<std::deque>();
   test_append_range_exception_safety_throwing_allocator<std::deque, int>();

libcxx/test/std/containers/sequences/deque/deque.modifiers/prepend_range.pass.cpp

@@ -31,6 +31,8 @@ int main(int, char**) {
     });
   });
   test_sequence_prepend_range_move_only<std::deque>();
+  // FIXME: This should work - see https://llvm.org/PR162605
+  // test_sequence_prepend_range_emplace_constructible<std::deque>();
 
   test_prepend_range_exception_safety_throwing_copy<std::deque>();
   test_prepend_range_exception_safety_throwing_allocator<std::deque, int>();

libcxx/test/std/containers/sequences/forwardlist/forwardlist.modifiers/prepend_range.pass.cpp

@@ -30,6 +30,7 @@ TEST_CONSTEXPR_CXX26 bool test() {
     });
   });
   test_sequence_prepend_range_move_only<std::forward_list>();
+  test_sequence_prepend_range_emplace_constructible<std::forward_list>();
 
   if (!TEST_IS_CONSTANT_EVALUATED) {
     test_prepend_range_exception_safety_throwing_copy<std::forward_list>();

libcxx/test/std/containers/sequences/insert_range_sequence_containers.h

@@ -643,6 +643,62 @@ constexpr void test_sequence_assign_range_move_only() {
   c.assign_range(in);
 }
 
+struct InPlaceOnly {
+  constexpr InPlaceOnly() {}
+  InPlaceOnly(const InPlaceOnly&) = delete;
+  InPlaceOnly(InPlaceOnly&&) = delete;
+  InPlaceOnly& operator=(const InPlaceOnly&) = delete;
+  InPlaceOnly& operator=(InPlaceOnly&&) = delete;
+};
+
+struct EmplaceConstructible {
+  EmplaceConstructible(const EmplaceConstructible&) = delete;
+  EmplaceConstructible& operator=(const EmplaceConstructible&) = delete;
+  EmplaceConstructible& operator=(EmplaceConstructible&&) = delete;
+  EmplaceConstructible(EmplaceConstructible&&) = delete;
+  constexpr EmplaceConstructible(const InPlaceOnly&) {}
+};
+
+template <template <class...> class Container>
+constexpr void test_sequence_append_range_emplace_constructible() {
+  InPlaceOnly input[5];
+  types::for_each(types::cpp20_input_iterator_list<InPlaceOnly*>{}, [&]<class Iter> {
+    std::ranges::subrange in(Iter(input), sentinel_wrapper<Iter>(Iter(input + 5)));
+    Container<EmplaceConstructible> c;
+    c.append_range(in);
+  });
+}
+
+template <template <class...> class Container>
+constexpr void test_sequence_prepend_range_emplace_constructible() {
+  InPlaceOnly input[5];
+  types::for_each(types::cpp20_input_iterator_list<InPlaceOnly*>{}, [&]<class Iter> {
+    std::ranges::subrange in(Iter(input), sentinel_wrapper<Iter>(Iter(input + 5)));
+    Container<EmplaceConstructible> c;
+    c.prepend_range(in);
+  });
+}
+
+// vector has a special requirement that the type also has to be Cpp17MoveInsertable
+
+struct EmplaceConstructibleAndMoveInsertable {
+  EmplaceConstructibleAndMoveInsertable(const EmplaceConstructibleAndMoveInsertable&) = delete;
+  EmplaceConstructibleAndMoveInsertable& operator=(const EmplaceConstructibleAndMoveInsertable&) = delete;
+  EmplaceConstructibleAndMoveInsertable& operator=(EmplaceConstructibleAndMoveInsertable&&) = delete;
+  constexpr EmplaceConstructibleAndMoveInsertable(EmplaceConstructibleAndMoveInsertable&&) {}
+  constexpr EmplaceConstructibleAndMoveInsertable(const InPlaceOnly&) {}
+};
+
+template <template <class...> class Container>
+constexpr void test_sequence_append_range_emplace_constructible_and_move_insertable() {
+  InPlaceOnly input[5];
+  types::for_each(types::cpp20_input_iterator_list<InPlaceOnly*>{}, [&]<class Iter> {
+    std::ranges::subrange in(Iter(input), sentinel_wrapper<Iter>(Iter(input + 5)));
+    Container<EmplaceConstructibleAndMoveInsertable> c;
+    c.append_range(in);
+  });
+}
+
 // Exception safety.
 
 template <template <class...> class Container>

libcxx/test/std/containers/sequences/list/list.modifiers/append_range.pass.cpp

@@ -31,6 +31,7 @@ TEST_CONSTEXPR_CXX26 bool test() {
     });
   });
   test_sequence_append_range_move_only<std::list>();
+  test_sequence_append_range_emplace_constructible<std::list>();
 
   if (!TEST_IS_CONSTANT_EVALUATED) {
     test_append_range_exception_safety_throwing_copy<std::list>();