[libcxx-commits] [lld] [libc] [mlir] [openmp] [llvm] [libcxxabi] [flang] [lldb] [libcxx] [clang-tools-extra] [clang] [compiler-rt] [libc++] Fix `take_view::__sentinel`'s `operator==` (PR #74655)

Jakub Mazurkiewicz via libcxx-commits libcxx-commits at lists.llvm.org
Thu Dec 7 08:54:03 PST 2023


https://github.com/JMazurkiewicz updated https://github.com/llvm/llvm-project/pull/74655

>From b3de573887cdd86fd6ce168bdcc6d729d73b13b2 Mon Sep 17 00:00:00 2001
From: Jakub Mazurkiewicz <mazkuba3 at gmail.com>
Date: Wed, 6 Dec 2023 14:03:51 +0100
Subject: [PATCH 1/9] [libc++] Fix `take_view::__sentinel`'s `operator==`

---
 libcxx/include/__ranges/take_view.h           |   2 +-
 .../base.pass.cpp                             |   5 +-
 .../ctor.pass.cpp                             |   0
 .../range.take.sentinel/eq.pass.cpp           | 192 ++++++++++++++++++
 .../range.take/sentinel/eq.pass.cpp           |  55 -----
 5 files changed, 194 insertions(+), 60 deletions(-)
 rename libcxx/test/std/ranges/range.adaptors/range.take/{sentinel => range.take.sentinel}/base.pass.cpp (83%)
 rename libcxx/test/std/ranges/range.adaptors/range.take/{sentinel => range.take.sentinel}/ctor.pass.cpp (100%)
 create mode 100644 libcxx/test/std/ranges/range.adaptors/range.take/range.take.sentinel/eq.pass.cpp
 delete mode 100644 libcxx/test/std/ranges/range.adaptors/range.take/sentinel/eq.pass.cpp

