[libcxx-commits] [PATCH] D103341: [libcxx][nfc] Cleanup cpp20_input_iterator.

[Christopher Di Bella via Phabricator via libcxx-commits]

cjdb added a comment.

The suggested changes from D102020 <https://reviews.llvm.org/D102020> don't properly consider the design of this type, nor its surrounding ecosystem. Please do not merge it, and instead use the appropriate helper types that are already in `test_iterators.h`.



================
Comment at: libcxx/test/support/test_iterators.h:636-637
 {   return !a.operator==(b); }
 
-#ifdef TEST_SUPPORTS_RANGES
-
-// clang-format off
+#if TEST_STD_VER >= 14
 
----------------
Why? This is a move-only iterator, which only makes sense in ranges land.


================
Comment at: libcxx/test/support/test_iterators.h:641-642
 struct cpp20_input_iterator {
-  using value_type = std::iter_value_t<I>;
-  using difference_type = std::iter_difference_t<I>;
+  using value_type = typename std::iterator_traits<I>::value_type;
+  using difference_type = typename std::iterator_traits<I>::difference_type;
   using iterator_concept = std::input_iterator_tag;
----------------
Why?


================
Comment at: libcxx/test/support/test_iterators.h:663
+  constexpr I const& base() const& { return base_; }
+  constexpr I base() && { return std::move(base_); }
 
----------------
This overload needs to keep its `[[nodiscard]]`.


================
Comment at: libcxx/test/support/test_iterators.h:665-676
+  struct sentinel {
+    friend cpp20_input_iterator;
+
+    sentinel() = default;
+    sentinel(I base) : base(base) {}
+
+  private:
----------------
Iterators don't have sentinel sub-types. We have a `sentinel_wrapper` for that.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D103341/new/

https://reviews.llvm.org/D103341