[libc++] NFCI: Remove code duplication and obsolete declarations in wrap_iter

[ldionne]

@Quuxplusone You suggested this cleanup back in D104669 IIRC.

[• Quuxplusone]

If buildkite is happy, I'm happy.

libcxx/include/__iterator/wrap_iter.h

221–227

Could this operator+ become a hidden friend of __wrap_iter?
I'm not asking for all the relational operators to become hidden friends, because they're parameterized on two types, _Iter1 and _Iter2 (and I don't understand the ramifications/rationale of that). But for this operator+, the transformation to hidden friend should be pretty trivial.

[ldionne]

Fix build in C++03 mode.

[ldionne]

Fixed and verified the modular build locally. We were missing an include of __algorithm/move.h in inplace_merge.h.

libcxx/include/__iterator/wrap_iter.h

221–227

I think I'd rather be consistent and have them all be non-member functions.

[bgraur]

Hi Louis,

This patch introduces an error when std::rel_ops are used.
Here's a small reproducer:

#include <utility> // for std::rel_ops.
#include <vector>

using namespace std::rel_ops;

struct T;
void fn() {
  std::vector<T>::const_iterator a;
  for (std::vector<T>::const_iterator i; i >= a;)
    ;
}

The compilation result:

repro.cc:9:44: error: use of overloaded operator '>=' is ambiguous (with operand types 'std::vector<T>::const_iterator' (aka '__wrap_iter<const T *>') and 'std::vector<T>::const_iterator' (aka '__wrap_iter<const T *>'))
  for (std::vector<T>::const_iterator i; i >= a;)
                                         ~ ^  ~
/google/src/cloud/bgraur/cherrypick/google3/third_party/crosstool/v18/llvm_unstable/toolchain/bin/../include/c++/v1/__utility/rel_ops.h:56:1: note: candidate function [with _Tp = std::__wrap_iter<const T *>]
operator>=(const _Tp& __x, const _Tp& __y)
^
/google/src/cloud/bgraur/cherrypick/google3/third_party/crosstool/v18/llvm_unstable/toolchain/bin/../include/c++/v1/__iterator/wrap_iter.h:201:6: note: candidate function [with _Iter1 = const T *, _Iter2 = const T *]
bool operator>=(const __wrap_iter<_Iter1>& __x, const __wrap_iter<_Iter2>& __y) _NOEXCEPT
     ^
1 error generated.

Could you please take a look?

[• Quuxplusone]

Amusing comment in GNU libstdc++ about this.
https://github.com/gcc-mirror/gcc/blob/master/libstdc%2B%2B-v3/include/bits/stl_iterator.h#L1081-L1087

It appears that the state-of-the-art fix is that we need to provide both

template <class _Iter1>
bool operator!=(const __wrap_iter<_Iter1>& __x, const __wrap_iter<_Iter1>& __y) _NOEXCEPT
{
    return !(__x == __y);
}

template <class _Iter1, class _Iter2>
bool operator!=(const __wrap_iter<_Iter1>& __x, const __wrap_iter<_Iter2>& __y) _NOEXCEPT
{
    return !(__x == __y);
}

and so on. The single-_Iter1 versions are precisely the bits @ldionne removed. I hadn't even noticed they were there! We should put them back, plus a test case.

libcxx/include/__algorithm/inplace_merge.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_INPLACE_MERGE_H
 #define _LIBCPP___ALGORITHM_INPLACE_MERGE_H
 
 #include <__config>
-#include <__algorithm/comp.h>
 #include <__algorithm/comp_ref_type.h>
-#include <__algorithm/rotate.h>
+#include <__algorithm/comp.h>
 #include <__algorithm/lower_bound.h>
 #include <__algorithm/min.h>
+#include <__algorithm/move.h>
+#include <__algorithm/rotate.h>
 #include <__algorithm/upper_bound.h>
 #include <__iterator/iterator_traits.h>
 #include <__utility/swap.h>
 #include <memory>
 
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
 #pragma GCC system_header
 #endif

libcxx/include/__iterator/wrap_iter.h

 #pragma GCC system_header
 #endif
 
 _LIBCPP_PUSH_MACROS
 #include <__undef_macros>
 
 _LIBCPP_BEGIN_NAMESPACE_STD
 
-template <class _Iter> class __wrap_iter;
-
-template <class _Iter1, class _Iter2>
-_LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_IF_NODEBUG
-bool
-operator==(const __wrap_iter<_Iter1>&, const __wrap_iter<_Iter2>&) _NOEXCEPT;
-
-template <class _Iter1, class _Iter2>
-_LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_IF_NODEBUG
-bool
-operator<(const __wrap_iter<_Iter1>&, const __wrap_iter<_Iter2>&) _NOEXCEPT;
-
-template <class _Iter1, class _Iter2>
-_LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_IF_NODEBUG
-bool
-operator!=(const __wrap_iter<_Iter1>&, const __wrap_iter<_Iter2>&) _NOEXCEPT;
-
-template <class _Iter1, class _Iter2>
-_LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_IF_NODEBUG
-bool
-operator>(const __wrap_iter<_Iter1>&, const __wrap_iter<_Iter2>&) _NOEXCEPT;
-
-template <class _Iter1, class _Iter2>
-_LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_IF_NODEBUG
-bool
-operator>=(const __wrap_iter<_Iter1>&, const __wrap_iter<_Iter2>&) _NOEXCEPT;
-
-template <class _Iter1, class _Iter2>
-_LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_IF_NODEBUG
-bool
-operator<=(const __wrap_iter<_Iter1>&, const __wrap_iter<_Iter2>&) _NOEXCEPT;
-
-#ifndef _LIBCPP_CXX03_LANG
-template <class _Iter1, class _Iter2>
-_LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_IF_NODEBUG
-auto
-operator-(const __wrap_iter<_Iter1>& __x, const __wrap_iter<_Iter2>& __y) _NOEXCEPT
--> decltype(__x.base() - __y.base());
-#else
-template <class _Iter1, class _Iter2>
-_LIBCPP_INLINE_VISIBILITY
-typename __wrap_iter<_Iter1>::difference_type
-operator-(const __wrap_iter<_Iter1>&, const __wrap_iter<_Iter2>&) _NOEXCEPT;
-#endif
-
-template <class _Iter>
-_LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_IF_NODEBUG
-__wrap_iter<_Iter>
-operator+(typename __wrap_iter<_Iter>::difference_type, __wrap_iter<_Iter>) _NOEXCEPT;
-
-template <class _Ip, class _Op> _Op _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX17 copy(_Ip, _Ip, _Op);
-template <class _B1, class _B2> _B2 _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX17 copy_backward(_B1, _B1, _B2);
-template <class _Ip, class _Op> _Op _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX17 move(_Ip, _Ip, _Op);
-template <class _B1, class _B2> _B2 _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX17 move_backward(_B1, _B1, _B2);
-
 template <class _Iter>
 class __wrap_iter
 {
 public:
     typedef _Iter                                                      iterator_type;
     typedef typename iterator_traits<iterator_type>::value_type        value_type;
     typedef typename iterator_traits<iterator_type>::difference_type   difference_type;
     typedef typename iterator_traits<iterator_type>::pointer           pointer;

 #else
     _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_IF_NODEBUG __wrap_iter(iterator_type __x) _NOEXCEPT : __i(__x) {}
 #endif
 
     template <class _Up> friend class __wrap_iter;
     template <class _CharT, class _Traits, class _Alloc> friend class basic_string;
     template <class _Tp, class _Alloc> friend class _LIBCPP_TEMPLATE_VIS vector;
     template <class _Tp, size_t> friend class _LIBCPP_TEMPLATE_VIS span;
-
-    template <class _Iter1, class _Iter2>
-    _LIBCPP_CONSTEXPR_IF_NODEBUG friend
-    bool
-    operator==(const __wrap_iter<_Iter1>&, const __wrap_iter<_Iter2>&) _NOEXCEPT;
-
-    template <class _Iter1, class _Iter2>
-    _LIBCPP_CONSTEXPR_IF_NODEBUG friend
-    bool
-    operator<(const __wrap_iter<_Iter1>&, const __wrap_iter<_Iter2>&) _NOEXCEPT;
-
-    template <class _Iter1, class _Iter2>
-    _LIBCPP_CONSTEXPR_IF_NODEBUG friend
-    bool
-    operator!=(const __wrap_iter<_Iter1>&, const __wrap_iter<_Iter2>&) _NOEXCEPT;
-
-    template <class _Iter1, class _Iter2>
-    _LIBCPP_CONSTEXPR_IF_NODEBUG friend
-    bool
-    operator>(const __wrap_iter<_Iter1>&, const __wrap_iter<_Iter2>&) _NOEXCEPT;
-
-    template <class _Iter1, class _Iter2>
-    _LIBCPP_CONSTEXPR_IF_NODEBUG friend
-    bool
-    operator>=(const __wrap_iter<_Iter1>&, const __wrap_iter<_Iter2>&) _NOEXCEPT;
-
-    template <class _Iter1, class _Iter2>
-    _LIBCPP_CONSTEXPR_IF_NODEBUG friend
-    bool
-    operator<=(const __wrap_iter<_Iter1>&, const __wrap_iter<_Iter2>&) _NOEXCEPT;
-
-#ifndef _LIBCPP_CXX03_LANG
-    template <class _Iter1, class _Iter2>
-    _LIBCPP_CONSTEXPR_IF_NODEBUG friend
-    auto
-    operator-(const __wrap_iter<_Iter1>& __x, const __wrap_iter<_Iter2>& __y) _NOEXCEPT
-    -> decltype(__x.base() - __y.base());
-#else
-    template <class _Iter1, class _Iter2>
-    _LIBCPP_CONSTEXPR_IF_NODEBUG friend
-    typename __wrap_iter<_Iter1>::difference_type
-    operator-(const __wrap_iter<_Iter1>&, const __wrap_iter<_Iter2>&) _NOEXCEPT;
-#endif
-
-    template <class _Iter1>
-    _LIBCPP_CONSTEXPR_IF_NODEBUG friend
-    __wrap_iter<_Iter1>
-    operator+(typename __wrap_iter<_Iter1>::difference_type, __wrap_iter<_Iter1>) _NOEXCEPT;
 };
 
-#if _LIBCPP_STD_VER <= 17
-template <class _It>
-struct __is_cpp17_contiguous_iterator<__wrap_iter<_It> > : true_type {};
-#endif
-
-template <class _Iter>
-_LIBCPP_CONSTEXPR
-decltype(_VSTD::__to_address(declval<_Iter>()))
-__to_address(__wrap_iter<_Iter> __w) _NOEXCEPT {
-    return _VSTD::__to_address(__w.base());
-}
-
 template <class _Iter1, class _Iter2>
-inline _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_IF_NODEBUG
-bool
-operator==(const __wrap_iter<_Iter1>& __x, const __wrap_iter<_Iter2>& __y) _NOEXCEPT
+_LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_IF_NODEBUG
+bool operator==(const __wrap_iter<_Iter1>& __x, const __wrap_iter<_Iter2>& __y) _NOEXCEPT
 {
     return __x.base() == __y.base();
 }
 
 template <class _Iter1, class _Iter2>
-inline _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_IF_NODEBUG
-bool
-operator<(const __wrap_iter<_Iter1>& __x, const __wrap_iter<_Iter2>& __y) _NOEXCEPT
+_LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_IF_NODEBUG
+bool operator<(const __wrap_iter<_Iter1>& __x, const __wrap_iter<_Iter2>& __y) _NOEXCEPT
 {
 #if _LIBCPP_DEBUG_LEVEL == 2
     _LIBCPP_ASSERT(__get_const_db()->__less_than_comparable(&__x, &__y),
                    "Attempted to compare incomparable iterators");
 #endif
     return __x.base() < __y.base();
 }
 
 template <class _Iter1, class _Iter2>
-inline _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_IF_NODEBUG
-bool
-operator!=(const __wrap_iter<_Iter1>& __x, const __wrap_iter<_Iter2>& __y) _NOEXCEPT
+_LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_IF_NODEBUG
+bool operator!=(const __wrap_iter<_Iter1>& __x, const __wrap_iter<_Iter2>& __y) _NOEXCEPT
 {
     return !(__x == __y);
 }
 
 template <class _Iter1, class _Iter2>
-inline _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_IF_NODEBUG
-bool
-operator>(const __wrap_iter<_Iter1>& __x, const __wrap_iter<_Iter2>& __y) _NOEXCEPT
+_LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_IF_NODEBUG
+bool operator>(const __wrap_iter<_Iter1>& __x, const __wrap_iter<_Iter2>& __y) _NOEXCEPT
 {
     return __y < __x;
 }
 
 template <class _Iter1, class _Iter2>
-inline _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_IF_NODEBUG
-bool
-operator>=(const __wrap_iter<_Iter1>& __x, const __wrap_iter<_Iter2>& __y) _NOEXCEPT
+_LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_IF_NODEBUG
+bool operator>=(const __wrap_iter<_Iter1>& __x, const __wrap_iter<_Iter2>& __y) _NOEXCEPT
 {
     return !(__x < __y);
 }
 
 template <class _Iter1, class _Iter2>
-inline _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_IF_NODEBUG
-bool
-operator<=(const __wrap_iter<_Iter1>& __x, const __wrap_iter<_Iter2>& __y) _NOEXCEPT
-{
-    return !(__y < __x);
-}
-
-template <class _Iter1>
-inline _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_IF_NODEBUG
-bool
-operator!=(const __wrap_iter<_Iter1>& __x, const __wrap_iter<_Iter1>& __y) _NOEXCEPT
-{
-    return !(__x == __y);
-}
-
-template <class _Iter1>
-inline _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_IF_NODEBUG
-bool
-operator>(const __wrap_iter<_Iter1>& __x, const __wrap_iter<_Iter1>& __y) _NOEXCEPT
-{
-    return __y < __x;
-}
-
-template <class _Iter1>
-inline _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_IF_NODEBUG
-bool
-operator>=(const __wrap_iter<_Iter1>& __x, const __wrap_iter<_Iter1>& __y) _NOEXCEPT
-{
-    return !(__x < __y);
-}
-
-template <class _Iter1>
-inline _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_IF_NODEBUG
-bool
-operator<=(const __wrap_iter<_Iter1>& __x, const __wrap_iter<_Iter1>& __y) _NOEXCEPT
+_LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_IF_NODEBUG
+bool operator<=(const __wrap_iter<_Iter1>& __x, const __wrap_iter<_Iter2>& __y) _NOEXCEPT
 {
     return !(__y < __x);
 }
 
-#ifndef _LIBCPP_CXX03_LANG
 template <class _Iter1, class _Iter2>
-inline _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_IF_NODEBUG
-auto
-operator-(const __wrap_iter<_Iter1>& __x, const __wrap_iter<_Iter2>& __y) _NOEXCEPT
+_LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_IF_NODEBUG
+#ifndef _LIBCPP_CXX03_LANG
+auto operator-(const __wrap_iter<_Iter1>& __x, const __wrap_iter<_Iter2>& __y) _NOEXCEPT
     -> decltype(__x.base() - __y.base())
-{
-#if _LIBCPP_DEBUG_LEVEL == 2
-    _LIBCPP_ASSERT(__get_const_db()->__less_than_comparable(&__x, &__y),
-                   "Attempted to subtract incompatible iterators");
-#endif
-    return __x.base() - __y.base();
-}
 #else
-template <class _Iter1, class _Iter2>
-inline _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_IF_NODEBUG
 typename __wrap_iter<_Iter1>::difference_type
 operator-(const __wrap_iter<_Iter1>& __x, const __wrap_iter<_Iter2>& __y) _NOEXCEPT
+#endif // C++03
 {
 #if _LIBCPP_DEBUG_LEVEL == 2
     _LIBCPP_ASSERT(__get_const_db()->__less_than_comparable(&__x, &__y),
                    "Attempted to subtract incompatible iterators");
 #endif
     return __x.base() - __y.base();
 }
-#endif
 
-template <class _Iter>
-inline _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_IF_NODEBUG
-__wrap_iter<_Iter>
-operator+(typename __wrap_iter<_Iter>::difference_type __n,
-          __wrap_iter<_Iter> __x) _NOEXCEPT
+template <class _Iter1>
+_LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_IF_NODEBUG
+__wrap_iter<_Iter1> operator+(typename __wrap_iter<_Iter1>::difference_type __n, __wrap_iter<_Iter1> __x) _NOEXCEPT
 {
     __x += __n;
     return __x;
 }
 
+#if _LIBCPP_STD_VER <= 17
+template <class _It>
+struct __is_cpp17_contiguous_iterator<__wrap_iter<_It> > : true_type {};
+#endif
+
+template <class _Iter>
+_LIBCPP_CONSTEXPR
+decltype(_VSTD::__to_address(declval<_Iter>()))
+__to_address(__wrap_iter<_Iter> __w) _NOEXCEPT {
+    return _VSTD::__to_address(__w.base());
+}
+
 _LIBCPP_END_NAMESPACE_STD
 
 _LIBCPP_POP_MACROS
 
 #endif // _LIBCPP___ITERATOR_WRAP_ITER_H