[libc++] Fix unwrapping ranges with different iterators and sentinels

[huixie90]

libcxx/include/__algorithm/copy.h

91–96

is it ok pass nullptr to __rewrap_iter. The implementation below seems that __first_un.second can be nullptr

libcxx/include/__algorithm/unwrap_range.h

15

unused?

33

random_access_iterator implies copy_constructible
(forward + are copyable)

35

not sure about the variable name __sent. because there is a template parameter called _Sent, at the first glance, I thought __sent is of type _Sent because of the name. but actually, it is of type _Iter

40

I'd be really cautious about nullptr, which is comparable with pointers. if any of the logic goes wrong, it could result in a runtime UB. Is it possible to avoid nullptr?

[philnik]

• Address comments

[philnik]

libcxx/include/__algorithm/unwrap_range.h

40

How exactly would anything go wrong at runtime? This results in pair<pair<_Iter, nullptr_t>, _Sent>. The nullptr_t should only ever be passed back to __rewrap_iter. If that weren't the case there would be type mismatches all over the place. (i.e. __rewrap_iter would just return the unwrapped pointer) But anyways, just to make it really hard to break I'll change it to a tag type, since there isn't any downside to do so.

[philnik]

Upload correct diff

[Mordante]

I really would like to encourage you to add a bit more comments to new code. Historically we haven't done that a lot, but IMO that makes some older parts of the code harder to understand.

libcxx/include/__algorithm/copy.h

13

Please also add this to the top level algorithm header.

72

Can we use fewer autos here. It's quite hard to understand the types without looking at other places.

74

__last has been moved from at line 70. It there a test that fails, if not please add a test.

libcxx/include/__algorithm/unwrap_range.h

31

Please don't use the automatically deduced auto.

35

Based on the code in __algorithm/copy.h with all .first.first and the like. I would prefer to use a small struct with good names for the types instead of a pair. I think that will aid the readability of the code in __algorithm/copy.h.

[huixie90]

LGTM.

Regarding testing, I am wondering if the tests cover those cases:
iterators that model c++20's random_access_range but only model c++17's InputIterator (this is very common though)
iterators that model c++17's RandomAccessIterator but only model C++20's input_or_output_iterator (or input_iterator) (this is not very common and I guess we need to define member typedef iterator_category and iterator_concept to make one)

Since the tests in D128611 and D128983 depend on this fix and currently commented with TODO. Depending the order of which patch land first, we need to update those tests

libcxx/include/__algorithm/unwrap_range.h

40

yeah I agree changing it to a tag type is better, at least more obvious to the readers. It is unclear to me what __rewrap_iter would do for nullptr. Since it is a pair, I would guess it might compare the first with second (since pointers are comparable with nullptr_t). With the tag type it is obvious that it is not going to be compared with first

[philnik]

libcxx/include/__algorithm/copy.h

13

Why should we do that? It's an implementation detail, and adding the include to the top level header just exposes that implementation detail (even for modules users).

74

See D129044.

libcxx/include/__algorithm/unwrap_range.h

31

I can see you point about readability in general, but in this case I don't think it helps in any way. It would essentially be pair<decltype(std::__unwrap_iter(std::move(std::declval<_Iter>()))), decltype(std::__unwrap_iter(std::move(std::declval<_Iter>())).first)> which is just a really long way to write "Find it out yourself". (The std::moves might be unnecessary, but removing them wouldn't make it any better)

35

I've got

template <class _OrigIter, class _Iter>
struct _UnwrappedRange {
  _OrigIter __rewrap;
  _Iter __first;
  _Iter __last;
};

right now, but that loses the information that you can only rewrap __first. That might be acceptable, Idk. I also really dislike __rewrap. WDYT?

[Mordante]

libcxx/include/__algorithm/copy.h

13

Are other header not allowed to use this? unwrap_iter is also in <algorithm> so IMO this should be there too.

libcxx/include/__algorithm/unwrap_range.h

31

I agree that's also not nice to read. But I think we should give some thought about whether we can do better. I feel this entire unwrapping looks quite hard to understand. The original unwrap code already took some effort to understand, but I feel it gets worse with ranges.

I will give it some more thought.

35

Maybe rename __rewrap to __base?
When it's not possible to rewrap __last we can add some comment why it's not possible.

