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

[Louis Dionne via Phabricator via libcxx-commits]

ldionne requested changes to this revision.
ldionne added a comment.

This looks OK to me apart from the `input_iterator` archetype naming.



================
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:
> Quuxplusone wrote:
> > 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.
> Actually, this is an important new test type for algorithms, iterator adaptors, and range adaptors.
I'm talking to Chris offline about this, but what I'd want:

`legacy_input_iterator` or `cpp17_input_iterator` => archetype for the "old" `InputIterator` concept, which includes copyability
`input_iterator` => archetype for the new, C++20 `std::input_iterator` concept, without copyability

Since the other iterators don't change in any meaningful way, we shouldn't introduce new names for those.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100271