[libc++] [ranges] Uncomment operator<=> in transform and iota iterators.

[• Quuxplusone]

Summary

The existing tests for transform_view::iterator weren't quite right,
and can be simplified now that we have more of C++20 available to us.
Having done that, let's use the same pattern for iota_view::iterator
as well.

The transform_view test had been commented out because we were missing std::is_eq (D110515). However, it turned out that the commented-out test was a bit bogus for a couple other reasons:

• it lacked coverage for the rewritten operators and operator==
• it was a bit overcomplicated (especially now that std::views::transform has landed)
• it wrongly assumed that the ContiguousView from "types.h" had non-three-way-comparable iterators

Note the design decision here to rely on the fact that (for now) random_access_iterator has no operator<=>. If we ever add one, this test's sanity-check will fail, and we'll have to go find a different legacy iterator to use instead.

[• Quuxplusone]

libcxx/include/__ranges/transform_view.h

271–276

I wonder if we should just replace this function body with = default instead. The only user-visible difference would be that it would become implicitly noexcept(auto) instead of noexcept(false).

283

Unfortunately, I am not convinced that it would be conforming to eliminate these redundant relational operators in the case that operator<=> exists. The user might have some clever way of observing whether it was actual-operator< or rewritten-candidate-operator<=> getting called. At least I'm not convinced that no such cleverness exists.

[• Quuxplusone]

Find and fix the same issue with iota_view::iterator.

[• Quuxplusone]

Oops, also use same_as<R> auto r = in the transform test, to match what I late-breakingly decided to do in the iota test.

[Mordante]

LGTM.

libcxx/include/__ranges/iota_view.h

 // -*- C++ -*-
 //===----------------------------------------------------------------------===//
 //
 // 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___RANGES_IOTA_VIEW_H
 #define _LIBCPP___RANGES_IOTA_VIEW_H
 
+#include <__compare/three_way_comparable.h>
+#include <__concepts/arithmetic.h>
+#include <__concepts/constructible.h>
+#include <__concepts/convertible_to.h>
+#include <__concepts/copyable.h>
+#include <__concepts/equality_comparable.h>
+#include <__concepts/invocable.h>
+#include <__concepts/same_as.h>
+#include <__concepts/semiregular.h>
+#include <__concepts/totally_ordered.h>
 #include <__config>
 #include <__debug>
 #include <__functional/ranges_operations.h>
 #include <__iterator/concepts.h>
 #include <__iterator/incrementable_traits.h>
 #include <__iterator/iterator_traits.h>
 #include <__iterator/unreachable_sentinel.h>
 #include <__ranges/copyable_box.h>
 #include <__ranges/enable_borrowed_range.h>
 #include <__ranges/view_interface.h>
 #include <__utility/forward.h>
 #include <__utility/move.h>
-#include <concepts>
 #include <type_traits>
 
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
 #pragma GCC system_header
 #endif
 
 _LIBCPP_BEGIN_NAMESPACE_STD
 
struct __iterator : public __iota_iterator_category<_Start> {
 
       _LIBCPP_HIDE_FROM_ABI
       friend constexpr bool operator>=(const __iterator& __x, const __iterator& __y)
         requires totally_ordered<_Start>
       {
         return !(__x < __y);
       }
 
-//       friend constexpr auto operator<=>(const __iterator& __x, const __iterator& __y)
-//         requires totally_ordered<_Start> && three_way_comparable<_Start>
-//       {
-//         return __x.__value_ <=> __y.__value_;
-//       }
+      friend constexpr auto operator<=>(const __iterator& __x, const __iterator& __y)
+        requires totally_ordered<_Start> && three_way_comparable<_Start>
+      {
+        return __x.__value_ <=> __y.__value_;
+      }
 
       _LIBCPP_HIDE_FROM_ABI
       friend constexpr __iterator operator+(__iterator __i, difference_type __n)
         requires __advanceable<_Start>
       {
         __i += __n;
         return __i;
       }

libcxx/include/__ranges/transform_view.h

