[libc++] Use __unwrap_iter_impl for both unwrapping and rewrapping

[philnik]

• Try to fix CI

[philnik]

• Use a tag type for move-only iterators

[philnik]

• Fix CI

[philnik]

• Try to fix CI

[philnik]

• Try to fix CI

[philnik]

• Next try

[philnik]

• Next try

[Mordante]

libcxx/include/__algorithm/move.h

92

Please don't use auto, it doesn't help to make the code easier to understand.

libcxx/include/__algorithm/move_backward.h

15

Why do I need pair.h here. Isn't this exported by unwrap_iter?

libcxx/include/__filesystem/u8path.h

97

As mentioned in D129040 I really like to avoid using pair when it makes the code unclear. Here it's quite hard to understand what __unwrap_iter(...).first is. Having a good name instead of first will help a lot.

[ldionne]

I understand the problem you're solving with move-only iterators and I do think it is worth solving. I'm just not sure how to do it best -- there's something I dislike about the approach taken here, IMO it creates a lot of complexity. I'll take another look and let you know if I can think of another approach.

libcxx/include/__algorithm/copy.h

61

Can you please explain the context around this patch in the commit message? Like you just did verbally during live review.

libcxx/include/__algorithm/unwrap_iter.h

25

Can you please update this documentation?

41

I like that you are grouping rewrap and unwrap into the same struct. However, we should (not necessarily in this patch) change the name from __unwrap_iter_impl to something else that is more accurate, such as __wrapped_iterator_traits (strawman proposal, this isn't perfect either).

Can you please add a TODO?

[philnik]

• Rebased
• Don't try to allow move-only iterators

[philnik]

I've thought about this some more and came to the conclusion that we shouldn't try to make __rewrap_iter move-only-iterator-friendly. The only places we use that is in copy, copy_backward, move and move_backward. I think the best way to handle this is to forward all these algorithms to copy and unwrap and rewrap everything there. This means that we can unwrap iterators better in all these algorithms and we only have to have the complicated code in one place. The way to forward them all would be to use reverse_iterator for the _backward algorithms and move_iterator for the move algorihtms. The correct way to unwrap to a memcpy would then be to check the return type of *iter and based on that check whether is_trivially_copy_constructible or is_trivially_move_constructible is true. (Forwarding to memcpy with a move_iterator passed to copy might actually be a bug currently)

[philnik]

• Try to fix CI

[philnik]

• Next Try

[ldionne]

Forwarding to memcpy with a move_iterator passed to copy might actually be a bug currently

Oof. This is pretty serious, can we please try to address this in priority for LLVM 15? By the way, that's an excellent catch.

libcxx/include/__algorithm/unwrap_iter.h

48

Based on https://eel.is/c++draft/iterator.concept.contiguous and https://eel.is/c++draft/alg.copy#3, it's actually not clear to me that we're allowed to do this, since we're bypassing any potential side effects in the contiguous iterator. I understand that the intent is for implementations to use optimizations on contiguous iterators, however it's not what the spec says, depending on how you read it.

Can you please start a discussion on the LWG reflector asking what the intent is and CC me?

Since this is pre-existing in libc++, I won't block this review on that, but I want to get clarity. If other implementers have consensus on "no we shouldn't be doing this", then we really shouldn't, cause it could be extremely surprising for users. If that's the case, then we should only special-case unwrap_iter for our own libc++ iterators. I hope that's not the case, cause these optimizations are sweet.

libcxx/utils/libcxx/test/params.py

41–42 ↗

(On Diff #443933)

Can you please file a GCC bug with a reproducer? Otherwise we'll be stuck with this forever.

libcxx/include/__algorithm/copy.h

#endif
   const size_t __n = static_cast<size_t>(__last - __first);
   if (__n > 0)
     ::__builtin_memmove(__result, __first, __n * sizeof(_OutValueT));
   return std::make_pair(__first + __n, __result + __n);
 }
 
 template <class _InIter, class _OutIter,
           __enable_if_t<is_same<typename remove_const<__iter_value_type<_InIter> >::type, __iter_value_type<_OutIter> >::value
                       && __is_cpp17_contiguous_iterator<_InIter>::value
                       && __is_cpp17_contiguous_iterator<_OutIter>::value
                       && 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 __first_base = std::__unwrap_iter(__first.base()).first;
+  auto __last_base = std::__unwrap_iter(__last.base()).first;
+  auto __result_un = std::__unwrap_iter(__result.base());
+
+  auto __result_base = __result_un.first;
+
   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));
