[libc++] Fix insertion into deque from prvalue ranges

[frederick-vs-ja]

When the iterator of the source range in *_range functions dereferences to a prvalue, it should be at most Cpp17InputIterator while possibly modeling random_access_iterator. When inserting such a range into a container, we should use ranges::prev/ranges::next instead of std::prev/std::next internally.

Fixes #159943.

[llvmbot]

@llvm/pr-subscribers-libcxx

Author: A. Jiang (frederick-vs-ja)

Changes

When the iterator of the source range in *_range functions dereferences to a prvalue, it should be at most Cpp17InputIterator while possibly modeling random_access_iterator. When inserting such a range into a container, we should use ranges::prev/ranges::next instead of std::prev/std::next internally.

In addition to deque, test coverage for prvalue ranges is added for all sequence containers. We can cover other containers and container adaptors later.

Fixes #159943.

---

Full diff: https://github.com/llvm/llvm-project/pull/160022.diff

3 Files Affected:

• (modified) libcxx/include/deque (+9-2)
• (modified) libcxx/test/std/containers/from_range_helpers.h (+35)
• (modified) libcxx/test/std/containers/sequences/insert_range_sequence_containers.h (+84-26)

diff --git a/libcxx/include/deque b/libcxx/include/deque
index 98d1dbbddb7e8..ea83c50bceea0 100644
--- a/libcxx/include/deque
+++ b/libcxx/include/deque
@@ -185,6 +185,7 @@ template <class T, class Allocator, class Predicate>
 #  include <__algorithm/copy_n.h>
 #  include <__algorithm/equal.h>
 #  include <__algorithm/fill_n.h>
+#  include <__algorithm/iterator_operations.h>
 #  include <__algorithm/lexicographical_compare.h>
 #  include <__algorithm/lexicographical_compare_three_way.h>
 #  include <__algorithm/max.h>
