[libc++] [test] Simplify (sized_)sentinel_wrapper; use it in more places.

[• Quuxplusone]

Summary

This is the latest installment in my series "Get rid of bogus operator== overloads that individual test files provide, for types they don't own."

Step 1 here is test_iterators.h: Ensure that sentinel_wrapper<It> is always valid and models sentinel_for<It> — even when It is an input iterator that's not equality-comparable with itself. This also brings the code style of sentinel_wrapper and sized_sentinel into line with the rest of the test iterators: use hidden-friend operators, provide an ADL base(it) function, and so on. As usual for test code, the simpler the better.

Step 2 is a handful of files that were using .base() on a sentinel. We got rid of that method in Step 1. Namely, these files were doing foo.base().base() where the inner .base() is the Standard-mandated .base() member of an STL type, and the outer .base() is the one we just removed. ADL base(foo.base()) is even a little easier on the eyes anyway. This affects range.take/sentinel/base.pass.cpp, range.take/sentinel/ctor.pass.cpp, range.transform/end.pass.cpp, range.transform/iterator/base.pass.cpp. Drive-by assert more intermediate results in all these tests.

Step 3 is types.h: Use the newly empowered sized_sentinel<It> in SizedForwardView and SizedRandomAccessView, eliminating several bogus operator- declarations! This has cascading effects in range.common.view/{begin,end}.pass.cpp (since the sentinel type of the view has changed spelling).

[ldionne]

This is a really nice cleanup, however I would like the drive-by refactorings to be split into a different NFC patch.

libcxx/test/std/ranges/range.adaptors/range.take/sentinel/ctor.pass.cpp

22

Same here, please keep for the other NFC refactoring patch.

libcxx/test/std/ranges/range.adaptors/range.transform/end.pass.cpp

37 ↗