+  return std::make_pair(__last, reverse_iterator<_OutIter>(std::__rewrap_iter(__result_un.second, __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>
+template <class _InIter, class _Sent, class _OutIter>
 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));
+  auto __first_un  = std::__unwrap_iter(std::move(__first));
+  auto __result_un = std::__unwrap_iter(std::move(__result));
+  auto __ret =
+      std::__copy_impl(std::move(__first_un.first), std::__unwrap_iter(__last).first, std::move(__result_un.first));
+  return std::make_pair(
+      std::__rewrap_iter(__first_un.second, std::move(__ret.first)),
+      std::__rewrap_iter(__result_un.second, std::move(__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/copy_backward.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_BACKWARD_H
 #define _LIBCPP___ALGORITHM_COPY_BACKWARD_H
 
 #include <__algorithm/copy.h>
 #include <__algorithm/unwrap_iter.h>
 #include <__config>
 #include <__iterator/iterator_traits.h>
 #include <__iterator/reverse_iterator.h>
+#include <__utility/move.h>
 #include <cstring>
 #include <type_traits>
 
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
 #  pragma GCC system_header
 #endif
 
 _LIBCPP_BEGIN_NAMESPACE_STD
 
 template <class _Iter1, class _Sent1, class _Iter2>
 inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_AFTER_CXX11
 pair<_Iter1, _Iter2> __copy_backward_impl(_Iter1 __first, _Sent1 __last, _Iter2 __result) {
   auto __ret = std::__copy(reverse_iterator<_Iter1>(__last),
                            reverse_iterator<_Sent1>(__first),
                            reverse_iterator<_Iter2>(__result));
   return pair<_Iter1, _Iter2>(__ret.first.base(), __ret.second.base());
 }
 
 template <class _Iter1, class _Sent1, class _Iter2>
 inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_AFTER_CXX11
 pair<_Iter1, _Iter2> __copy_backward(_Iter1 __first, _Sent1 __last, _Iter2 __result) {
-  auto __ret = std::__copy_backward_impl(std::__unwrap_iter(__first),
-                                         std::__unwrap_iter(__last),
-                                         std::__unwrap_iter(__result));
-  return pair<_Iter1, _Iter2>(std::__rewrap_iter(__first, __ret.first), std::__rewrap_iter(__result, __ret.second));
+  auto __first_un = std::__unwrap_iter(std::move(__first));
+  auto __result_un = std::__unwrap_iter(std::move(__result));
+  auto __ret = std::__copy_backward_impl(__first_un.first,
+                                         std::__unwrap_iter(__last).first,
+                                         __result_un.first);
+  return pair<_Iter1, _Iter2>(
+      std::__rewrap_iter(__first_un.second, __ret.first), std::__rewrap_iter(__result_un.second, __ret.second));
 }
 
 template <class _BidirectionalIterator1, class _BidirectionalIterator2>
 inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_AFTER_CXX17
 _BidirectionalIterator2
 copy_backward(_BidirectionalIterator1 __first, _BidirectionalIterator1 __last, _BidirectionalIterator2 __result) {
   return std::__copy_backward(__first, __last, __result).second;
 }
 
 _LIBCPP_END_NAMESPACE_STD
 
 #endif // _LIBCPP___ALGORITHM_COPY_BACKWARD_H

libcxx/include/__algorithm/move.h

template <class _InIter,
                      && __is_cpp17_contiguous_iterator<_InIter>::value
                      && __is_cpp17_contiguous_iterator<_OutIter>::value
                      && is_trivially_move_assignable<__iter_value_type<_OutIter> >::value, int> = 0>
 inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_AFTER_CXX14
 pair<reverse_iterator<_InIter>, reverse_iterator<_OutIter> >
 __move_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);
+  auto __first_base = std::__unwrap_iter(__first.base()).first;
+  auto __last_base = std::__unwrap_iter(__last.base()).first;
+  auto __result_un = std::__unwrap_iter(__result.base());
+  auto __result_first = __result_un.first - (__first_base - __last_base);
   std::__move_impl(__last_base, __first_base, __result_first);
-  return std::make_pair(__last, reverse_iterator<_OutIter>(std::__rewrap_iter(__result.base(), __result_first)));
+  return std::make_pair(__last, reverse_iterator<_OutIter>(std::__rewrap_iter(__result_un.second, __result_first)));
 }
 
 template <class _InIter, class _Sent, class _OutIter>
 inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_AFTER_CXX11
-__enable_if_t<is_copy_constructible<_InIter>::value
-           && is_copy_constructible<_Sent>::value
-           && is_copy_constructible<_OutIter>::value, pair<_InIter, _OutIter> >
+pair<_InIter, _OutIter>
 __move(_InIter __first, _Sent __last, _OutIter __result) {
-  auto __ret = std::__move_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));
-}
-
-template <class _InIter, class _Sent, class _OutIter>
-inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_AFTER_CXX11
-__enable_if_t<!is_copy_constructible<_InIter>::value
-           || !is_copy_constructible<_Sent>::value
-           || !is_copy_constructible<_OutIter>::value, pair<_InIter, _OutIter> >
-__move(_InIter __first, _Sent __last, _OutIter __result) {
-  return std::__move_impl(std::move(__first), std::move(__last), std::move(__result));
+  auto __first_un = std::__unwrap_iter(std::move(__first));
+  auto __result_un = std::__unwrap_iter(std::move(__result));
+  auto __ret =
+      std::__move_impl(std::move(__first_un.first), std::__unwrap_iter(__last).first, std::move(__result_un.first));
+  return std::make_pair(
+      std::__rewrap_iter(__first_un.second, std::move(__ret.first)),
+      std::__rewrap_iter(__result_un.second, std::move(__ret.second)));
 }
 
 template <class _InputIterator, class _OutputIterator>
 inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_AFTER_CXX17
 _OutputIterator move(_InputIterator __first, _InputIterator __last, _OutputIterator __result) {
   return std::__move(__first, __last, __result).second;
 }
 
 _LIBCPP_END_NAMESPACE_STD
 
 #endif // _LIBCPP___ALGORITHM_MOVE_H