diff --git a/libcxx/include/__ranges/take_view.h b/libcxx/include/__ranges/take_view.h
index 4204017d9249b..811428e529f59 100644
--- a/libcxx/include/__ranges/take_view.h
+++ b/libcxx/include/__ranges/take_view.h
@@ -183,7 +183,7 @@ class take_view<_View>::__sentinel {
   template<bool _OtherConst = !_Const>
     requires sentinel_for<sentinel_t<_Base>, iterator_t<__maybe_const<_OtherConst, _View>>>
   _LIBCPP_HIDE_FROM_ABI
-  friend constexpr bool operator==(const _Iter<_Const>& __lhs, const __sentinel& __rhs) {
+  friend constexpr bool operator==(const _Iter<_OtherConst>& __lhs, const __sentinel& __rhs) {
     return __lhs.count() == 0 || __lhs.base() == __rhs.__end_;
   }
 };
diff --git a/libcxx/test/std/ranges/range.adaptors/range.take/sentinel/base.pass.cpp b/libcxx/test/std/ranges/range.adaptors/range.take/range.take.sentinel/base.pass.cpp
similarity index 83%
rename from libcxx/test/std/ranges/range.adaptors/range.take/sentinel/base.pass.cpp
rename to libcxx/test/std/ranges/range.adaptors/range.take/range.take.sentinel/base.pass.cpp
index c949eb7cc0846..15b2b5476e86d 100644
--- a/libcxx/test/std/ranges/range.adaptors/range.take/sentinel/base.pass.cpp
+++ b/libcxx/test/std/ranges/range.adaptors/range.take/range.take.sentinel/base.pass.cpp
@@ -8,10 +8,7 @@
 
 // UNSUPPORTED: c++03, c++11, c++14, c++17
 
-// sentinel() = default;
-// constexpr explicit sentinel(sentinel_t<Base> end);
-// constexpr sentinel(sentinel<!Const> s)
-//   requires Const && convertible_to<sentinel_t<V>, sentinel_t<Base>>;
+// constexpr sentinel_t<Base> base() const;
 
 #include <ranges>
 #include <cassert>
diff --git a/libcxx/test/std/ranges/range.adaptors/range.take/sentinel/ctor.pass.cpp b/libcxx/test/std/ranges/range.adaptors/range.take/range.take.sentinel/ctor.pass.cpp
similarity index 100%
rename from libcxx/test/std/ranges/range.adaptors/range.take/sentinel/ctor.pass.cpp
rename to libcxx/test/std/ranges/range.adaptors/range.take/range.take.sentinel/ctor.pass.cpp
diff --git a/libcxx/test/std/ranges/range.adaptors/range.take/range.take.sentinel/eq.pass.cpp b/libcxx/test/std/ranges/range.adaptors/range.take/range.take.sentinel/eq.pass.cpp
new file mode 100644
index 0000000000000..f20c29b4c6471
--- /dev/null
+++ b/libcxx/test/std/ranges/range.adaptors/range.take/range.take.sentinel/eq.pass.cpp
@@ -0,0 +1,192 @@
+//===----------------------------------------------------------------------===//
+//
+// 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
+
+// friend constexpr bool operator==(const CI<Const>& y, const sentinel& x);
+// template<bool OtherConst = !Const>
+//   requires sentinel_for<sentinel_t<Base>, iterator_t<maybe-const<OtherConst, V>>>
+// friend constexpr bool operator==(const CI<OtherConst>& y, const sentinel& x);
+
+#include <cassert>
+#include <cstddef>
+#include <ranges>
+#include <type_traits>
+#include <utility>
+
+#include "test_iterators.h"
+
+template <bool Const>
+class StrictIterator {
+  using Base = std::conditional_t<Const, const int*, int*>;
+  Base base_;
+
+public:
+  using value_type      = int;
+  using difference_type = std::ptrdiff_t;
+
+  constexpr explicit StrictIterator(Base base) : base_(base) {}
+
+  StrictIterator(StrictIterator&&)            = default;
+  StrictIterator& operator=(StrictIterator&&) = default;
+
+  constexpr StrictIterator& operator++() {
+    ++base_;
+    return *this;
+  }
+
+  constexpr void operator++(int) { ++*this; }
+  constexpr decltype(auto) operator*() const { return *base_; }
+  constexpr Base base() const { return base_; }
+};
+
+static_assert(std::input_iterator<StrictIterator<false>>);
+static_assert(!std::copyable<StrictIterator<false>>);
+static_assert(!std::forward_iterator<StrictIterator<false>>);
+static_assert(std::input_iterator<StrictIterator<true>>);
+static_assert(!std::copyable<StrictIterator<true>>);
+static_assert(!std::forward_iterator<StrictIterator<true>>);
+
+template <bool Const>
+class StrictSentinel {
+  using Base = std::conditional_t<Const, const int*, int*>;
+  Base base_;
+
+public:
+  StrictSentinel() = default;
+  constexpr explicit StrictSentinel(Base base) : base_(base) {}
+
+  friend constexpr bool operator==(const StrictIterator<Const>& it, const StrictSentinel& se) {
+    return it.base() == se.base_;
+  }
+
+  friend constexpr bool operator==(const StrictIterator<!Const>& it, const StrictSentinel& se) {
+    return it.base() == se.base_;
+  }
+};
+
+static_assert(std::sentinel_for<StrictSentinel<true>, StrictIterator<false>>);
+static_assert(std::sentinel_for<StrictSentinel<true>, StrictIterator<true>>);
+static_assert(std::sentinel_for<StrictSentinel<false>, StrictIterator<false>>);
+static_assert(std::sentinel_for<StrictSentinel<false>, StrictIterator<true>>);
+
+struct NotSimpleView : std::ranges::view_base {
+  template <std::size_t N>
+  constexpr explicit NotSimpleView(int (&arr)[N]) : b_(arr), e_(arr + N) {}
+
+  constexpr StrictIterator<false> begin() { return StrictIterator<false>{b_}; }
+  constexpr StrictSentinel<false> end() { return StrictSentinel<false>{e_}; }
+
+  constexpr StrictIterator<true> begin() const { return StrictIterator<true>{b_}; }
+  constexpr StrictSentinel<true> end() const { return StrictSentinel<true>{e_}; }
+
+private:
+  int* b_;
+  int* e_;
+};
+
+static_assert(std::ranges::range<NotSimpleView>);
+static_assert(std::ranges::range<const NotSimpleView>);
+
+struct FancyNonSimpleView : std::ranges::view_base {
+  int* begin();
+  sentinel_wrapper<int*> end();
+
+  long* begin() const;
+  sentinel_wrapper<long*> end() const;
+};
+
+static_assert(std::ranges::range<FancyNonSimpleView>);
+static_assert(std::ranges::range<const FancyNonSimpleView>);
+
+template <class T, class U>
+concept weakly_equality_comparable_with = requires(const T& t, const U& u) {
+  t == u;
+  t != u;
+  u == t;
+  u != t;
+};
+
+constexpr bool test() {
+  int buffer[8] = {1, 2, 3, 4, 5, 6, 7, 8};
+
+  {   // Compare CI<Const> with sentinel<Const>
+    { // Const == true
+      const std::ranges::take_view<NotSimpleView> tv(NotSimpleView{buffer}, 4);
+      std::same_as<bool> decltype(auto) b1 = (tv.end() == std::ranges::next(tv.begin(), 4));
+      assert(b1);
+      std::same_as<bool> decltype(auto) b2 = (std::ranges::next(tv.begin(), 4) == tv.end());
+      assert(b2);
+      std::same_as<bool> decltype(auto) b3 = (tv.end() != tv.begin());
+      assert(b3);
+      std::same_as<bool> decltype(auto) b4 = (tv.begin() != tv.end());
+      assert(b4);
+    }
+
+    { // Const == false
+      std::ranges::take_view<NotSimpleView> tv(NotSimpleView{buffer}, 4);
+      std::same_as<bool> decltype(auto) b1 = (tv.end() == std::ranges::next(tv.begin(), 4));
+      assert(b1);
+      std::same_as<bool> decltype(auto) b2 = (std::ranges::next(tv.begin(), 4) == tv.end());
+      assert(b2);
+      std::same_as<bool> decltype(auto) b3 = (tv.end() != tv.begin());
+      assert(b3);
+      std::same_as<bool> decltype(auto) b4 = (tv.begin() != tv.end());
+      assert(b4);
+    }
+  }
+
+  {   // Compare CI<Const> with sentinel<!Const>
+    { // Const == true
+      std::ranges::take_view<NotSimpleView> tv(NotSimpleView{buffer}, 4);
+      std::same_as<bool> decltype(auto) b1 = (tv.end() == std::ranges::next(std::as_const(tv).begin(), 4));
+      assert(b1);
+      std::same_as<bool> decltype(auto) b2 = (std::ranges::next(std::as_const(tv).begin(), 4) == tv.end());
+      assert(b2);
+      std::same_as<bool> decltype(auto) b3 = (tv.end() != std::as_const(tv).begin());
+      assert(b3);
+      std::same_as<bool> decltype(auto) b4 = (std::as_const(tv).begin() != tv.end());
+      assert(b4);
+    }
+
+    { // Const == false
+      std::ranges::take_view<NotSimpleView> tv(NotSimpleView{buffer}, 4);
+      std::same_as<bool> decltype(auto) b1 = (std::as_const(tv).end() == std::ranges::next(tv.begin(), 4));
+      assert(b1);
+      std::same_as<bool> decltype(auto) b2 = (std::ranges::next(tv.begin(), 4) == std::as_const(tv).end());
+      assert(b2);
+      std::same_as<bool> decltype(auto) b3 = (std::as_const(tv).end() != tv.begin());
+      assert(b3);
+      std::same_as<bool> decltype(auto) b4 = (tv.begin() != std::as_const(tv).end());
+      assert(b4);
+    }
+  }
+
+  { // Check invalid comparisons between CI<Const> and sentinel<!Const>
+    using TakeView = std::ranges::take_view<FancyNonSimpleView>;
+    static_assert(
+        !weakly_equality_comparable_with<std::ranges::iterator_t<const TakeView>, std::ranges::sentinel_t<TakeView>>);
+    static_assert(
+        !weakly_equality_comparable_with<std::ranges::iterator_t<TakeView>, std::ranges::sentinel_t<const TakeView>>);
+
+    // Those should be valid
+    static_assert(
+        weakly_equality_comparable_with<std::ranges::iterator_t<TakeView>, std::ranges::sentinel_t<TakeView>>);
+    static_assert(
+        weakly_equality_comparable_with<std::ranges::iterator_t<TakeView>, std::ranges::sentinel_t<TakeView>>);
+  }
+
+  return true;
+}
+
+int main(int, char**) {
+  test();
+  static_assert(test());
+
+  return 0;
+}
diff --git a/libcxx/test/std/ranges/range.adaptors/range.take/sentinel/eq.pass.cpp b/libcxx/test/std/ranges/range.adaptors/range.take/sentinel/eq.pass.cpp
deleted file mode 100644
index eb265a7e03481..0000000000000
--- a/libcxx/test/std/ranges/range.adaptors/range.take/sentinel/eq.pass.cpp
+++ /dev/null
@@ -1,55 +0,0 @@
-//===----------------------------------------------------------------------===//
-//
-// 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
-
-// sentinel() = default;
-// 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};
-
-  {
-    {
-      const std::ranges::take_view<MoveOnlyView> tv(MoveOnlyView{buffer}, 4);
-      assert(tv.end() == std::ranges::next(tv.begin(), 4));
-      assert(std::ranges::next(tv.begin(), 4) == tv.end());
-    }
-
-    {
-      std::ranges::take_view<MoveOnlyView> tv(MoveOnlyView{buffer}, 4);
-      assert(tv.end() == std::ranges::next(tv.begin(), 4));
-      assert(std::ranges::next(tv.begin(), 4) == tv.end());
-    }
-  }
-
-  {
-    std::ranges::take_view<MoveOnlyView> tvNonConst(MoveOnlyView{buffer}, 4);
-    const std::ranges::take_view<MoveOnlyView> tvConst(MoveOnlyView{buffer}, 4);
-    assert(tvNonConst.end() == std::ranges::next(tvConst.begin(), 4));
-    assert(std::ranges::next(tvConst.begin(), 4) == tvNonConst.end());
-  }
-
-  return true;
-}
-
-int main(int, char**) {
-  test();
-  static_assert(test());
-
-  return 0;
-}