 // -*- C++ -*-
 //===----------------------------------------------------------------------===//
 //
 // 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___RANGES_TRANSFORM_VIEW_H
 #define _LIBCPP___RANGES_TRANSFORM_VIEW_H
 
+#include <__compare/three_way_comparable.h>
+#include <__concepts/constructible.h>
+#include <__concepts/convertible_to.h>
+#include <__concepts/copyable.h>
+#include <__concepts/derived_from.h>
+#include <__concepts/equality_comparable.h>
+#include <__concepts/invocable.h>
 #include <__config>
 #include <__functional/bind_back.h>
 #include <__functional/invoke.h>
 #include <__iterator/concepts.h>
 #include <__iterator/iter_swap.h>
 #include <__iterator/iterator_traits.h>
 #include <__memory/addressof.h>
 #include <__ranges/access.h>
 #include <__ranges/all.h>
 #include <__ranges/concepts.h>
 #include <__ranges/copyable_box.h>
 #include <__ranges/empty.h>
 #include <__ranges/range_adaptor.h>
 #include <__ranges/size.h>
 #include <__ranges/view_interface.h>
 #include <__utility/forward.h>
 #include <__utility/in_place.h>
 #include <__utility/move.h>
-#include <concepts>
 #include <type_traits>
 
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
 #pragma GCC system_header
 #endif
 
 _LIBCPP_BEGIN_NAMESPACE_STD
 
public:
   _LIBCPP_HIDE_FROM_ABI
   constexpr decltype(auto) operator[](difference_type __n) const
     noexcept(noexcept(_VSTD::invoke(*__parent_->__func_, __current_[__n])))
     requires random_access_range<_Base>
   {
     return _VSTD::invoke(*__parent_->__func_, __current_[__n]);
   }
 
   _LIBCPP_HIDE_FROM_ABI
   friend constexpr bool operator==(const __iterator& __x, const __iterator& __y)
     requires equality_comparable<iterator_t<_Base>>
   {
     return __x.__current_ == __y.__current_;
   }
 
   _LIBCPP_HIDE_FROM_ABI
   friend constexpr bool operator<(const __iterator& __x, const __iterator& __y)
     requires random_access_range<_Base>
   {
     return __x.__current_ < __y.__current_;
   }
 
   _LIBCPP_HIDE_FROM_ABI
   friend constexpr bool operator>(const __iterator& __x, const __iterator& __y)
     requires random_access_range<_Base>
   {
     return __x.__current_ > __y.__current_;
   }
 
   _LIBCPP_HIDE_FROM_ABI
   friend constexpr bool operator<=(const __iterator& __x, const __iterator& __y)
     requires random_access_range<_Base>
   {
     return __x.__current_ <= __y.__current_;
   }
 
   _LIBCPP_HIDE_FROM_ABI
   friend constexpr bool operator>=(const __iterator& __x, const __iterator& __y)
     requires random_access_range<_Base>
   {
     return __x.__current_ >= __y.__current_;
   }
 
-// TODO: Fix this as soon as soon as three_way_comparable is implemented.
-//   _LIBCPP_HIDE_FROM_ABI
-//   friend constexpr auto operator<=>(const __iterator& __x, const __iterator& __y)
-//     requires random_access_range<_Base> && three_way_comparable<iterator_t<_Base>>
-//   {
-//     return __x.__current_ <=> __y.__current_;
-//   }
+#if !defined(_LIBCPP_HAS_NO_SPACESHIP_OPERATOR)
+  _LIBCPP_HIDE_FROM_ABI
+  friend constexpr auto operator<=>(const __iterator& __x, const __iterator& __y)
+    requires random_access_range<_Base> && three_way_comparable<iterator_t<_Base>>
+  {
+    return __x.__current_ <=> __y.__current_;
+  }
+#endif // !defined(_LIBCPP_HAS_NO_SPACESHIP_OPERATOR)
 
   _LIBCPP_HIDE_FROM_ABI
   friend constexpr __iterator operator+(__iterator __i, difference_type __n)
     requires random_access_range<_Base>
   {
     return __iterator{*__i.__parent_, __i.__current_ + __n};
   }
 

libcxx/test/std/ranges/range.adaptors/range.transform/iterator/compare.pass.cpp

 //===----------------------------------------------------------------------===//
 //
 // 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
 //
 //===----------------------------------------------------------------------===//
 
 // UNSUPPORTED: c++03, c++11, c++14, c++17
 // UNSUPPORTED: libcpp-no-concepts
 // UNSUPPORTED: libcpp-has-no-incomplete-ranges
 
-// transform_view::<iterator>::operator{<,>,<=,>=}
+// transform_view::<iterator>::operator{<,>,<=,>=,==,!=,<=>}
 
 #include <ranges>
 #include <compare>
 
 #include "test_macros.h"
+#include "test_iterators.h"
 #include "../types.h"
 