libcxx/include/__algorithm/move_backward.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_MOVE_BACKWARD_H
 #define _LIBCPP___ALGORITHM_MOVE_BACKWARD_H
 
 #include <__algorithm/unwrap_iter.h>
 #include <__config>
 #include <__utility/move.h>
+#include <__utility/pair.h>
 #include <cstring>
 #include <type_traits>
 
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
 #  pragma GCC system_header
 #endif
 
 _LIBCPP_BEGIN_NAMESPACE_STD

 inline _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX17
 _BidirectionalIterator2
 move_backward(_BidirectionalIterator1 __first, _BidirectionalIterator1 __last,
               _BidirectionalIterator2 __result)
 {
     if (__libcpp_is_constant_evaluated()) {
         return _VSTD::__move_backward_constexpr(__first, __last, __result);
     } else {
-        return _VSTD::__rewrap_iter(__result,
-            _VSTD::__move_backward(_VSTD::__unwrap_iter(__first),
-                                   _VSTD::__unwrap_iter(__last),
-                                   _VSTD::__unwrap_iter(__result)));
+        auto __result_un = _VSTD::__unwrap_iter(__result);
+        return std::__rewrap_iter(__result_un.second,
+            _VSTD::__move_backward(_VSTD::__unwrap_iter(__first).first,
+                                   _VSTD::__unwrap_iter(__last).first,
+                                   __result_un.first));
     }
 }
 
 _LIBCPP_END_NAMESPACE_STD
 
 #endif // _LIBCPP___ALGORITHM_MOVE_BACKWARD_H

libcxx/include/__algorithm/sort.h

 template <class _RandomAccessIterator, class _Comp>
 inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_AFTER_CXX17
 void __sort_impl(_RandomAccessIterator __first, _RandomAccessIterator __last, _Comp& __comp) {
   _LIBCPP_DEBUG_RANDOMIZE_RANGE(__first, __last);
   using _Comp_ref = typename __comp_ref_type<_Comp>::type;
   if (__libcpp_is_constant_evaluated()) {
     std::__partial_sort<_Comp_ref>(__first, __last, __last, _Comp_ref(__comp));
   } else {
-    std::__sort<_Comp_ref>(std::__unwrap_iter(__first), std::__unwrap_iter(__last), _Comp_ref(__comp));
+    std::__sort<_Comp_ref>(std::__unwrap_iter(__first).first, std::__unwrap_iter(__last).first, _Comp_ref(__comp));
   }
 }
 
 template <class _RandomAccessIterator, class _Comp>
 inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_AFTER_CXX17
 void sort(_RandomAccessIterator __first, _RandomAccessIterator __last, _Comp __comp) {
   std::__sort_impl(std::move(__first), std::move(__last), __comp);
 }

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 <__utility/pair.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.
 // 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.
 
 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
 
 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(declval<_Iter>()));