@@ -1916,6 +1917,12 @@ template <class _Tp, class _Allocator>
 template <class _BiIter>
 _LIBCPP_HIDE_FROM_ABI typename deque<_Tp, _Allocator>::iterator
 deque<_Tp, _Allocator>::__insert_bidirectional(const_iterator __p, _BiIter __f, _BiIter __l, size_type __n) {
+#  if _LIBCPP_STD_VER >= 20
+  using _Ops = _IterOps<
+      conditional_t<__has_bidirectional_iterator_category<_BiIter>::value, _ClassicAlgPolicy, _RangeAlgPolicy>>;
+#  else
+  using _Ops = _IterOps<_ClassicAlgPolicy>;
+#  endif
   size_type __pos     = __p - begin();
   size_type __to_end  = size() - __pos;
   allocator_type& __a = __alloc();
@@ -1928,7 +1935,7 @@ deque<_Tp, _Allocator>::__insert_bidirectional(const_iterator __p, _BiIter __f,
     iterator __i         = __old_begin;
     _BiIter __m          = __f;
     if (__n > __pos) {
-      __m = __pos < __n / 2 ? std::prev(__l, __pos) : std::next(__f, __n - __pos);
+      __m = __pos < __n / 2 ? _Ops::prev(__l, __pos) : _Ops::next(__f, __n - __pos);
       for (_BiIter __j = __m; __j != __f; --__start_, ++__size())
         __alloc_traits::construct(__a, std::addressof(*--__i), *--__j);
       __n = __pos;
@@ -1955,7 +1962,7 @@ deque<_Tp, _Allocator>::__insert_bidirectional(const_iterator __p, _BiIter __f,
     _BiIter __m        = __l;
     size_type __de     = size() - __pos;
     if (__n > __de) {
-      __m = __de < __n / 2 ? std::next(__f, __de) : std::prev(__l, __n - __de);
+      __m = __de < __n / 2 ? _Ops::next(__f, __de) : _Ops::prev(__l, __n - __de);
       for (_BiIter __j = __m; __j != __l; ++__i, (void)++__j, ++__size())
         __alloc_traits::construct(__a, std::addressof(*__i), *__j);
       __n = __de;
diff --git a/libcxx/test/std/containers/from_range_helpers.h b/libcxx/test/std/containers/from_range_helpers.h
index edf1d6ce528d7..45cbd803cac8c 100644
--- a/libcxx/test/std/containers/from_range_helpers.h
+++ b/libcxx/test/std/containers/from_range_helpers.h
@@ -10,9 +10,12 @@
 #define SUPPORT_FROM_RANGE_HELPERS_H
 
 #include <array>
+#include <concepts>
 #include <cstddef>
 #include <iterator>
+#include <ranges>
 #include <type_traits>
+#include <utility>
 #include <vector>
 
 #include "min_allocator.h"
@@ -50,6 +53,38 @@ constexpr auto wrap_input(std::vector<T>& input) {
   return std::ranges::subrange(std::move(b), std::move(e));
 }
 
+struct DecayCopy {
+  template <class T>
+    requires std::convertible_to<T, std::decay_t<T>>
+  constexpr std::decay_t<T> operator()(T&& t) {
+    return std::forward<T>(t);
+  }
+};
+
+template <class Iter, class Sent, std::ranges::input_range Range>
+constexpr auto wrap_input_decay(Range&& input) {
+  auto b = Iter(std::ranges::begin(input));
+  auto e = Sent(Iter(std::ranges::end(input)));
+  if constexpr (std::is_reference_v<std::iter_reference_t<Iter>>)
+    return std::ranges::subrange(std::move(b), std::move(e)) | std::views::transform(DecayCopy{});
+  else
+    return std::ranges::subrange(std::move(b), std::move(e));
+}
+
+template <class Iter, class Sent, class T, std::size_t N>
+constexpr auto wrap_input_decay(std::array<T, N>& input) {
+  auto b = Iter(input.data());
+  auto e = Sent(Iter(input.data() + input.size()));
+  return std::ranges::subrange(std::move(b), std::move(e)) | std::views::transform(DecayCopy{});
+}
+
+template <class Iter, class Sent, class T>
+constexpr auto wrap_input_decay(std::vector<T>& input) {
+  auto b = Iter(input.data());
+  auto e = Sent(Iter(input.data() + input.size()));
+  return std::ranges::subrange(std::move(b), std::move(e)) | std::views::transform(DecayCopy{});
+}
+
 struct KeyValue {
   int key;    // Only the key is considered for equality comparison.
   char value; // Allows distinguishing equivalent instances.
diff --git a/libcxx/test/std/containers/sequences/insert_range_sequence_containers.h b/libcxx/test/std/containers/sequences/insert_range_sequence_containers.h
index 9f404c46df778..07f069e3176ce 100644
--- a/libcxx/test/std/containers/sequences/insert_range_sequence_containers.h
+++ b/libcxx/test/std/containers/sequences/insert_range_sequence_containers.h
@@ -402,14 +402,30 @@ constexpr void test_sequence_insert_range(Validate validate) {
   auto get_pos = [](auto& c, auto& test_case) { return std::ranges::next(c.begin(), static_cast<D>(test_case.index)); };
 
   auto test = [&](auto& test_case) {
-    Container c(test_case.initial.begin(), test_case.initial.end());
-    auto in  = wrap_input<Iter, Sent>(test_case.input);
-    auto pos = get_pos(c, test_case);
-
-    auto result = c.insert_range(pos, in);
-    assert(result == get_pos(c, test_case));
-    validate(c);
-    return std::ranges::equal(c, test_case.expected);
+    {
+      Container c(test_case.initial.begin(), test_case.initial.end());
+      auto in  = wrap_input<Iter, Sent>(test_case.input);
+      auto pos = get_pos(c, test_case);
+
+      auto result = c.insert_range(pos, in);
+      assert(result == get_pos(c, test_case));
+      validate(c);
+      if (!std::ranges::equal(c, test_case.expected))
+        return false;
+    }
+    {
+      Container c(test_case.initial.begin(), test_case.initial.end());
+      auto in  = wrap_input_decay<Iter, Sent>(test_case.input);
+      auto pos = get_pos(c, test_case);
+
+      auto result = c.insert_range(pos, in);
+      assert(result == get_pos(c, test_case));
+      validate(c);
+      if (!std::ranges::equal(c, test_case.expected))
+        return false;
+    }
+
+    return true;
   };
 
   { // Empty container.
@@ -469,12 +485,26 @@ constexpr void test_sequence_prepend_range(Validate validate) {
   using T = typename Container::value_type;
 
   auto test = [&](auto& test_case) {
-    Container c(test_case.initial.begin(), test_case.initial.end());
-    auto in = wrap_input<Iter, Sent>(test_case.input);
-
-    c.prepend_range(in);
-    validate(c);
-    return std::ranges::equal(c, test_case.expected);
+    {
+      Container c(test_case.initial.begin(), test_case.initial.end());
+      auto in = wrap_input<Iter, Sent>(test_case.input);
+
+      c.prepend_range(in);
+      validate(c);
+      if (!std::ranges::equal(c, test_case.expected))
+        return false;
+    }
+    {
+      Container c(test_case.initial.begin(), test_case.initial.end());
+      auto in = wrap_input_decay<Iter, Sent>(test_case.input);
+
+      c.prepend_range(in);
+      validate(c);
+      if (!std::ranges::equal(c, test_case.expected))
+        return false;
+    }
+
+    return true;
   };
 
   { // Empty container.
@@ -512,12 +542,26 @@ constexpr void test_sequence_append_range(Validate validate) {
   using T = typename Container::value_type;
 
   auto test = [&](auto& test_case) {
-    Container c(test_case.initial.begin(), test_case.initial.end());
-    auto in = wrap_input<Iter, Sent>(test_case.input);
-
-    c.append_range(in);
-    validate(c);
-    return std::ranges::equal(c, test_case.expected);
+    {
+      Container c(test_case.initial.begin(), test_case.initial.end());
+      auto in = wrap_input<Iter, Sent>(test_case.input);
+
+      c.append_range(in);
+      validate(c);
+      if (!std::ranges::equal(c, test_case.expected))
+        return false;
+    }
+    {
+      Container c(test_case.initial.begin(), test_case.initial.end());
+      auto in = wrap_input_decay<Iter, Sent>(test_case.input);
+
+      c.append_range(in);
+      validate(c);
+      if (!std::ranges::equal(c, test_case.expected))
+        return false;
+    }
+
+    return true;
   };
 
   { // Empty container.
@@ -563,12 +607,26 @@ constexpr void test_sequence_assign_range(Validate validate) {
   auto& input_long_range    = FullContainer_Begin_LongRange<T>.input;
 
   auto test = [&](auto& initial, auto& input) {
-    Container c(initial.begin(), initial.end());
-    auto in = wrap_input<Iter, Sent>(input);
-
-    c.assign_range(in);
-    validate(c);
-    return std::ranges::equal(c, input);
+    {
+      Container c(initial.begin(), initial.end());
+      auto in = wrap_input<Iter, Sent>(input);
+
+      c.assign_range(in);
+      validate(c);
+      if (!std::ranges::equal(c, input))
+        return false;
+    }
+    {
+      Container c(initial.begin(), initial.end());
+      auto in = wrap_input_decay<Iter, Sent>(input);
+
+      c.assign_range(in);
+      validate(c);
+      if (!std::ranges::equal(c, input))
+        return false;
+    }
+
+    return true;
   };
 
   { // Empty container.