[libcxx-commits] [PATCH] D100271: [libcxx][iterator][ranges] adds `input_iterator` and `input_range`

Mon Apr 26 09:57:12 PDT 2021

Quuxplusone requested changes to this revision.
Quuxplusone added inline comments.
This revision now requires changes to proceed.

================
Comment at: libcxx/test/support/test_iterators.h:638
+template <class I>
+struct cxx20_input_iterator {
+  using value_type = std::iter_value_t<I>;
----------------
cjdb wrote:
> zoecarver wrote:
> > Side note: I know we sort of already started down the road of `cxx20_` naming, but part of me doesn't really like this, because hopefully (and probably) this will just be "The Future of Iterators" so it would be great if we could name the other iterators "legacy" or whatever and make these new iterators seem as though they aren't the alternative. I don't know if this is feasible, or worth of the time it might cost, though. 
> D101242 takes care of renaming all the existing iterators. `cxx20_` is to make it unambiguous for non-ranges contributors that it's different (this optimises for the reader).
Actually, what you should do in `test_iterators.h` is simply make sure that our existing `input_iterator` is a `std::input_iterator`; that our existing `forward_iterator` is a `std::forward_iterator`; etc. If they don't conform to the C++20 concepts, then //that// is a bug worth fixing. (But I believe they do conform.)

If our existing `test_iterators.h` already provides C++20-compatible iterators (which, again, //is// the intention — and I recently even added a C++20 `contiguous_iterator` to this file!), then there's nothing to do here.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100271