[philnik]

Rebased and updated to new style

[ldionne]

Please ping me on Discord once you have implemented all comments just so I can do another quick pass, and we can ship this in time for LLVM 15.

libcxx/.clang-format

71

Shouldn't be here.

libcxx/include/__algorithm/unwrap_range.h

45

std::move?

48

This would avoid a copy?

53

Please leave at least some sort of documentation explaining what this file and group of functions do. And in particular, it would be useful to explain why they are useful/needed.

59

Would it make sense to std::move here?

74

I had completely missed this because it's a large-ish block.

77

std::move

82

std::move

libcxx/include/__iterator/reverse_iterator.h

335–339

I think this diff doesn't belong here.

[philnik]

• Address comments

[ldionne]

libcxx/include/__algorithm/unwrap_range.h

29

[philnik]

• Address comments

libcxx/include/__algorithm/copy.h

13

Other headers are allowed to use this, but they should include __algorithm/unwrap_range.h and not algorithm.

libcxx/include/__algorithm/unwrap_range.h

48

It turns out that this causes the overloads to be ambiguous. I'll try to improve it later, since this code probably still needs sum debugging.

[philnik]

• Try to fix CI

[philnik]

• Try to fix CI

[philnik]

• Next try

libcxx/.clang-format

 IndentWrappedFunctionNames: false
 IndentRequires: true
 InsertTrailingCommas: Wrapped
 KeepEmptyLinesAtTheStartOfBlocks: false
 MaxEmptyLinesToKeep: 1
 NamespaceIndentation: Inner
 PackConstructorInitializers: NextLine
 
-PenaltyIndentedWhitespace: 61
+PenaltyIndentedWhitespace: 2
 
 Language: Cpp
 Standard: c++20
 SpacesInAngles: Leave
 
 AlwaysBreakTemplateDeclarations: true
 PointerAlignment: Left
 

libcxx/include/CMakeLists.txt

