[libc++][test] Move `common_input_iterator` to `test_iterators.h`

[jloser]

Summary

There's a bit of duplication in use of common_input_iterator as noted in
https://reviews.llvm.org/D141216. Lift common_input_iterator into
test_iterators.h.

[philnik]

Thanks for the cleanup! LGTM with comment applied and green CI.

libcxx/test/support/test_iterators.h

700

[jloser]

Rebase and use >= 20 instead of > 17

[jloser]

libcxx/test/support/test_iterators.h

700

Just switched to using >= 20. Did we decide on using >= always going forward? I thought there was a discussion within the last 6 months or so, but I forget and am used to the old-school way.

[jloser]

@philnik mind if I land this? CI isn't green yet due to unrelated issues.

1. https://reviews.llvm.org/harbormaster/unit/view/5710279/ is fixed with https://reviews.llvm.org/D141284

1. https://reviews.llvm.org/harbormaster/unit/view/5710280/ is a separate issue — module map problem?

[philnik]

In D141238#4036364, @jloser wrote:

@philnik mind if I land this? CI isn't green yet due to unrelated issues.

1. https://reviews.llvm.org/harbormaster/unit/view/5710279/ is fixed with https://reviews.llvm.org/D141284

1. https://reviews.llvm.org/harbormaster/unit/view/5710280/ is a separate issue — module map problem?

Given that I've found a small issue I think it would be better to rebase and have a green CI. The second failure is also a problem with includes. I'll fix it myself.

libcxx/test/support/test_iterators.h

700

Yes, we decided to use >=. Not sure when exactly it was, but I think was within this release cycle.

720

@huixie90 This is different in the two versions we have. Is std::iter_reference_t<Base> correct here, or should we just use decltype(auto)?

[jloser]

Rebase

[jloser]

libcxx/test/support/test_iterators.h

720

Thanks for calling this out explicitly. I noticed this and stuck with std::iter_reference_t<Base> rather than decltype(auto) that @CaseyCarter fixed in https://reviews.llvm.org/D141216. I think using iter_reference_t is fine, but I'll let others chime in here.

[jloser]

Use decltype(auto) in operator* in case the base type is const qualified

libcxx/test/support/test_iterators.h

720

Actually, based on the rationale in https://reviews.llvm.org/D141216, it seems the problem is when the template type parameter Base is const-qualified, we'd actually need to use std::iter_const_reference_t and not std::iter_reference_t. Given that, it's easier to just always use decltype(auto) and let the deduction happen for us.

But I'm not sure we have a call site or test mandating that behavior, but we should change it to use decltype(auto) AFAICT now. Is that right, @CaseyCarter @huixie90? In any case, I just changed it to use decltype(auto).

[huixie90]

LGTM with green CI

libcxx/test/support/test_iterators.h

720

Yeah. decltype(auto) looks good to me. It should always be iter_reference_t though. The problem in D141216 was I mistakenly hard coded it to be int*. iter_const_reference_t is just a different thing (with slightly confusing name)

[jloser]

Rebase in quest for the green CI

[jloser]

I landed this as the CI failures are unrelated

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

constexpr Sent end() const {
     if constexpr (std::is_same_v<Iter, Sent>) {
       return Iter(this->data_ + this->size_);
     } else {
       return Sent(Iter(this->data_ + this->size_));
     }
   }
 };
 
-// an `input_iterator` that can be used in a `common_range`
-template <class Base>
-struct common_input_iterator {
-  Base it_;
-
-  using value_type = std::iter_value_t<Base>;
-  using difference_type = std::intptr_t;
-  using iterator_concept = std::input_iterator_tag;
-
-  constexpr common_input_iterator() = default;
-  constexpr explicit common_input_iterator(Base it) : it_(it) {}
-
-  constexpr common_input_iterator& operator++() {
-    ++it_;
-    return *this;
-  }
-  constexpr void operator++(int) { ++it_; }
-
-  constexpr std::iter_reference_t<Base> operator*() const { return *it_; }
-
-  friend constexpr bool operator==(common_input_iterator const&, common_input_iterator const&) = default;
-};
-
 using InputCommonInner = BufferView<common_input_iterator<int*>>;
 static_assert(std::ranges::input_range<InputCommonInner>);
 static_assert(!std::ranges::forward_range<InputCommonInner>);
 static_assert(std::ranges::common_range<InputCommonInner>);
 
 using InputNonCommonInner = BufferView<common_input_iterator<int*>, sentinel_wrapper<common_input_iterator<int*>>>;
 static_assert(std::ranges::input_range<InputNonCommonInner>);
 static_assert(!std::ranges::forward_range<InputNonCommonInner>);

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

 static_assert(!std::ranges::sized_range<ContiguousNonCommonView>);
 
 using ContiguousNonCommonSized = BasicView<int*, sized_sentinel<int*>>;
 
 static_assert(std::ranges::contiguous_range<ContiguousNonCommonSized>);
 static_assert(!std::ranges::common_range<ContiguousNonCommonSized>);
 static_assert(std::ranges::sized_range<ContiguousNonCommonSized>);
 