>From 774a106996bcfde059d53aa7f01504836a0d91e5 Mon Sep 17 00:00:00 2001
From: Jakub Mazurkiewicz <mazkuba3 at gmail.com>
Date: Thu, 7 Dec 2023 13:47:45 +0100
Subject: [PATCH 2/9] Check `const TakeView` instead of `TakeView` in
 (duplicated) static assertion

Comment: https://github.com/llvm/llvm-project/pull/74655#discussion_r1418511110
---
 .../range.adaptors/range.take/range.take.sentinel/eq.pass.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libcxx/test/std/ranges/range.adaptors/range.take/range.take.sentinel/eq.pass.cpp b/libcxx/test/std/ranges/range.adaptors/range.take/range.take.sentinel/eq.pass.cpp
index f20c29b4c6471..7bddce84878bf 100644
--- a/libcxx/test/std/ranges/range.adaptors/range.take/range.take.sentinel/eq.pass.cpp
+++ b/libcxx/test/std/ranges/range.adaptors/range.take/range.take.sentinel/eq.pass.cpp
@@ -177,8 +177,8 @@ constexpr bool test() {
     // Those should be valid
     static_assert(
         weakly_equality_comparable_with<std::ranges::iterator_t<TakeView>, std::ranges::sentinel_t<TakeView>>);
-    static_assert(
-        weakly_equality_comparable_with<std::ranges::iterator_t<TakeView>, std::ranges::sentinel_t<TakeView>>);
+    static_assert(weakly_equality_comparable_with<std::ranges::iterator_t<const TakeView>,
+                                                  std::ranges::sentinel_t<const TakeView>>);
   }
 
   return true;

>From 2cc7f7367a05f911038c8393bbccd84b2bf2c044 Mon Sep 17 00:00:00 2001
From: Jakub Mazurkiewicz <mazkuba3 at gmail.com>
Date: Thu, 7 Dec 2023 13:59:14 +0100
Subject: [PATCH 3/9] Add tests for comparisons returning `false`

Comment: https://github.com/llvm/llvm-project/pull/74655/files#r1418520744
---
 .../range.take/range.take.sentinel/eq.pass.cpp   | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/libcxx/test/std/ranges/range.adaptors/range.take/range.take.sentinel/eq.pass.cpp b/libcxx/test/std/ranges/range.adaptors/range.take/range.take.sentinel/eq.pass.cpp