+
+  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));
+  }
+
+  static _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR _ToAddressT __unwrap(_Iter __i) _NOEXCEPT {
+    return std::__to_address(__i);
 };
 
 #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);
+template<class _Iter,
+         class _Impl = __unwrap_iter_impl<_Iter>,
+         __enable_if_t<is_copy_constructible<_Iter>::value, int> = 0>
+inline _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX11
+pair<decltype(_Impl::__unwrap(std::declval<_Iter>())), _Iter>
+__unwrap_iter(_Iter __i) _NOEXCEPT {
+  auto __unwrapped = _Impl::__unwrap(__i);
+  return std::make_pair(std::move(__unwrapped), std::move(__i));
 }
 
-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));
-  }
-};
+struct __move_only_iterator_tag {};
 
-template <class _OrigIter>
-struct __rewrap_iter_impl<_OrigIter, _OrigIter> {
-  static _LIBCPP_CONSTEXPR _OrigIter __apply(_OrigIter, _OrigIter __result) {
-    return __result;
-  }
-};
+template<class _Iter,
+         __enable_if_t<!is_copy_constructible<_Iter>::value, int> = 0>
+inline _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR
+pair<_Iter, __move_only_iterator_tag> __unwrap_iter(_Iter __i) _NOEXCEPT {
+  return std::make_pair(std::move(__i), __move_only_iterator_tag());
 
-template<class _OrigIter>
-_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR
-_OrigIter __rewrap_iter(_OrigIter, _OrigIter __result)
-{
-    return __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));
 }
 
-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 _Iter>
+_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR _Iter __rewrap_iter(__move_only_iterator_tag, _Iter __iter) _NOEXCEPT {
+  return __iter;
 }
 
 _LIBCPP_END_NAMESPACE_STD
 
 #endif // _LIBCPP___ALGORITHM_UNWRAP_ITER_H

libcxx/include/__filesystem/u8path.h

 #ifndef _LIBCPP_HAS_NO_CHAR8_T
       is_same<typename __is_pathable<_Source>::__char_type, char8_t>::value ||
 #endif
       is_same<typename __is_pathable<_Source>::__char_type, char>::value,
       "u8path(Source const&) requires Source have a character type of type "
       "'char' or 'char8_t'");
 #if defined(_LIBCPP_WIN32API)
   using _Traits = __is_pathable<_Source>;
-  return u8path(_VSTD::__unwrap_iter(_Traits::__range_begin(__s)), _VSTD::__unwrap_iter(_Traits::__range_end(__s)));
-#else
+  return u8path(
+      _VSTD::__unwrap_iter(_Traits::__range_begin(__s)).first, _VSTD::__unwrap_iter(_Traits::__range_end(__s)).first);
+#  else
   return path(__s);
-#endif
+#  endif
 }
 
 _LIBCPP_AVAILABILITY_FILESYSTEM_POP
 
 _LIBCPP_END_NAMESPACE_FILESYSTEM
 
 #endif // _LIBCPP_CXX03_LANG
 
 #endif // _LIBCPP___FILESYSTEM_U8PATH_H

libcxx/include/__format/buffer.h

 
 public:
   _LIBCPP_HIDE_FROM_ABI explicit __format_buffer(_OutIt __out_it)
     requires(same_as<_Storage, __internal_storage<_CharT>>)
   : __output_(__storage_.begin(), __storage_.__buffer_size, this), __writer_(_VSTD::move(__out_it)) {}
 
   _LIBCPP_HIDE_FROM_ABI explicit __format_buffer(_OutIt __out_it) requires(
       same_as<_Storage, __direct_storage<_CharT>>)