 constexpr bool test() {
   {
-    std::ranges::transform_view<ContiguousView, PlusOne> transformView1;
-    auto iter1 = std::move(transformView1).begin();
-    std::ranges::transform_view<ContiguousView, PlusOne> transformView2;
-    auto iter2 = std::move(transformView2).begin();
-    assert(iter1 == iter2);
-    assert(iter1 + 1 != iter2);
-    assert(iter1 + 1 == iter2 + 1);
-
-    assert(iter1 < iter1 + 1);
-    assert(iter1 + 1 > iter1);
-    assert(iter1 <= iter1 + 1);
-    assert(iter1 <= iter2);
-    assert(iter1 + 1 >= iter2);
-    assert(iter1     >= iter2);
+    // Test a new-school iterator with operator<=>; the transform iterator should also have operator<=>.
+    using It = three_way_contiguous_iterator<int*>;
+    static_assert(std::three_way_comparable<It>);
+    using R = std::ranges::transform_view<std::ranges::subrange<It>, PlusOne>;
+    static_assert(std::three_way_comparable<std::ranges::iterator_t<R>>);
+
+    int a[] = {1,2,3};
+    std::same_as<R> auto r = std::ranges::subrange<It>(It(a), It(a+3)) | std::views::transform(PlusOne());
+    auto iter1 = r.begin();
+    auto iter2 = iter1 + 1;
+
+    assert(!(iter1 < iter1));  assert(iter1 < iter2);     assert(!(iter2 < iter1));
+    assert(iter1 <= iter1);    assert(iter1 <= iter2);    assert(!(iter2 <= iter1));
+    assert(!(iter1 > iter1));  assert(!(iter1 > iter2));  assert(iter2 > iter1);
+    assert(iter1 >= iter1);    assert(!(iter1 >= iter2)); assert(iter2 >= iter1);
+    assert(iter1 == iter1);    assert(!(iter1 == iter2)); assert(iter2 == iter2);
+    assert(!(iter1 != iter1)); assert(iter1 != iter2);    assert(!(iter2 != iter2));
+
+    assert((iter1 <=> iter2) == std::strong_ordering::less);
+    assert((iter1 <=> iter1) == std::strong_ordering::equal);
+    assert((iter2 <=> iter1) == std::strong_ordering::greater);
   }
 
-// TODO: when three_way_comparable is implemented and std::is_eq is implemented,
-// uncomment this.
-//   {
-//     std::ranges::transform_view<ThreeWayCompView, PlusOne> transformView1;
-//     auto iter1 = transformView1.begin();
-//     std::ranges::transform_view<ThreeWayCompView, PlusOne> transformView2;
-//     auto iter2 = transformView2.begin();
-//
-//     assert(std::is_eq(iter1   <=> iter2));
-//     assert(std::is_lteq(iter1 <=> iter2));
-//     ++iter2;
-//     assert(std::is_neq(iter1  <=> iter2));
-//     assert(std::is_lt(iter1   <=> iter2));
-//     assert(std::is_gt(iter2   <=> iter1));
-//     assert(std::is_gteq(iter2 <=> iter1));
-//
-//     static_assert( std::three_way_comparable<std::iterator_t<std::ranges::transform_view<ThreeWayCompView, PlusOne>>>);
-//     static_assert(!std::three_way_comparable<std::iterator_t<std::ranges::transform_view<ContiguousView, PlusOne>>>);
-//   }
+  {
+    // Test an old-school iterator with no operator<=>; the transform iterator shouldn't have operator<=> either.
+    using It = random_access_iterator<int*>;
+    static_assert(!std::three_way_comparable<It>);
+    using R = std::ranges::transform_view<std::ranges::subrange<It>, PlusOne>;
+    static_assert(!std::three_way_comparable<std::ranges::iterator_t<R>>);
+
+    int a[] = {1,2,3};
+    std::same_as<R> auto r = std::ranges::subrange<It>(It(a), It(a+3)) | std::views::transform(PlusOne());
+    auto iter1 = r.begin();
+    auto iter2 = iter1 + 1;
+
+    assert(!(iter1 < iter1));  assert(iter1 < iter2);     assert(!(iter2 < iter1));
+    assert(iter1 <= iter1);    assert(iter1 <= iter2);    assert(!(iter2 <= iter1));
+    assert(!(iter1 > iter1));  assert(!(iter1 > iter2));  assert(iter2 > iter1);
+    assert(iter1 >= iter1);    assert(!(iter1 >= iter2)); assert(iter2 >= iter1);
+    assert(iter1 == iter1);    assert(!(iter1 == iter2)); assert(iter2 == iter2);
+    assert(!(iter1 != iter1)); assert(iter1 != iter2);    assert(!(iter2 != iter2));
+  }
 
   return true;
 }
 
 int main(int, char**) {
   test();
   static_assert(test());
 
   return 0;
 }

libcxx/test/std/ranges/range.adaptors/range.transform/types.h