index 7bddce84878bf..052dfae0f976b 100644
--- a/libcxx/test/std/ranges/range.adaptors/range.take/range.take.sentinel/eq.pass.cpp
+++ b/libcxx/test/std/ranges/range.adaptors/range.take/range.take.sentinel/eq.pass.cpp
@@ -126,6 +126,10 @@ constexpr bool test() {
       assert(b3);
       std::same_as<bool> decltype(auto) b4 = (tv.begin() != tv.end());
       assert(b4);
+      std::same_as<bool> decltype(auto) b5 = (std::ranges::next(tv.begin(), 1) == tv.end());
+      assert(!b5);
+      std::same_as<bool> decltype(auto) b6 = (std::ranges::next(tv.begin(), 4) != tv.end());
+      assert(!b6);
     }
 
     { // Const == false
@@ -138,6 +142,10 @@ constexpr bool test() {
       assert(b3);
       std::same_as<bool> decltype(auto) b4 = (tv.begin() != tv.end());
       assert(b4);
+      std::same_as<bool> decltype(auto) b5 = (std::ranges::next(tv.begin(), 2) == tv.end());
+      assert(!b5);
+      std::same_as<bool> decltype(auto) b6 = (std::ranges::next(tv.begin(), 4) != tv.end());
+      assert(!b6);
     }
   }
 
@@ -152,6 +160,10 @@ constexpr bool test() {
       assert(b3);
       std::same_as<bool> decltype(auto) b4 = (std::as_const(tv).begin() != tv.end());
       assert(b4);
+      std::same_as<bool> decltype(auto) b5 = (std::ranges::next(std::as_const(tv).begin(), 1) == tv.end());
+      assert(!b5);
+      std::same_as<bool> decltype(auto) b6 = (std::ranges::next(std::as_const(tv).begin(), 4) != tv.end());
+      assert(!b6);
     }
 
     { // Const == false
@@ -164,6 +176,10 @@ constexpr bool test() {
       assert(b3);
       std::same_as<bool> decltype(auto) b4 = (tv.begin() != std::as_const(tv).end());
       assert(b4);
+      std::same_as<bool> decltype(auto) b5 = (std::ranges::next(tv.begin(), 2) == std::as_const(tv).end());
+      assert(!b5);
+      std::same_as<bool> decltype(auto) b6 = (std::ranges::next(tv.begin(), 4) != std::as_const(tv).end());
+      assert(!b6);
     }
   }
 

>From 3337a0f3616ea21f7953a1d9f9f16adc6e501b5f Mon Sep 17 00:00:00 2001
From: Jakub Mazurkiewicz <mazkuba3 at gmail.com>
Date: Thu, 7 Dec 2023 14:10:55 +0100
Subject: [PATCH 4/9] Turn `StrictIterator` into alias template for
 `cpp20_input_iterator`

---
 .../range.take.sentinel/eq.pass.cpp           | 34 ++-----------------
 1 file changed, 3 insertions(+), 31 deletions(-)

diff --git a/libcxx/test/std/ranges/range.adaptors/range.take/range.take.sentinel/eq.pass.cpp b/libcxx/test/std/ranges/range.adaptors/range.take/range.take.sentinel/eq.pass.cpp
index 052dfae0f976b..db91c7713f713 100644
--- a/libcxx/test/std/ranges/range.adaptors/range.take/range.take.sentinel/eq.pass.cpp
+++ b/libcxx/test/std/ranges/range.adaptors/range.take/range.take.sentinel/eq.pass.cpp
@@ -22,35 +22,7 @@
 #include "test_iterators.h"
 
 template <bool Const>