-template <class Base = int*>
-struct common_input_iterator {
-  Base it_;
-
-  using value_type = int;
-  using difference_type = std::intptr_t;
-  using iterator_concept = std::input_iterator_tag;
-
-  constexpr common_input_iterator() = default;
-  constexpr explicit common_input_iterator(Base it) : it_(it) {}
-
-  constexpr common_input_iterator& operator++() {
-    ++it_;
-    return *this;
-  }
-  constexpr void operator++(int) { ++it_; }
-
-  constexpr decltype(auto) operator*() const { return *it_; }
-
-  friend constexpr bool operator==(common_input_iterator const&, common_input_iterator const&) = default;
-};
-
-using InputCommonView = BasicView<common_input_iterator<>>;
+using InputCommonView = BasicView<common_input_iterator<int*>>;
 static_assert(std::ranges::input_range<InputCommonView>);
 static_assert(!std::ranges::forward_range<InputCommonView>);
 static_assert(std::ranges::common_range<InputCommonView>);
 LIBCPP_STATIC_ASSERT(std::ranges::__simple_view<InputCommonView>);
 
 using NonSimpleInputCommonView = BasicView<common_input_iterator<const int*>, common_input_iterator<const int*>,
                                            common_input_iterator<int*>, common_input_iterator<int*>>;
 static_assert(std::ranges::input_range<NonSimpleInputCommonView>);
 static_assert(!std::ranges::forward_range<NonSimpleInputCommonView>);
 static_assert(std::ranges::common_range<NonSimpleInputCommonView>);
 LIBCPP_STATIC_ASSERT(!std::ranges::__simple_view<NonSimpleInputCommonView>);
 
-using InputNonCommonView = BasicView<common_input_iterator<>, sentinel_wrapper<common_input_iterator<>>>;
+using InputNonCommonView = BasicView<common_input_iterator<int*>, sentinel_wrapper<common_input_iterator<int*>>>;
 static_assert(std::ranges::input_range<InputNonCommonView>);
 static_assert(!std::ranges::forward_range<InputNonCommonView>);
 static_assert(!std::ranges::common_range<InputNonCommonView>);
 LIBCPP_STATIC_ASSERT(std::ranges::__simple_view<InputNonCommonView>);
 
 using NonSimpleInputNonCommonView =
     BasicView<common_input_iterator<const int*>, sentinel_wrapper<common_input_iterator<const int*>>,
               common_input_iterator<int*>, sentinel_wrapper<common_input_iterator<int*>>>;

libcxx/test/support/test_iterators.h

public:
   template <class T>
   void operator,(T const&) = delete;
 };
 template <class It>
 cpp20_output_iterator(It) -> cpp20_output_iterator<It>;
 
 static_assert(std::output_iterator<cpp20_output_iterator<int*>, int>);
 
+#  if TEST_STD_VER >= 20
+
+// An `input_iterator` that can be used in a `std::ranges::common_range`
+template <class Base>
+struct common_input_iterator {
+  Base it_;
+
+  using value_type       = std::iter_value_t<Base>;
+  using difference_type  = std::intptr_t;
+  using iterator_concept = std::input_iterator_tag;
+
+  constexpr common_input_iterator() = default;
+  constexpr explicit common_input_iterator(Base it) : it_(it) {}
+
+  constexpr common_input_iterator& operator++() {
+    ++it_;
+    return *this;
+  }
+  constexpr void operator++(int) { ++it_; }
+
+  constexpr decltype(auto) operator*() const { return *it_; }
+
+  friend constexpr bool operator==(common_input_iterator const&, common_input_iterator const&) = default;
+};
+
+#  endif // TEST_STD_VER >= 20
+
 // Iterator adaptor that counts the number of times the iterator has had a successor/predecessor
 // operation called. Has two recorders:
 // * `stride_count`, which records the total number of calls to an op++, op--, op+=, or op-=.
 // * `stride_displacement`, which records the displacement of the calls. This means that both
 //   op++/op+= will increase the displacement counter by 1, and op--/op-= will decrease the
 //   displacement counter by 1.
 template <class It>
 class stride_counting_iterator {