 using CountedIter = stride_counting_iterator<forward_iterator<int*>>;
 struct CountedView : std::ranges::view_base {
   constexpr CountedIter begin() { return CountedIter(ForwardIter(globalBuff)); }
   constexpr CountedIter begin() const { return CountedIter(ForwardIter(globalBuff)); }
   constexpr CountedIter end() { return CountedIter(ForwardIter(globalBuff + 8)); }
   constexpr CountedIter end() const { return CountedIter(ForwardIter(globalBuff + 8)); }
 };
 
-using ThreeWayCompIter = three_way_contiguous_iterator<int*>;
-struct ThreeWayCompView : std::ranges::view_base {
-  constexpr ThreeWayCompIter begin() { return ThreeWayCompIter(globalBuff); }
-  constexpr ThreeWayCompIter begin() const { return ThreeWayCompIter(globalBuff); }
-  constexpr ThreeWayCompIter end() { return ThreeWayCompIter(globalBuff + 8); }
-  constexpr ThreeWayCompIter end() const { return ThreeWayCompIter(globalBuff + 8); }
-};
-
 struct TimesTwo {
   constexpr int operator()(int x) const { return x * 2; }
 };
 
 struct PlusOneMutable {
   constexpr int operator()(int x) { return x + 1; }
 };
 

libcxx/test/std/ranges/range.factories/range.iota.view/iterator/compare.pass.cpp

 //===----------------------------------------------------------------------===//
 //
 // 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
 //
 //===----------------------------------------------------------------------===//
 
 // UNSUPPORTED: c++03, c++11, c++14, c++17
 // UNSUPPORTED: libcpp-no-concepts
 // UNSUPPORTED: libcpp-has-no-incomplete-ranges
 
-// friend constexpr bool operator<(const iterator& x, const iterator& y)
-//   requires totally_ordered<W>;
-// friend constexpr bool operator>(const iterator& x, const iterator& y)
-//   requires totally_ordered<W>;
-// friend constexpr bool operator<=(const iterator& x, const iterator& y)
-//   requires totally_ordered<W>;
-// friend constexpr bool operator>=(const iterator& x, const iterator& y)
-//   requires totally_ordered<W>;
-// friend constexpr bool operator==(const iterator& x, const iterator& y)
-//   requires equality_comparable<W>;
-
-// TODO: test spaceship operator once it's implemented.
+// iota_view::<iterator>::operator{<,>,<=,>=,==,!=,<=>}
 
 #include <ranges>
-#include <cassert>
+#include <compare>
 
 #include "test_macros.h"
+#include "test_iterators.h"
 #include "../types.h"
 