-      : __output_(_VSTD::__unwrap_iter(__out_it), size_t(-1), this),
+      : __output_(std::__unwrap_iter(__out_it).first, size_t(-1), this),
         __writer_(_VSTD::move(__out_it)) {}
 
   _LIBCPP_HIDE_FROM_ABI auto make_output_iterator() {
     return __output_.make_output_iterator();
   }
 
   _LIBCPP_HIDE_FROM_ABI void flush(_CharT* __ptr, size_t __size) {
     __writer_.flush(__ptr, __size);

 /// exceed the maximum number of code units.
 template <class _OutIt, __formatter::__char_type _CharT>
   requires(output_iterator<_OutIt, const _CharT&>)
 class _LIBCPP_TEMPLATE_VIS __format_to_n_buffer_base<_OutIt, _CharT, true> {
   using _Size = iter_difference_t<_OutIt>;
 
 public:
   _LIBCPP_HIDE_FROM_ABI explicit __format_to_n_buffer_base(_OutIt __out_it, _Size __n)
-      : __output_(_VSTD::__unwrap_iter(__out_it), __n, this), __writer_(_VSTD::move(__out_it)) {
+      : __output_(std::__unwrap_iter(__out_it).first, __n, this), __writer_(_VSTD::move(__out_it)) {
     if (__n <= 0) [[unlikely]]
       __output_.reset(__storage_.begin(), __storage_.__buffer_size);
   }
 
   _LIBCPP_HIDE_FROM_ABI void flush(_CharT* __ptr, size_t __size) {
     // A flush to the direct writer happens in two occasions:
     // - The format function has written the maximum number of allowed code
     //   units. At this point it's no longer valid to write to this writer. So

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());
   }
 };
 
 _LIBCPP_END_NAMESPACE_STD
 
 #endif // _LIBCPP___ITERATOR_REVERSE_ITERATOR_H

libcxx/test/libcxx/iterators/unwrap_iter.pass.cpp

 #include <cassert>
 #include <string>
 #include <type_traits>
 
 #include "test_iterators.h"
 #include "test_macros.h"
 
 template <class Iter>
-using UnwrapT = decltype(std::__unwrap_iter(std::declval<Iter>()));
+using UnwrapT = decltype(std::__unwrap_iter(std::declval<Iter>()).first);
 
 template <class Iter>
 using rev_iter = std::reverse_iterator<Iter>;
 
 template <class Iter>
 using rev_rev_iter = rev_iter<rev_iter<Iter> >;
 
 static_assert(std::is_same<UnwrapT<int*>, int*>::value, "");
 static_assert(std::is_same<UnwrapT<std::__wrap_iter<int*> >, int*>::value, "");
 static_assert(std::is_same<UnwrapT<rev_iter<int*> >, std::reverse_iterator<int*> >::value, "");
 static_assert(std::is_same<UnwrapT<rev_rev_iter<int*> >, int*>::value, "");
 static_assert(std::is_same<UnwrapT<rev_rev_iter<std::__wrap_iter<int*> > >, int*>::value, "");
 static_assert(std::is_same<UnwrapT<rev_rev_iter<rev_iter<std::__wrap_iter<int*> > > >, rev_iter<std::__wrap_iter<int*> > >::value, "");
 
 static_assert(std::is_same<UnwrapT<random_access_iterator<int*> >, random_access_iterator<int*> >::value, "");
 static_assert(std::is_same<UnwrapT<rev_iter<random_access_iterator<int*> > >, rev_iter<random_access_iterator<int*> > >::value, "");
 static_assert(std::is_same<UnwrapT<rev_rev_iter<random_access_iterator<int*> > >, random_access_iterator<int*> >::value, "");
 static_assert(std::is_same<UnwrapT<rev_rev_iter<rev_iter<random_access_iterator<int*> > > >, rev_iter<random_access_iterator<int*> > >::value, "");
 
 TEST_CONSTEXPR_CXX20 bool test() {
   std::string str = "Banane";
   using Iter = std::string::iterator;
 
-  assert(std::__unwrap_iter(str.begin()) == str.data());
-  assert(std::__unwrap_iter(str.end()) == str.data() + str.size());
-  assert(std::__unwrap_iter(rev_rev_iter<Iter>(rev_iter<Iter>(str.begin()))) == str.data());
-  assert(std::__unwrap_iter(rev_rev_iter<Iter>(rev_iter<Iter>(str.end()))) == str.data() + str.size());
+  assert(std::__unwrap_iter(str.begin()).first == str.data());
+  assert(std::__unwrap_iter(str.end()).first == str.data() + str.size());
+  assert(std::__unwrap_iter(rev_rev_iter<Iter>(rev_iter<Iter>(str.begin()))).first == str.data());
+  assert(std::__unwrap_iter(rev_rev_iter<Iter>(rev_iter<Iter>(str.end()))).first == str.data() + str.size());
 
   return true;
 }
 
 int main(int, char**) {
   test();
 #if TEST_STD_VER > 17
   static_assert(test());
 #endif
 
   return 0;
 }