(On Diff #394400)

Same here, please keep for a separate patch.

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

41 ↗

(On Diff #394400)

This is the only change that actually pertains to this patch IMO, i.e. calling base(it) instead of it.base(). I like the rest of the refactoring you're doing, but can you please keep it for a separate patch?

libcxx/test/support/test_iterators.h

843–844

846

858–859

Let's not make this change in this patch -- let's do it separately.

[• Quuxplusone]

Scale back to just the refactor of test_iterators.h, removal of sent.base() and its effects, and a couple of places where we should be using sized_sentinel — i.e. all the sentinel-specific stuff.
Other stuff will move to a different PR.

[ldionne]

Thanks, this is a great cleanup and it will allow using sentinel_wrapper with non-default-constructible iterators, which was lacking.

[• Quuxplusone]

libcxx/test/std/ranges/range.adaptors/range.take/sentinel/ctor.pass.cpp

22

The NFC refactoring patch is now D116002.

libcxx/test/std/ranges/range.adaptors/range.common.view/begin.pass.cpp

int main(int, char**) {
   static_assert(test());
 
   // The non-constexpr tests:
   int buf[8] = {1, 2, 3, 4, 5, 6, 7, 8};
 
   {
     SizedForwardView view{buf, buf + 8};
     std::ranges::common_view<SizedForwardView> common(view);
-    using CommonIter = std::common_iterator<ForwardIter, sentinel_wrapper<ForwardIter>>;
+    using CommonIter = std::common_iterator<ForwardIter, sized_sentinel<ForwardIter>>;
     std::same_as<CommonIter> auto begin = common.begin();
     assert(begin == std::ranges::begin(view));
   }
   {
     SizedForwardView view{buf, buf + 8};
     std::ranges::common_view<SizedForwardView> const common(view);
-    using CommonIter = std::common_iterator<ForwardIter, sentinel_wrapper<ForwardIter>>;
+    using CommonIter = std::common_iterator<ForwardIter, sized_sentinel<ForwardIter>>;
     std::same_as<CommonIter> auto begin = common.begin();
     assert(begin == std::ranges::begin(view));
   }
 
   {
     MoveOnlyView view{buf, buf + 8};
     std::ranges::common_view<MoveOnlyView> common(std::move(view));
     using CommonIter = std::common_iterator<int*, sentinel_wrapper<int*>>;

libcxx/test/std/ranges/range.adaptors/range.common.view/end.pass.cpp

 
 int main(int, char**) {
   test();
   static_assert(test());
 
   {
     int buf[8] = {1, 2, 3, 4, 5, 6, 7, 8};
 
-    using CommonForwardIter = std::common_iterator<ForwardIter, sentinel_wrapper<ForwardIter>>;
+    using CommonForwardIter = std::common_iterator<ForwardIter, sized_sentinel<ForwardIter>>;
     using CommonIntIter = std::common_iterator<int*, sentinel_wrapper<int*>>;
 
     {
       SizedForwardView view{buf, buf + 8};
       std::ranges::common_view<SizedForwardView> common(view);
       std::same_as<CommonForwardIter> auto end = common.end();
       assert(end == CommonForwardIter(std::ranges::end(view)));
     }

libcxx/test/std/ranges/range.adaptors/range.common.view/types.h

 static_assert(std::copyable<CopyableView>);
 
 using ForwardIter = forward_iterator<int*>;
 struct SizedForwardView : std::ranges::view_base {
   int* begin_;
   int* end_;
   constexpr explicit SizedForwardView(int* b, int* e) : begin_(b), end_(e) { }
   constexpr auto begin() const { return forward_iterator<int*>(begin_); }
-  constexpr auto end() const { return sentinel_wrapper<forward_iterator<int*>>(forward_iterator<int*>(end_)); }
+  constexpr auto end() const { return sized_sentinel<forward_iterator<int*>>(forward_iterator<int*>(end_)); }
 };
-// Required to make SizedForwardView a sized view.
-constexpr auto operator-(sentinel_wrapper<ForwardIter> sent, ForwardIter iter) {
-  return sent.base().base() - iter.base();
-}
-constexpr auto operator-(ForwardIter iter, sentinel_wrapper<ForwardIter> sent) {
-  return iter.base() - sent.base().base();
-}
 static_assert(std::ranges::view<SizedForwardView>);
 static_assert(std::ranges::forward_range<SizedForwardView>);
 static_assert(std::ranges::sized_range<SizedForwardView>);
 
 using RandomAccessIter = random_access_iterator<int*>;
 struct SizedRandomAccessView : std::ranges::view_base {
   int* begin_;
   int* end_;
   constexpr explicit SizedRandomAccessView(int* b, int* e) : begin_(b), end_(e) { }
   constexpr auto begin() const { return random_access_iterator<int*>(begin_); }
-  constexpr auto end() const { return sentinel_wrapper<random_access_iterator<int*>>(random_access_iterator<int*>(end_)); }
+  constexpr auto end() const { return sized_sentinel<random_access_iterator<int*>>(random_access_iterator<int*>(end_)); }
 };
-// Required to make SizedRandomAccessView a sized view.
-constexpr auto operator-(sentinel_wrapper<RandomAccessIter> sent, RandomAccessIter iter) {
-  return sent.base().base() - iter.base();
-}
-constexpr auto operator-(RandomAccessIter iter, sentinel_wrapper<RandomAccessIter> sent) {
-  return iter.base() - sent.base().base();
-}
 static_assert(std::ranges::view<SizedRandomAccessView>);
 static_assert(std::ranges::random_access_range<SizedRandomAccessView>);
 static_assert(std::ranges::sized_range<SizedRandomAccessView>);
 
 struct CommonView : std::ranges::view_base {
   int* begin_;
   int* end_;
   constexpr explicit CommonView(int* b, int* e) : begin_(b), end_(e) { }

libcxx/test/std/ranges/range.adaptors/range.take/sentinel/base.pass.cpp

 #include "../types.h"
 
 constexpr bool test() {
   int buffer[8] = {1, 2, 3, 4, 5, 6, 7, 8};
   auto sw = sentinel_wrapper<int *>(buffer + 8); // Note: not 4, but that's OK.
 
   {
     const std::ranges::take_view<MoveOnlyView> tv(MoveOnlyView{buffer}, 4);
-    assert(tv.end().base().base() == sw.base());
+    assert(base(tv.end().base()) == base(sw));
     ASSERT_SAME_TYPE(decltype(tv.end().base()), sentinel_wrapper<int *>);
   }
 
   {
     std::ranges::take_view<MoveOnlyView> tv(MoveOnlyView{buffer}, 4);
-    assert(tv.end().base().base() == sw.base());
+    assert(base(tv.end().base()) == base(sw));
     ASSERT_SAME_TYPE(decltype(tv.end().base()), sentinel_wrapper<int *>);
   }
 
   return true;
 }
 
 int main(int, char**) {
   test();
   static_assert(test());
 
   return 0;
 }

libcxx/test/std/ranges/range.adaptors/range.take/sentinel/ctor.pass.cpp

 // constexpr explicit sentinel(sentinel_t<Base> end);
 // constexpr sentinel(sentinel<!Const> s)
 //   requires Const && convertible_to<sentinel_t<V>, sentinel_t<Base>>;
 
 #include <ranges>
 #include <cassert>
 
 #include "test_macros.h"
 #include "test_iterators.h"
 #include "../types.h"
 
 constexpr bool test() {
   int buffer[8] = {1, 2, 3, 4, 5, 6, 7, 8};
 
   {
     // Test the default ctor.
     std::ranges::take_view<MoveOnlyView> tv(MoveOnlyView{buffer}, 4);
int buffer[8] = {1, 2, 3, 4, 5, 6, 7, 8};
     assert(sent2 == std::ranges::next(tvConst.begin(), 5));
   }
 
   {
     std::ranges::take_view<CopyableView> tv(CopyableView{buffer}, 6);
     auto sw = sentinel_wrapper<int *>(buffer + 6);
     using Sent = decltype(tv.end());
     Sent sent = Sent(sw);
-    assert(sent.base().base() == sw.base());
+    assert(base(sent.base()) == base(sw));
   }
 
   return true;
 }
 
 int main(int, char**) {
   test();
   static_assert(test());
 
   return 0;
 }

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

 static_assert(std::ranges::contiguous_range<CopyableView>);
 static_assert(std::copyable<CopyableView>);
 
 using ForwardIter = forward_iterator<int*>;
 struct SizedForwardView : std::ranges::view_base {
   int *ptr_;
   constexpr explicit SizedForwardView(int* ptr) : ptr_(ptr) {}
   constexpr auto begin() const { return ForwardIter(ptr_); }
-  constexpr auto end() const { return sentinel_wrapper<ForwardIter>(ForwardIter(ptr_ + 8)); }
+  constexpr auto end() const { return sized_sentinel<ForwardIter>(ForwardIter(ptr_ + 8)); }
 };
-// Required to make SizedForwardView a sized view.
-constexpr auto operator-(sentinel_wrapper<ForwardIter> sent, ForwardIter iter) {
-  return sent.base().base() - iter.base();
-}
-constexpr auto operator-(ForwardIter iter, sentinel_wrapper<ForwardIter> sent) {
-  return iter.base() - sent.base().base();
-}
 static_assert(std::ranges::view<SizedForwardView>);
 static_assert(std::ranges::forward_range<SizedForwardView>);
 static_assert(std::ranges::sized_range<SizedForwardView>);
 
 using RandomAccessIter = random_access_iterator<int*>;
 struct SizedRandomAccessView : std::ranges::view_base {
   int *ptr_;
   constexpr explicit SizedRandomAccessView(int* ptr) : ptr_(ptr) {}
   constexpr auto begin() const { return RandomAccessIter(ptr_); }
-  constexpr auto end() const { return sentinel_wrapper<RandomAccessIter>(RandomAccessIter(ptr_ + 8)); }
+  constexpr auto end() const { return sized_sentinel<RandomAccessIter>(RandomAccessIter(ptr_ + 8)); }
 };
-// Required to make SizedRandomAccessView a sized view.
-constexpr auto operator-(sentinel_wrapper<RandomAccessIter> sent, RandomAccessIter iter) {
-  return sent.base().base() - iter.base();
-}
-constexpr auto operator-(RandomAccessIter iter, sentinel_wrapper<RandomAccessIter> sent) {
-  return iter.base() - sent.base().base();
-}
 static_assert(std::ranges::view<SizedRandomAccessView>);
 static_assert(std::ranges::random_access_range<SizedRandomAccessView>);
 static_assert(std::ranges::sized_range<SizedRandomAccessView>);
 
 #endif // TEST_STD_RANGES_RANGE_ADAPTORS_RANGE_TAKE_TYPES_H

libcxx/test/std/strings/string.view/string.view.cons/from_range.pass.cpp

 static_assert(std::is_constructible_v<std::string_view, std::vector<char>&&>);
 static_assert(std::is_constructible_v<std::string_view, const std::vector<char>&&>);
 
 using SizedButNotContiguousRange = std::ranges::subrange<random_access_iterator<char*>>;
 static_assert(!std::ranges::contiguous_range<SizedButNotContiguousRange>);
 static_assert(std::ranges::sized_range<SizedButNotContiguousRange>);
 static_assert(!std::is_constructible_v<std::string_view, SizedButNotContiguousRange>);
 
-using ContiguousButNotSizedRange = std::ranges::subrange<contiguous_iterator<char*>, sentinel_wrapper<char*>, std::ranges::subrange_kind::unsized>;
+using ContiguousButNotSizedRange = std::ranges::subrange<contiguous_iterator<char*>, sentinel_wrapper<contiguous_iterator<char*>>, std::ranges::subrange_kind::unsized>;
 static_assert(std::ranges::contiguous_range<ContiguousButNotSizedRange>);
 static_assert(!std::ranges::sized_range<ContiguousButNotSizedRange>);
 static_assert(!std::is_constructible_v<std::string_view, ContiguousButNotSizedRange>);
 
 static_assert(!std::is_constructible_v<std::string_view, std::vector<char16_t>>); // different CharT
 
 struct WithStringViewConversionOperator {
   char* begin() const;

libcxx/test/support/test_iterators.h

public:
     void operator,(T const &) = delete;
 
 private:
     I base_;
     difference_type stride_count_ = 0;
     difference_type stride_displacement_ = 0;
 };
 
-template<class T, class U>
-concept sentinel_for_base = requires(U const& u) {
-    u.base();
-    requires std::input_or_output_iterator<std::remove_cvref_t<decltype(u.base())>>;
-    requires std::equality_comparable_with<T, decltype(u.base())>;
-};
-
-template <std::input_or_output_iterator I>
+template <class It>
 class sentinel_wrapper {
 public:
-    sentinel_wrapper() = default;
-    constexpr explicit sentinel_wrapper(I base) : base_(std::move(base)) {}
-
-    constexpr bool operator==(const I& other) const requires std::equality_comparable<I> {
-        return base_ == other;
-    }
-
-    constexpr const I& base() const& { return base_; }
-    constexpr I base() && { return std::move(base_); }
-
-    template<std::input_or_output_iterator I2>
-        requires sentinel_for_base<I, I2>
-    constexpr bool operator==(const I2& other) const {
-        return base_ == other.base();
-    }
-
+    explicit sentinel_wrapper() = default;
+    constexpr explicit sentinel_wrapper(const It& it) : base_(base(it)) {}
+    constexpr bool operator==(const It& other) const { return base_ == base(other); }
+    friend constexpr It base(const sentinel_wrapper& s) { return It(s.base_); }
 private:
-    I base_ = I();
+    decltype(base(std::declval<It>())) base_;
 };
 
-template <std::input_or_output_iterator I>
+template <class It>
 class sized_sentinel {
 public:
-    sized_sentinel() = default;
-    constexpr explicit sized_sentinel(I base) : base_(std::move(base)) {}
-
-    constexpr bool operator==(const I& other) const requires std::equality_comparable<I> {
-        return base_ == other;
-    }
-
-    constexpr const I& base() const& { return base_; }
-    constexpr I base() && { return std::move(base_); }
-
-    template<std::input_or_output_iterator I2>
-        requires sentinel_for_base<I, I2>
-    constexpr bool operator==(const I2& other) const {
-        return base_ == other.base();
-    }
-
+    explicit sized_sentinel() = default;
+    constexpr explicit sized_sentinel(const It& it) : base_(base(it)) {}
+    constexpr bool operator==(const It& other) const { return base_ == base(other); }
+    friend constexpr auto operator-(const sized_sentinel& s, const It& i) { return s.base_ - base(i); }
+    friend constexpr auto operator-(const It& i, const sized_sentinel& s) { return base(i) - s.base_; }
+    friend constexpr It base(const sized_sentinel& s) { return It(s.base_); }
 private:
-    I base_ = I();
+    decltype(base(std::declval<It>())) base_;
 };
 
-template <std::input_or_output_iterator I>
-constexpr auto operator-(sized_sentinel<I> sent, std::input_or_output_iterator auto iter) {
-  return sent.base() - iter;
-}
-
-template <std::input_or_output_iterator I>
-constexpr auto operator-(std::input_or_output_iterator auto iter, sized_sentinel<I> sent) {
-  return iter - sent.base();
-}
-
 template <class It>
 class three_way_contiguous_iterator
 {
     static_assert(std::is_pointer_v<It>, "Things probably break in this case");
 
     It it_;
 
     template <class U> friend class three_way_contiguous_iterator;