 constexpr bool test() {
   {
-    const std::ranges::iota_view<int> io(0);
-    assert(                  io.begin()  ==                   io.begin() );
-    assert(                  io.begin()  != std::ranges::next(io.begin()));
-    assert(                  io.begin()  <  std::ranges::next(io.begin()));
-    assert(std::ranges::next(io.begin()) >                    io.begin() );
-    assert(                  io.begin()  <= std::ranges::next(io.begin()));
-    assert(std::ranges::next(io.begin()) >=                   io.begin() );
-    assert(                  io.begin()  <=                   io.begin() );
-    assert(                  io.begin()  >=                   io.begin() );
-  }
-  {
-    std::ranges::iota_view<int> io(0);
-    assert(                  io.begin()  ==                   io.begin() );
-    assert(                  io.begin()  != std::ranges::next(io.begin()));
-    assert(                  io.begin()  <  std::ranges::next(io.begin()));
-    assert(std::ranges::next(io.begin()) >                    io.begin() );
-    assert(                  io.begin()  <= std::ranges::next(io.begin()));
-    assert(std::ranges::next(io.begin()) >=                   io.begin() );
-    assert(                  io.begin()  <=                   io.begin() );
-    assert(                  io.begin()  >=                   io.begin() );
+    // Test `int`, which has operator<=>; the iota iterator should also have operator<=>.
+    using R = std::ranges::iota_view<int>;
+    static_assert(std::three_way_comparable<std::ranges::iterator_t<R>>);
+
+    std::same_as<R> auto r = std::views::iota(42);
+    auto iter1 = r.begin();
+    auto iter2 = iter1 + 1;
+
+    assert(!(iter1 < iter1));  assert(iter1 < iter2);     assert(!(iter2 < iter1));
+    assert(iter1 <= iter1);    assert(iter1 <= iter2);    assert(!(iter2 <= iter1));
+    assert(!(iter1 > iter1));  assert(!(iter1 > iter2));  assert(iter2 > iter1);
+    assert(iter1 >= iter1);    assert(!(iter1 >= iter2)); assert(iter2 >= iter1);
+    assert(iter1 == iter1);    assert(!(iter1 == iter2)); assert(iter2 == iter2);
+    assert(!(iter1 != iter1)); assert(iter1 != iter2);    assert(!(iter2 != iter2));
+
+    assert((iter1 <=> iter2) == std::strong_ordering::less);
+    assert((iter1 <=> iter1) == std::strong_ordering::equal);
+    assert((iter2 <=> iter1) == std::strong_ordering::greater);
   }
+
   {
-    const std::ranges::iota_view<SomeInt> io(SomeInt(0));
-    assert(                  io.begin()  ==                   io.begin() );
-    assert(                  io.begin()  != std::ranges::next(io.begin()));
-    assert(                  io.begin()  <  std::ranges::next(io.begin()));
-    assert(std::ranges::next(io.begin()) >                    io.begin() );
-    assert(                  io.begin()  <= std::ranges::next(io.begin()));
-    assert(std::ranges::next(io.begin()) >=                   io.begin() );
-    assert(                  io.begin()  <=                   io.begin() );
-    assert(                  io.begin()  >=                   io.begin() );
+    // Test a new-school iterator with operator<=>; the iota iterator should also have operator<=>.
+    using It = three_way_contiguous_iterator<int*>;
+    static_assert(std::three_way_comparable<It>);
+    using R = std::ranges::iota_view<It>;
+    static_assert(std::three_way_comparable<std::ranges::iterator_t<R>>);
+
+    int a[] = {1,2,3};
+    std::same_as<R> auto r = std::views::iota(It(a));
+    auto iter1 = r.begin();
+    auto iter2 = iter1 + 1;
+
+    assert(!(iter1 < iter1));  assert(iter1 < iter2);     assert(!(iter2 < iter1));
+    assert(iter1 <= iter1);    assert(iter1 <= iter2);    assert(!(iter2 <= iter1));
+    assert(!(iter1 > iter1));  assert(!(iter1 > iter2));  assert(iter2 > iter1);
+    assert(iter1 >= iter1);    assert(!(iter1 >= iter2)); assert(iter2 >= iter1);
+    assert(iter1 == iter1);    assert(!(iter1 == iter2)); assert(iter2 == iter2);
+    assert(!(iter1 != iter1)); assert(iter1 != iter2);    assert(!(iter2 != iter2));
+
+    assert((iter1 <=> iter2) == std::strong_ordering::less);
+    assert((iter1 <=> iter1) == std::strong_ordering::equal);
+    assert((iter2 <=> iter1) == std::strong_ordering::greater);
   }
+
   {
-    std::ranges::iota_view<SomeInt> io(SomeInt(0));
-    assert(                  io.begin()  ==                   io.begin() );
-    assert(                  io.begin()  != std::ranges::next(io.begin()));
-    assert(                  io.begin()  <  std::ranges::next(io.begin()));
-    assert(std::ranges::next(io.begin()) >                    io.begin() );
-    assert(                  io.begin()  <= std::ranges::next(io.begin()));
-    assert(std::ranges::next(io.begin()) >=                   io.begin() );
-    assert(                  io.begin()  <=                   io.begin() );
-    assert(                  io.begin()  >=                   io.begin() );
+    // Test an old-school iterator with no operator<=>; the iota iterator shouldn't have operator<=> either.
+    using It = random_access_iterator<int*>;
+    static_assert(!std::three_way_comparable<It>);
+    using R = std::ranges::iota_view<It>;
+    static_assert(!std::three_way_comparable<std::ranges::iterator_t<R>>);
+
+    int a[] = {1,2,3};
+    std::same_as<R> auto r = std::views::iota(It(a));
+    auto iter1 = r.begin();
+    auto iter2 = iter1 + 1;
+
+    assert(!(iter1 < iter1));  assert(iter1 < iter2);     assert(!(iter2 < iter1));
+    assert(iter1 <= iter1);    assert(iter1 <= iter2);    assert(!(iter2 <= iter1));
+    assert(!(iter1 > iter1));  assert(!(iter1 > iter2));  assert(iter2 > iter1);
+    assert(iter1 >= iter1);    assert(!(iter1 >= iter2)); assert(iter2 >= iter1);
+    assert(iter1 == iter1);    assert(!(iter1 == iter2)); assert(iter2 == iter2);
+    assert(!(iter1 != iter1)); assert(iter1 != iter2);    assert(!(iter2 != iter2));
   }
 
   return true;
 }
 
 int main(int, char**) {
   test();
   static_assert(test());
 
   return 0;
 }