-class StrictIterator {
-  using Base = std::conditional_t<Const, const int*, int*>;
-  Base base_;
-
-public:
-  using value_type      = int;
-  using difference_type = std::ptrdiff_t;
-
-  constexpr explicit StrictIterator(Base base) : base_(base) {}
-
-  StrictIterator(StrictIterator&&)            = default;
-  StrictIterator& operator=(StrictIterator&&) = default;
-
-  constexpr StrictIterator& operator++() {
-    ++base_;
-    return *this;
-  }
-
-  constexpr void operator++(int) { ++*this; }
-  constexpr decltype(auto) operator*() const { return *base_; }
-  constexpr Base base() const { return base_; }
-};
-
-static_assert(std::input_iterator<StrictIterator<false>>);
-static_assert(!std::copyable<StrictIterator<false>>);
-static_assert(!std::forward_iterator<StrictIterator<false>>);
-static_assert(std::input_iterator<StrictIterator<true>>);
-static_assert(!std::copyable<StrictIterator<true>>);
-static_assert(!std::forward_iterator<StrictIterator<true>>);
+using StrictIterator = cpp20_input_iterator<std::conditional_t<Const, const int*, int*>>;
 
 template <bool Const>
 class StrictSentinel {
@@ -62,11 +34,11 @@ class StrictSentinel {
   constexpr explicit StrictSentinel(Base base) : base_(base) {}
 
   friend constexpr bool operator==(const StrictIterator<Const>& it, const StrictSentinel& se) {
-    return it.base() == se.base_;
+    return base(it) == se.base_;
   }
 
   friend constexpr bool operator==(const StrictIterator<!Const>& it, const StrictSentinel& se) {
-    return it.base() == se.base_;
+    return base(it) == se.base_;
   }
 };
 

>From 4d13e66ed204cd3e92958cb3bae6bfc5f1bba30a Mon Sep 17 00:00:00 2001
From: Jakub Mazurkiewicz <mazkuba3 at gmail.com>
Date: Thu, 7 Dec 2023 14:12:51 +0100
Subject: [PATCH 5/9] Rename `StrictIterator` to `MaybeConstIterator`

---
 .../range.take/range.take.sentinel/eq.pass.cpp | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/libcxx/test/std/ranges/range.adaptors/range.take/range.take.sentinel/eq.pass.cpp b/libcxx/test/std/ranges/range.adaptors/range.take/range.take.sentinel/eq.pass.cpp
index db91c7713f713..de79b236c6ea0 100644
--- a/libcxx/test/std/ranges/range.adaptors/range.take/range.take.sentinel/eq.pass.cpp
+++ b/libcxx/test/std/ranges/range.adaptors/range.take/range.take.sentinel/eq.pass.cpp
@@ -22,7 +22,7 @@
 #include "test_iterators.h"
 
 template <bool Const>
-using StrictIterator = cpp20_input_iterator<std::conditional_t<Const, const int*, int*>>;
+using MaybeConstIterator = cpp20_input_iterator<std::conditional_t<Const, const int*, int*>>;
 
 template <bool Const>
 class StrictSentinel {
@@ -33,28 +33,28 @@ class StrictSentinel {
   StrictSentinel() = default;
   constexpr explicit StrictSentinel(Base base) : base_(base) {}
 
-  friend constexpr bool operator==(const StrictIterator<Const>& it, const StrictSentinel& se) {
+  friend constexpr bool operator==(const MaybeConstIterator<Const>& it, const StrictSentinel& se) {
     return base(it) == se.base_;
   }
 
-  friend constexpr bool operator==(const StrictIterator<!Const>& it, const StrictSentinel& se) {
+  friend constexpr bool operator==(const MaybeConstIterator<!Const>& it, const StrictSentinel& se) {
     return base(it) == se.base_;
   }
 };
 
-static_assert(std::sentinel_for<StrictSentinel<true>, StrictIterator<false>>);
-static_assert(std::sentinel_for<StrictSentinel<true>, StrictIterator<true>>);
-static_assert(std::sentinel_for<StrictSentinel<false>, StrictIterator<false>>);
-static_assert(std::sentinel_for<StrictSentinel<false>, StrictIterator<true>>);
+static_assert(std::sentinel_for<StrictSentinel<true>, MaybeConstIterator<false>>);
+static_assert(std::sentinel_for<StrictSentinel<true>, MaybeConstIterator<true>>);
+static_assert(std::sentinel_for<StrictSentinel<false>, MaybeConstIterator<false>>);
+static_assert(std::sentinel_for<StrictSentinel<false>, MaybeConstIterator<true>>);
 
 struct NotSimpleView : std::ranges::view_base {
   template <std::size_t N>
   constexpr explicit NotSimpleView(int (&arr)[N]) : b_(arr), e_(arr + N) {}
 
-  constexpr StrictIterator<false> begin() { return StrictIterator<false>{b_}; }
+  constexpr MaybeConstIterator<false> begin() { return MaybeConstIterator<false>{b_}; }
   constexpr StrictSentinel<false> end() { return StrictSentinel<false>{e_}; }
 
-  constexpr StrictIterator<true> begin() const { return StrictIterator<true>{b_}; }
+  constexpr MaybeConstIterator<true> begin() const { return MaybeConstIterator<true>{b_}; }
   constexpr StrictSentinel<true> end() const { return StrictSentinel<true>{e_}; }
 
 private:

>From ffd4028ccbfcb97fe0eaf3ecf17aa69c8f9e6f54 Mon Sep 17 00:00:00 2001
From: Jakub Mazurkiewicz <mazkuba3 at gmail.com>
Date: Thu, 7 Dec 2023 14:16:07 +0100
Subject: [PATCH 6/9] Rename `FancyNonSimpleView` to
 `NonCrossConstComparableView`

Comment: https://github.com/llvm/llvm-project/pull/74655/files#r1418509780
---
 .../range.take/range.take.sentinel/eq.pass.cpp            | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/libcxx/test/std/ranges/range.adaptors/range.take/range.take.sentinel/eq.pass.cpp b/libcxx/test/std/ranges/range.adaptors/range.take/range.take.sentinel/eq.pass.cpp
index de79b236c6ea0..ce52ae30bf0cd 100644
--- a/libcxx/test/std/ranges/range.adaptors/range.take/range.take.sentinel/eq.pass.cpp
+++ b/libcxx/test/std/ranges/range.adaptors/range.take/range.take.sentinel/eq.pass.cpp
@@ -65,7 +65,7 @@ struct NotSimpleView : std::ranges::view_base {
 static_assert(std::ranges::range<NotSimpleView>);
 static_assert(std::ranges::range<const NotSimpleView>);
 
-struct FancyNonSimpleView : std::ranges::view_base {
+struct NonCrossConstComparableView : std::ranges::view_base {
   int* begin();
   sentinel_wrapper<int*> end();
 
@@ -73,8 +73,8 @@ struct FancyNonSimpleView : std::ranges::view_base {
   sentinel_wrapper<long*> end() const;
 };
 
-static_assert(std::ranges::range<FancyNonSimpleView>);
-static_assert(std::ranges::range<const FancyNonSimpleView>);
+static_assert(std::ranges::range<NonCrossConstComparableView>);
+static_assert(std::ranges::range<const NonCrossConstComparableView>);
 
 template <class T, class U>
 concept weakly_equality_comparable_with = requires(const T& t, const U& u) {
@@ -156,7 +156,7 @@ constexpr bool test() {
   }
 
   { // Check invalid comparisons between CI<Const> and sentinel<!Const>
-    using TakeView = std::ranges::take_view<FancyNonSimpleView>;
+    using TakeView = std::ranges::take_view<NonCrossConstComparableView>;
     static_assert(
         !weakly_equality_comparable_with<std::ranges::iterator_t<const TakeView>, std::ranges::sentinel_t<TakeView>>);
     static_assert(

>From 65be3393af08e8cf54f77daa21a04c950a21497c Mon Sep 17 00:00:00 2001
From: Jakub Mazurkiewicz <mazkuba3 at gmail.com>
Date: Thu, 7 Dec 2023 14:18:23 +0100
Subject: [PATCH 7/9] Rename `NonSimpleView` to `CrossConstComparableView`

---
 .../range.take/range.take.sentinel/eq.pass.cpp   | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/libcxx/test/std/ranges/range.adaptors/range.take/range.take.sentinel/eq.pass.cpp b/libcxx/test/std/ranges/range.adaptors/range.take/range.take.sentinel/eq.pass.cpp
index ce52ae30bf0cd..a2e08f36a56c2 100644
--- a/libcxx/test/std/ranges/range.adaptors/range.take/range.take.sentinel/eq.pass.cpp
+++ b/libcxx/test/std/ranges/range.adaptors/range.take/range.take.sentinel/eq.pass.cpp
@@ -47,9 +47,9 @@ static_assert(std::sentinel_for<StrictSentinel<true>, MaybeConstIterator<true>>)
 static_assert(std::sentinel_for<StrictSentinel<false>, MaybeConstIterator<false>>);
 static_assert(std::sentinel_for<StrictSentinel<false>, MaybeConstIterator<true>>);
 
-struct NotSimpleView : std::ranges::view_base {
+struct CrossConstComparableView : std::ranges::view_base {
   template <std::size_t N>
-  constexpr explicit NotSimpleView(int (&arr)[N]) : b_(arr), e_(arr + N) {}
+  constexpr explicit CrossConstComparableView(int (&arr)[N]) : b_(arr), e_(arr + N) {}
 
   constexpr MaybeConstIterator<false> begin() { return MaybeConstIterator<false>{b_}; }
   constexpr StrictSentinel<false> end() { return StrictSentinel<false>{e_}; }
@@ -62,8 +62,8 @@ struct NotSimpleView : std::ranges::view_base {
   int* e_;
 };
 
-static_assert(std::ranges::range<NotSimpleView>);
-static_assert(std::ranges::range<const NotSimpleView>);
+static_assert(std::ranges::range<CrossConstComparableView>);
+static_assert(std::ranges::range<const CrossConstComparableView>);
 
 struct NonCrossConstComparableView : std::ranges::view_base {
   int* begin();
@@ -89,7 +89,7 @@ constexpr bool test() {
 
   {   // Compare CI<Const> with sentinel<Const>
     { // Const == true
-      const std::ranges::take_view<NotSimpleView> tv(NotSimpleView{buffer}, 4);
+      const std::ranges::take_view<CrossConstComparableView> tv(CrossConstComparableView{buffer}, 4);
       std::same_as<bool> decltype(auto) b1 = (tv.end() == std::ranges::next(tv.begin(), 4));
       assert(b1);
       std::same_as<bool> decltype(auto) b2 = (std::ranges::next(tv.begin(), 4) == tv.end());
@@ -105,7 +105,7 @@ constexpr bool test() {
     }
 
     { // Const == false
-      std::ranges::take_view<NotSimpleView> tv(NotSimpleView{buffer}, 4);
+      std::ranges::take_view<CrossConstComparableView> tv(CrossConstComparableView{buffer}, 4);
       std::same_as<bool> decltype(auto) b1 = (tv.end() == std::ranges::next(tv.begin(), 4));
       assert(b1);
       std::same_as<bool> decltype(auto) b2 = (std::ranges::next(tv.begin(), 4) == tv.end());
@@ -123,7 +123,7 @@ constexpr bool test() {
 
   {   // Compare CI<Const> with sentinel<!Const>
     { // Const == true
-      std::ranges::take_view<NotSimpleView> tv(NotSimpleView{buffer}, 4);
+      std::ranges::take_view<CrossConstComparableView> tv(CrossConstComparableView{buffer}, 4);
       std::same_as<bool> decltype(auto) b1 = (tv.end() == std::ranges::next(std::as_const(tv).begin(), 4));
       assert(b1);
       std::same_as<bool> decltype(auto) b2 = (std::ranges::next(std::as_const(tv).begin(), 4) == tv.end());
@@ -139,7 +139,7 @@ constexpr bool test() {
     }
 
     { // Const == false
-      std::ranges::take_view<NotSimpleView> tv(NotSimpleView{buffer}, 4);
+      std::ranges::take_view<CrossConstComparableView> tv(CrossConstComparableView{buffer}, 4);
       std::same_as<bool> decltype(auto) b1 = (std::as_const(tv).end() == std::ranges::next(tv.begin(), 4));
       assert(b1);
       std::same_as<bool> decltype(auto) b2 = (std::ranges::next(tv.begin(), 4) == std::as_const(tv).end());

>From 599038804b7e7b4cfb204fdb5a77d5f0805aac6d Mon Sep 17 00:00:00 2001
From: Jakub Mazurkiewicz <mazkuba3 at gmail.com>
Date: Thu, 7 Dec 2023 14:19:20 +0100
Subject: [PATCH 8/9] Rename `StrictSentinel` to `CrossConstComparableSentinel`

Comment: https://github.com/llvm/llvm-project/pull/74655#discussion_r1418507136
---
 .../range.take.sentinel/eq.pass.cpp           | 22 +++++++++----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/libcxx/test/std/ranges/range.adaptors/range.take/range.take.sentinel/eq.pass.cpp b/libcxx/test/std/ranges/range.adaptors/range.take/range.take.sentinel/eq.pass.cpp
index a2e08f36a56c2..9353d65944a23 100644
--- a/libcxx/test/std/ranges/range.adaptors/range.take/range.take.sentinel/eq.pass.cpp
+++ b/libcxx/test/std/ranges/range.adaptors/range.take/range.take.sentinel/eq.pass.cpp
@@ -25,37 +25,37 @@ template <bool Const>
 using MaybeConstIterator = cpp20_input_iterator<std::conditional_t<Const, const int*, int*>>;
 
 template <bool Const>
-class StrictSentinel {
+class CrossConstComparableSentinel {
   using Base = std::conditional_t<Const, const int*, int*>;
   Base base_;
 
 public:
-  StrictSentinel() = default;
-  constexpr explicit StrictSentinel(Base base) : base_(base) {}
+  CrossConstComparableSentinel() = default;
+  constexpr explicit CrossConstComparableSentinel(Base base) : base_(base) {}
 
-  friend constexpr bool operator==(const MaybeConstIterator<Const>& it, const StrictSentinel& se) {
+  friend constexpr bool operator==(const MaybeConstIterator<Const>& it, const CrossConstComparableSentinel& se) {
     return base(it) == se.base_;
   }
 
-  friend constexpr bool operator==(const MaybeConstIterator<!Const>& it, const StrictSentinel& se) {
+  friend constexpr bool operator==(const MaybeConstIterator<!Const>& it, const CrossConstComparableSentinel& se) {
     return base(it) == se.base_;
   }
 };
 
-static_assert(std::sentinel_for<StrictSentinel<true>, MaybeConstIterator<false>>);
-static_assert(std::sentinel_for<StrictSentinel<true>, MaybeConstIterator<true>>);
-static_assert(std::sentinel_for<StrictSentinel<false>, MaybeConstIterator<false>>);
-static_assert(std::sentinel_for<StrictSentinel<false>, MaybeConstIterator<true>>);
+static_assert(std::sentinel_for<CrossConstComparableSentinel<true>, MaybeConstIterator<false>>);
+static_assert(std::sentinel_for<CrossConstComparableSentinel<true>, MaybeConstIterator<true>>);
+static_assert(std::sentinel_for<CrossConstComparableSentinel<false>, MaybeConstIterator<false>>);
+static_assert(std::sentinel_for<CrossConstComparableSentinel<false>, MaybeConstIterator<true>>);
 
 struct CrossConstComparableView : std::ranges::view_base {
   template <std::size_t N>
   constexpr explicit CrossConstComparableView(int (&arr)[N]) : b_(arr), e_(arr + N) {}
 
   constexpr MaybeConstIterator<false> begin() { return MaybeConstIterator<false>{b_}; }
-  constexpr StrictSentinel<false> end() { return StrictSentinel<false>{e_}; }
+  constexpr CrossConstComparableSentinel<false> end() { return CrossConstComparableSentinel<false>{e_}; }
 
   constexpr MaybeConstIterator<true> begin() const { return MaybeConstIterator<true>{b_}; }
-  constexpr StrictSentinel<true> end() const { return StrictSentinel<true>{e_}; }
+  constexpr CrossConstComparableSentinel<true> end() const { return CrossConstComparableSentinel<true>{e_}; }
 
 private:
   int* b_;

>From dd074d476a3008cdd975858f5a3529a47a54e1b4 Mon Sep 17 00:00:00 2001
From: Jakub Mazurkiewicz <mazkuba3 at gmail.com>
Date: Thu, 7 Dec 2023 16:10:19 +0100
Subject: [PATCH 9/9] Use `"test_comparisons.h"` header

---
 .../range.take.sentinel/eq.pass.cpp           | 76 ++++++-------------
 libcxx/test/support/test_comparisons.h        |  2 +-
 2 files changed, 24 insertions(+), 54 deletions(-)

diff --git a/libcxx/test/std/ranges/range.adaptors/range.take/range.take.sentinel/eq.pass.cpp b/libcxx/test/std/ranges/range.adaptors/range.take/range.take.sentinel/eq.pass.cpp
index 9353d65944a23..e6f433e30f60d 100644
--- a/libcxx/test/std/ranges/range.adaptors/range.take/range.take.sentinel/eq.pass.cpp
+++ b/libcxx/test/std/ranges/range.adaptors/range.take/range.take.sentinel/eq.pass.cpp
@@ -19,6 +19,7 @@
 #include <type_traits>
 #include <utility>
 
+#include "test_comparisons.h"
 #include "test_iterators.h"
 
 template <bool Const>
@@ -85,73 +86,42 @@ concept weakly_equality_comparable_with = requires(const T& t, const U& u) {
 };
 
 constexpr bool test() {
-  int buffer[8] = {1, 2, 3, 4, 5, 6, 7, 8};
+  int buffer[8]                      = {1, 2, 3, 4, 5, 6, 7, 8};
+  using CrossConstComparableTakeView = std::ranges::take_view<CrossConstComparableView>;
 
   {   // Compare CI<Const> with sentinel<Const>
     { // Const == true
-      const std::ranges::take_view<CrossConstComparableView> tv(CrossConstComparableView{buffer}, 4);
-      std::same_as<bool> decltype(auto) b1 = (tv.end() == std::ranges::next(tv.begin(), 4));
-      assert(b1);
-      std::same_as<bool> decltype(auto) b2 = (std::ranges::next(tv.begin(), 4) == tv.end());
-      assert(b2);
-      std::same_as<bool> decltype(auto) b3 = (tv.end() != tv.begin());
-      assert(b3);
-      std::same_as<bool> decltype(auto) b4 = (tv.begin() != tv.end());
-      assert(b4);
-      std::same_as<bool> decltype(auto) b5 = (std::ranges::next(tv.begin(), 1) == tv.end());
-      assert(!b5);
-      std::same_as<bool> decltype(auto) b6 = (std::ranges::next(tv.begin(), 4) != tv.end());
-      assert(!b6);
+      AssertEqualityReturnBool<std::ranges::iterator_t<const CrossConstComparableTakeView>,
+                               std::ranges::sentinel_t<const CrossConstComparableTakeView>>();
+      const CrossConstComparableTakeView tv(CrossConstComparableView{buffer}, 4);
+      assert(testEquality(std::ranges::next(tv.begin(), 4), tv.end(), true));
+      assert(testEquality(tv.begin(), tv.end(), false));
     }
 
     { // Const == false
-      std::ranges::take_view<CrossConstComparableView> tv(CrossConstComparableView{buffer}, 4);
-      std::same_as<bool> decltype(auto) b1 = (tv.end() == std::ranges::next(tv.begin(), 4));
-      assert(b1);
-      std::same_as<bool> decltype(auto) b2 = (std::ranges::next(tv.begin(), 4) == tv.end());
-      assert(b2);
-      std::same_as<bool> decltype(auto) b3 = (tv.end() != tv.begin());
-      assert(b3);
-      std::same_as<bool> decltype(auto) b4 = (tv.begin() != tv.end());
-      assert(b4);
-      std::same_as<bool> decltype(auto) b5 = (std::ranges::next(tv.begin(), 2) == tv.end());
-      assert(!b5);
-      std::same_as<bool> decltype(auto) b6 = (std::ranges::next(tv.begin(), 4) != tv.end());
-      assert(!b6);
+      AssertEqualityReturnBool<std::ranges::iterator_t<CrossConstComparableTakeView>,
+                               std::ranges::sentinel_t<CrossConstComparableTakeView>>();
+      CrossConstComparableTakeView tv(CrossConstComparableView{buffer}, 4);
+      assert(testEquality(std::ranges::next(tv.begin(), 4), tv.end(), true));
+      assert(testEquality(std::ranges::next(tv.begin(), 1), tv.end(), false));
     }
   }
 
   {   // Compare CI<Const> with sentinel<!Const>
     { // Const == true
-      std::ranges::take_view<CrossConstComparableView> tv(CrossConstComparableView{buffer}, 4);
-      std::same_as<bool> decltype(auto) b1 = (tv.end() == std::ranges::next(std::as_const(tv).begin(), 4));
-      assert(b1);
-      std::same_as<bool> decltype(auto) b2 = (std::ranges::next(std::as_const(tv).begin(), 4) == tv.end());
-      assert(b2);
-      std::same_as<bool> decltype(auto) b3 = (tv.end() != std::as_const(tv).begin());
-      assert(b3);
-      std::same_as<bool> decltype(auto) b4 = (std::as_const(tv).begin() != tv.end());
-      assert(b4);
-      std::same_as<bool> decltype(auto) b5 = (std::ranges::next(std::as_const(tv).begin(), 1) == tv.end());
-      assert(!b5);
-      std::same_as<bool> decltype(auto) b6 = (std::ranges::next(std::as_const(tv).begin(), 4) != tv.end());
-      assert(!b6);
+      AssertEqualityReturnBool<std::ranges::iterator_t<const CrossConstComparableTakeView>,
+                               std::ranges::sentinel_t<CrossConstComparableTakeView>>();
+      CrossConstComparableTakeView tv(CrossConstComparableView{buffer}, 4);
+      assert(testEquality(std::ranges::next(std::as_const(tv).begin(), 4), tv.end(), true));
+      assert(testEquality(std::ranges::next(std::as_const(tv).begin(), 2), tv.end(), false));
     }
 
     { // Const == false
-      std::ranges::take_view<CrossConstComparableView> tv(CrossConstComparableView{buffer}, 4);
-      std::same_as<bool> decltype(auto) b1 = (std::as_const(tv).end() == std::ranges::next(tv.begin(), 4));
-      assert(b1);
-      std::same_as<bool> decltype(auto) b2 = (std::ranges::next(tv.begin(), 4) == std::as_const(tv).end());
-      assert(b2);
-      std::same_as<bool> decltype(auto) b3 = (std::as_const(tv).end() != tv.begin());
-      assert(b3);
-      std::same_as<bool> decltype(auto) b4 = (tv.begin() != std::as_const(tv).end());
-      assert(b4);
-      std::same_as<bool> decltype(auto) b5 = (std::ranges::next(tv.begin(), 2) == std::as_const(tv).end());
-      assert(!b5);
-      std::same_as<bool> decltype(auto) b6 = (std::ranges::next(tv.begin(), 4) != std::as_const(tv).end());
-      assert(!b6);
+      AssertEqualityReturnBool<std::ranges::iterator_t<CrossConstComparableTakeView>,
+                               std::ranges::sentinel_t<const CrossConstComparableTakeView>>();
+      CrossConstComparableTakeView tv(CrossConstComparableView{buffer}, 4);
+      assert(testEquality(std::ranges::next(tv.begin(), 4), std::as_const(tv).end(), true));
+      assert(testEquality(std::ranges::next(tv.begin(), 3), std::as_const(tv).end(), false));
     }
   }
 
diff --git a/libcxx/test/support/test_comparisons.h b/libcxx/test/support/test_comparisons.h
index e006f69f8bf67..b01c7fe0d6c05 100644
--- a/libcxx/test/support/test_comparisons.h
+++ b/libcxx/test/support/test_comparisons.h
@@ -213,7 +213,7 @@ void AssertEqualityAreNoexcept()
 }
 
 template <class T, class U = T>
-void AssertEqualityReturnBool()
+constexpr void AssertEqualityReturnBool()
 {
     ASSERT_SAME_TYPE(decltype(std::declval<const T&>() == std::declval<const U&>()), bool);
     ASSERT_SAME_TYPE(decltype(std::declval<const T&>() != std::declval<const U&>()), bool);



More information about the libcxx-commits mailing list