set(files
   __algorithm/sort_heap.h
   __algorithm/stable_partition.h
   __algorithm/stable_sort.h
   __algorithm/swap_ranges.h
   __algorithm/transform.h
   __algorithm/unique.h
   __algorithm/unique_copy.h
   __algorithm/unwrap_iter.h
+  __algorithm/unwrap_range.h
   __algorithm/upper_bound.h
   __assert
   __availability
   __bit/bit_cast.h
   __bit/byteswap.h
   __bit_reference
   __bits
   __bsd_locale_defaults.h

libcxx/include/__algorithm/copy.h

 //===----------------------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef _LIBCPP___ALGORITHM_COPY_H
 #define _LIBCPP___ALGORITHM_COPY_H
 
 #include <__algorithm/unwrap_iter.h>
+#include <__algorithm/unwrap_range.h>
 #include <__config>
 #include <__iterator/iterator_traits.h>
 #include <__iterator/reverse_iterator.h>
 #include <__utility/move.h>
 #include <__utility/pair.h>
 #include <cstring>
 #include <type_traits>
 
template <class _InIter, class _OutIter,
                       && is_trivially_copy_assignable<__iter_value_type<_OutIter> >::value, int> = 0>
 inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_AFTER_CXX11
 pair<reverse_iterator<_InIter>, reverse_iterator<_OutIter> >
 __copy_impl(reverse_iterator<_InIter> __first,
             reverse_iterator<_InIter> __last,
             reverse_iterator<_OutIter> __result) {
   auto __first_base = std::__unwrap_iter(__first.base());
   auto __last_base = std::__unwrap_iter(__last.base());
   auto __result_base = std::__unwrap_iter(__result.base());
   auto __result_first = __result_base - (__first_base - __last_base);
   std::__copy_impl(__last_base, __first_base, __result_first);
   return std::make_pair(__last, reverse_iterator<_OutIter>(std::__rewrap_iter(__result.base(), __result_first)));
 }
 
 template <class _InIter, class _Sent, class _OutIter,
           __enable_if_t<!(is_copy_constructible<_InIter>::value
                        && is_copy_constructible<_Sent>::value
                        && is_copy_constructible<_OutIter>::value), int> = 0 >
 inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_AFTER_CXX11
 pair<_InIter, _OutIter> __copy(_InIter __first, _Sent __last, _OutIter __result) {
   return std::__copy_impl(std::move(__first), std::move(__last), std::move(__result));
 }
 
 template <class _InIter, class _Sent, class _OutIter,
           __enable_if_t<is_copy_constructible<_InIter>::value
                      && is_copy_constructible<_Sent>::value
                      && is_copy_constructible<_OutIter>::value, int> = 0>
 inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_AFTER_CXX11
-pair<_InIter, _OutIter>
-__copy(_InIter __first, _Sent __last, _OutIter __result) {
-  auto __ret = std::__copy_impl(std::__unwrap_iter(__first), std::__unwrap_iter(__last), std::__unwrap_iter(__result));
-  return std::make_pair(std::__rewrap_iter(__first, __ret.first), std::__rewrap_iter(__result, __ret.second));
+pair<_InIter, _OutIter> __copy(_InIter __first, _Sent __last, _OutIter __result) {
+  auto __range = std::__unwrap_range(__first, __last);
+  auto __ret   = std::__copy_impl(std::move(__range.first), std::move(__range.second), std::__unwrap_iter(__result));
+  return std::make_pair(
+      std::__rewrap_range<_Sent>(__first, __ret.first), std::__rewrap_iter(__result, __ret.second));
 }
 
 template <class _InputIterator, class _OutputIterator>
 inline _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX17
 _OutputIterator
 copy(_InputIterator __first, _InputIterator __last, _OutputIterator __result) {
   return std::__copy(__first, __last, __result).second;
 }
 
 _LIBCPP_END_NAMESPACE_STD
 
 #endif // _LIBCPP___ALGORITHM_COPY_H

libcxx/include/__algorithm/set_intersection.h

 //===----------------------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef _LIBCPP___ALGORITHM_SET_INTERSECTION_H
 #define _LIBCPP___ALGORITHM_SET_INTERSECTION_H
 
 #include <__algorithm/comp.h>
 #include <__algorithm/comp_ref_type.h>
 #include <__algorithm/iterator_operations.h>
 #include <__config>
 #include <__iterator/iterator_traits.h>
+#include <__iterator/next.h>
 #include <__utility/move.h>
 
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
 #  pragma GCC system_header
 #endif
 
 _LIBCPP_BEGIN_NAMESPACE_STD
 

libcxx/include/__algorithm/unwrap_iter.h

 //===----------------------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef _LIBCPP___ALGORITHM_UNWRAP_ITER_H
 #define _LIBCPP___ALGORITHM_UNWRAP_ITER_H
 
 #include <__config>
 #include <__iterator/iterator_traits.h>
 #include <__memory/pointer_traits.h>
+#include <__utility/move.h>
 #include <type_traits>
 
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
 #  pragma GCC system_header
 #endif
 
 _LIBCPP_BEGIN_NAMESPACE_STD
 
-// The job of __unwrap_iter is to lower contiguous iterators (such as
-// vector<T>::iterator) into pointers, to reduce the number of template
-// instantiations and to enable pointer-based optimizations e.g. in std::copy.
-// For iterators that are not contiguous, it must be a no-op.
+// TODO: Change the name of __unwrap_iter_impl to something more appropriate
+// The job of __unwrap_iter is to remove iterator wrappers (like reverse_iterator or __wrap_iter),
+// to reduce the number of template instantiations and to enable pointer-based optimizations e.g. in std::copy.
 // In debug mode, we don't do this.
 //
-// __unwrap_iter is non-constexpr for user-defined iterators whose
-// `to_address` and/or `operator->` is non-constexpr. This is okay; but we
-// try to avoid doing __unwrap_iter in constant-evaluated contexts anyway.
-//
 // Some algorithms (e.g. std::copy, but not std::sort) need to convert an
-// "unwrapped" result back into a contiguous iterator. Since contiguous iterators
-// are random-access, we can do this portably using iterator arithmetic; this
-// is the job of __rewrap_iter.
+// "unwrapped" result back into the original iterator type. Doing that is the job of __rewrap_iter.
 
+// Default case - we can't unwrap anything
 template <class _Iter, bool = __is_cpp17_contiguous_iterator<_Iter>::value>
 struct __unwrap_iter_impl {
-    static _LIBCPP_CONSTEXPR _Iter
-    __apply(_Iter __i) _NOEXCEPT {
-        return __i;
-    }
+  static _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR _Iter __rewrap(_Iter, _Iter __iter) { return __iter; }
+  static _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR _Iter __unwrap(_Iter __i) _NOEXCEPT { return __i; }
 };
 
 #ifndef _LIBCPP_ENABLE_DEBUG_MODE
 
+// It's a contiguous iterator, so we can use a raw pointer instead
 template <class _Iter>
 struct __unwrap_iter_impl<_Iter, true> {
-    static _LIBCPP_CONSTEXPR decltype(_VSTD::__to_address(declval<_Iter>()))
-    __apply(_Iter __i) _NOEXCEPT {
-        return _VSTD::__to_address(__i);
-    }
-};
+  using _ToAddressT = decltype(std::__to_address(std::declval<_Iter>()));
 
-#endif // !_LIBCPP_ENABLE_DEBUG_MODE
-
-template<class _Iter, class _Impl = __unwrap_iter_impl<_Iter> >
-inline _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR
-decltype(_Impl::__apply(declval<_Iter>()))
-__unwrap_iter(_Iter __i) _NOEXCEPT
-{
-    return _Impl::__apply(__i);
+  static _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR _Iter __rewrap(_Iter __orig_iter, _ToAddressT __unwrapped_iter) {
+    return __orig_iter + (__unwrapped_iter - std::__to_address(__orig_iter));
   }
 
-template <class _OrigIter, class _UnwrappedIter>
-struct __rewrap_iter_impl {
-  static _LIBCPP_CONSTEXPR _OrigIter __apply(_OrigIter __first, _UnwrappedIter __result) {
-    // Precondition: __result is reachable from __first
-    // Precondition: _OrigIter is a contiguous iterator
-    return __first + (__result - std::__unwrap_iter(__first));
+  static _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR _ToAddressT __unwrap(_Iter __i) _NOEXCEPT {
+    return std::__to_address(__i);
   }
 };
 
-template <class _OrigIter>
-struct __rewrap_iter_impl<_OrigIter, _OrigIter> {
-  static _LIBCPP_CONSTEXPR _OrigIter __apply(_OrigIter, _OrigIter __result) {
-    return __result;
-  }
-};
+#endif // !_LIBCPP_ENABLE_DEBUG_MODE
 
-template<class _OrigIter>
-_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR
-_OrigIter __rewrap_iter(_OrigIter, _OrigIter __result)
-{
-    return __result;
+template<class _Iter,
+         class _Impl = __unwrap_iter_impl<_Iter>,
+         __enable_if_t<is_copy_constructible<_Iter>::value, int> = 0>
+inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_AFTER_CXX11
+decltype(_Impl::__unwrap(std::declval<_Iter>())) __unwrap_iter(_Iter __i) _NOEXCEPT {
+  return _Impl::__unwrap(__i);
 }
 
-template<class _OrigIter, class _UnwrappedIter, class _Impl = __rewrap_iter_impl<_OrigIter, _UnwrappedIter> >
-_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR
-_OrigIter __rewrap_iter(_OrigIter __first, _UnwrappedIter __result)
-{
-  return _Impl::__apply(__first, __result);
+template <class _OrigIter, class _Iter, class _Impl = __unwrap_iter_impl<_OrigIter> >
+_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR _OrigIter __rewrap_iter(_OrigIter __orig_iter, _Iter __iter) _NOEXCEPT {
+  return _Impl::__rewrap(std::move(__orig_iter), std::move(__iter));
 }
 
 _LIBCPP_END_NAMESPACE_STD
 
 #endif // _LIBCPP___ALGORITHM_UNWRAP_ITER_H

libcxx/include/__algorithm/unwrap_range.h

+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef _LIBCPP___ALGORITHM_UNWRAP_RANGE_H
+#define _LIBCPP___ALGORITHM_UNWRAP_RANGE_H
+
+#include <__algorithm/unwrap_iter.h>
+#include <__concepts/constructible.h>
+#include <__config>
+#include <__iterator/concepts.h>
+#include <__iterator/next.h>
+#include <__utility/declval.h>
+#include <__utility/move.h>
+#include <__utility/pair.h>
+
+#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
+#  pragma GCC system_header
+#endif
+
+_LIBCPP_BEGIN_NAMESPACE_STD
+
+#if _LIBCPP_STD_VER > 17
+template <class _Iter, class _Sent>
+struct __unwrap_range_impl {
+  _LIBCPP_HIDE_FROM_ABI static constexpr auto __unwrap(_Iter __first, _Sent __sent)
+    requires random_access_iterator<_Iter> && sized_sentinel_for<_Sent, _Iter>
+  {
+    auto __last = ranges::next(__first, __sent);
+    return pair{std::__unwrap_iter(std::move(__first)), std::__unwrap_iter(std::move(__last))};
+  }
+
+  _LIBCPP_HIDE_FROM_ABI static constexpr auto __unwrap(_Iter __first, _Sent __last) {
+    return pair{std::move(__first), std::move(__last)};
+  }
+
+  _LIBCPP_HIDE_FROM_ABI static constexpr auto
+  __rewrap(_Iter __orig_iter, decltype(std::__unwrap_iter(__orig_iter)) __iter)
+    requires random_access_iterator<_Iter> && sized_sentinel_for<_Sent, _Iter>
+  {
+    return std::__rewrap_iter(__orig_iter, __iter);
+  }
+
+  _LIBCPP_HIDE_FROM_ABI static constexpr auto __rewrap(_Iter, _Iter __iter) { return __iter; }
+};
+
+template <class _Iter>
+struct __unwrap_range_impl<_Iter, _Iter> {
+  _LIBCPP_HIDE_FROM_ABI static constexpr auto __unwrap(_Iter __first, _Iter __last) {
+    return pair{std::__unwrap_iter(std::move(__first)), std::__unwrap_iter(std::move(__last))};
+  }
+
+  _LIBCPP_HIDE_FROM_ABI static constexpr auto
+  __rewrap(_Iter __orig_iter, decltype(std::__unwrap_iter(__orig_iter)) __iter) {
+    return std::__rewrap_iter(__orig_iter, __iter);
+  }
+};
+
+template <class _Iter, class _Sent>
+_LIBCPP_HIDE_FROM_ABI constexpr auto __unwrap_range(_Iter __first, _Sent __last) {
+  return __unwrap_range_impl<_Iter, _Sent>::__unwrap(std::move(__first), std::move(__last));
+}
+
+template <class _Sent,
+          class _Iter,
+          class _Unwrapped = decltype(std::__unwrap_range(std::declval<_Iter>(), std::declval<_Sent>()))>
+_LIBCPP_HIDE_FROM_ABI constexpr _Iter __rewrap_range(_Iter __orig_iter, _Unwrapped __iter) {
+  return __unwrap_range_impl<_Iter, _Sent>::__rewrap(std::move(__orig_iter), std::move(__iter));
+}
+#else
+template <class _Iter, class _Unwrapped = decltype(std::__unwrap_iter(std::declval<_Iter>()))>
+_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR pair<_Unwrapped, _Unwrapped> __unwrap_range(_Iter __first, _Iter __last) {
+  return std::make_pair(std::__unwrap_iter(__first), std::__unwrap_iter(__last));
+}
+
+template <class _Iter, class _Unwrapped = decltype(std::__unwrap_iter(std::declval<_Iter>()))>
+_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR _Iter __rewrap_range(_Iter __orig_iter, _Unwrapped __iter) {
+  return std::__rewrap_iter(__orig_iter, __iter);
+}
+#endif
+
+_LIBCPP_END_NAMESPACE_STD
+
+#endif // _LIBCPP___ALGORITHM_UNWRAP_RANGE_H

libcxx/include/__iterator/reverse_iterator.h

 }
 #endif
 
 template <class _Iter>
 using _ReverseWrapper = reverse_iterator<reverse_iterator<_Iter> >;
 
 template <class _Iter, bool __b>
 struct __unwrap_iter_impl<_ReverseWrapper<_Iter>, __b> {
-  static _LIBCPP_CONSTEXPR decltype(std::__unwrap_iter(std::declval<_Iter>()))
-  __apply(_ReverseWrapper<_Iter> __i) _NOEXCEPT {
-    return std::__unwrap_iter(__i.base().base());
-  }
-};
-
-template <class _OrigIter, class _UnwrappedIter>
-struct __rewrap_iter_impl<_ReverseWrapper<_OrigIter>, _UnwrappedIter> {
-  template <class _Iter>
-  struct _ReverseWrapperCount {
-    static _LIBCPP_CONSTEXPR const size_t value = 1;
-  };
+  using _UnwrappedIter = decltype(__unwrap_iter_impl<_Iter>::__unwrap(std::declval<_Iter>()));
 
-  template <class _Iter>
-  struct _ReverseWrapperCount<_ReverseWrapper<_Iter> > {
-    static _LIBCPP_CONSTEXPR const size_t value = 1 + _ReverseWrapperCount<_Iter>::value;
-  };
+  static _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR _ReverseWrapper<_Iter>
+  __rewrap(_ReverseWrapper<_Iter> __orig_iter, _UnwrappedIter __unwrapped_iter) {
+    return _ReverseWrapper<_Iter>(
+        reverse_iterator<_Iter>(__unwrap_iter_impl<_Iter>::__rewrap(__orig_iter.base().base(), __unwrapped_iter)));
+  }
 
-  template <size_t _RewrapCount, class _OIter, class _UIter, __enable_if_t<_RewrapCount != 0, int> = 0>
-  _LIBCPP_HIDE_FROM_ABI static _LIBCPP_CONSTEXPR _ReverseWrapper<_OIter> __rewrap(_ReverseWrapper<_OIter> __iter1,
-                                                                                  _UIter __iter2) {
-    return _ReverseWrapper<_OIter>(
-        reverse_iterator<_OIter>(__rewrap<_RewrapCount - 1>(__iter1.base().base(), __iter2)));
-  }
-
-  template <size_t _RewrapCount, class _OIter, class _UIter, __enable_if_t<_RewrapCount == 0, int> = 0>
-  _LIBCPP_HIDE_FROM_ABI static _LIBCPP_CONSTEXPR decltype(std::__rewrap_iter(std::declval<_OIter>(),
-                                                                             std::declval<_UIter>()))
-  __rewrap(_OIter __iter1, _UIter __iter2) {
-    return std::__rewrap_iter(__iter1, __iter2);
-  }
-
-  _LIBCPP_HIDE_FROM_ABI static _LIBCPP_CONSTEXPR _ReverseWrapper<_OrigIter> __apply(_ReverseWrapper<_OrigIter> __iter1,
-                                                                                    _UnwrappedIter __iter2) {
-    return __rewrap<_ReverseWrapperCount<_OrigIter>::value>(__iter1, __iter2);
+  static _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR _UnwrappedIter __unwrap(_ReverseWrapper<_Iter> __i) _NOEXCEPT {
+    return __unwrap_iter_impl<_Iter>::__unwrap(__i.base().base());
   }
 };
 
 #if _LIBCPP_STD_VER > 17 && !defined(_LIBCPP_HAS_NO_INCOMPLETE_RANGES)
 template <ranges::bidirectional_range _Range>
 _LIBCPP_HIDE_FROM_ABI constexpr ranges::
     subrange<reverse_iterator<ranges::iterator_t<_Range>>, reverse_iterator<ranges::iterator_t<_Range>>>
     __reverse_range(_Range&& __range) {
   auto __first = ranges::begin(__range);
   return {std::make_reverse_iterator(ranges::next(__first, ranges::end(__range))), std::make_reverse_iterator(__first)};
 }
 #endif
 
 _LIBCPP_END_NAMESPACE_STD
 
 #endif // _LIBCPP___ITERATOR_REVERSE_ITERATOR_H

libcxx/test/libcxx/private_headers.verify.cpp

 #include <__algorithm/sort_heap.h> // expected-error@*:* {{use of private header from outside its module: '__algorithm/sort_heap.h'}}
 #include <__algorithm/stable_partition.h> // expected-error@*:* {{use of private header from outside its module: '__algorithm/stable_partition.h'}}
 #include <__algorithm/stable_sort.h> // expected-error@*:* {{use of private header from outside its module: '__algorithm/stable_sort.h'}}
 #include <__algorithm/swap_ranges.h> // expected-error@*:* {{use of private header from outside its module: '__algorithm/swap_ranges.h'}}
 #include <__algorithm/transform.h> // expected-error@*:* {{use of private header from outside its module: '__algorithm/transform.h'}}
 #include <__algorithm/unique.h> // expected-error@*:* {{use of private header from outside its module: '__algorithm/unique.h'}}
 #include <__algorithm/unique_copy.h> // expected-error@*:* {{use of private header from outside its module: '__algorithm/unique_copy.h'}}
 #include <__algorithm/unwrap_iter.h> // expected-error@*:* {{use of private header from outside its module: '__algorithm/unwrap_iter.h'}}
+#include <__algorithm/unwrap_range.h> // expected-error@*:* {{use of private header from outside its module: '__algorithm/unwrap_range.h'}}
 #include <__algorithm/upper_bound.h> // expected-error@*:* {{use of private header from outside its module: '__algorithm/upper_bound.h'}}
 #include <__availability> // expected-error@*:* {{use of private header from outside its module: '__availability'}}
 #include <__bit/bit_cast.h> // expected-error@*:* {{use of private header from outside its module: '__bit/bit_cast.h'}}
 #include <__bit/byteswap.h> // expected-error@*:* {{use of private header from outside its module: '__bit/byteswap.h'}}
 #include <__bit_reference> // expected-error@*:* {{use of private header from outside its module: '__bit_reference'}}
 #include <__bits> // expected-error@*:* {{use of private header from outside its module: '__bits'}}
 #include <__charconv/chars_format.h> // expected-error@*:* {{use of private header from outside its module: '__charconv/chars_format.h'}}
 #include <__charconv/from_chars_result.h> // expected-error@*:* {{use of private header from outside its module: '__charconv/from_chars_result.h'}}

libcxx/test/std/algorithms/alg.modifying.operations/alg.copy/ranges.copy.pass.cpp

constexpr void test_iterators() {
       auto range = std::ranges::subrange(In(in.data()), Sent(In(in.data() + in.size())));
       auto ret = std::ranges::copy(range, Out(out.data()));
       assert(base(ret.in) == in.data());
       assert(base(ret.out) == out.data());
     }
   }
 }
 
+template <class In, class Out>
+constexpr void test_sentinels() {
+  test_iterators<In, Out>();
+  test_iterators<In, Out, sized_sentinel<In>>();
+  test_iterators<In, Out, sentinel_wrapper<In>>();
+}
+
 template <class Out>
 constexpr void test_in_iterators() {
   test_iterators<cpp20_input_iterator<int*>, Out, sentinel_wrapper<cpp20_input_iterator<int*>>>();
-  test_iterators<forward_iterator<int*>, Out>();
-  test_iterators<bidirectional_iterator<int*>, Out>();
-  test_iterators<random_access_iterator<int*>, Out>();
-  test_iterators<contiguous_iterator<int*>, Out>();
+  test_sentinels<forward_iterator<int*>, Out>();
+  test_sentinels<bidirectional_iterator<int*>, Out>();
+  test_sentinels<random_access_iterator<int*>, Out>();
+  test_sentinels<contiguous_iterator<int*>, Out>();
 }
 
 template <class Out>
 constexpr void test_proxy_in_iterators() {
   test_iterators<ProxyIterator<cpp20_input_iterator<int*>>, Out, sentinel_wrapper<ProxyIterator<cpp20_input_iterator<int*>>>>();
   test_iterators<ProxyIterator<forward_iterator<int*>>, Out>();
   test_iterators<ProxyIterator<bidirectional_iterator<int*>>, Out>();
   test_iterators<ProxyIterator<random_access_iterator<int*>>, Out>();

libcxx/utils/libcxx/test/params.py

_warningFlags = [
   '-Wno-literal-suffix', # GCC
   '-Wno-user-defined-literals', # Clang
 
   # GCC warns about this when TEST_IS_CONSTANT_EVALUATED is used on a non-constexpr
   # function. (This mostely happens in C++11 mode.)
   # TODO(mordante) investigate a solution for this issue.
   '-Wno-tautological-compare',
 
+  # -Wstringop-overread seems to be a bit buggy currently
+  '-Wno-stringop-overread',
+
   # These warnings should be enabled in order to support the MSVC
   # team using the test suite; They enable the warnings below and
   # expect the test suite to be clean.
   '-Wsign-compare',
   '-Wunused-variable',
   '-Wunused-parameter',
   '-Wunreachable-code',
   '-Wno-unused-local-typedef',