[libcxx-commits] [PATCH] D101242: [libcxx][nfc] renames test iterator types to `legacy_*_iterator`

[Arthur O'Dwyer via Phabricator via libcxx-commits]

Quuxplusone added inline comments.


================
Comment at: libcxx/test/support/test_iterators.h:64
 public:
     typedef          std::input_iterator_tag                   iterator_category;
     typedef typename Traits::value_type                        value_type;
----------------
cjdb wrote:
> Quuxplusone wrote:
> > @cjdb wrote:
> > > tl;dr yes. At minimum, the iterator tags are now mandatory.
> > 
> > The existing iterators already have tags, though. So that's not a concern for us.
> > 
> > I suggested to Louis in chat but will put here for the record: If the main concern of this patch is to make sure we have a test iterator which is a "non-comparable input iterator" (comparable only to its sentinel type, but not to itself), then I suggest you leave everything the way it is in main but then add the new C++20-only, sentinel-comparison-only test iterator, like this:
> > ```
> > #if TEST_STD_VER >= 20
> > template<class It>
> > class incomparable_input_iterator {
> >     // This is an input iterator according to C++>=20, but not according to C++<=17.
> >     // It is comparable with its sentinel type, but not with its own type.
> >     [...mostly the same stuff as the regular input iterator test type...]
> > public:
> >     struct sentinel {
> >         constexpr explicit sentinel(It it) : it_(it) {}
> >     };
> >     friend constexpr bool operator==(const incomparable_input_iterator& x, const sentinel& y)
> >         {return x.it_ == y.it_;}
> > };
> > #endif
> > ```
> > This probably belongs right above `contiguous_iterator`, circa line 315, because we intend it to be new in C++20.
> >  If the main concern of this patch is to make sure we have a test iterator which is a "non-comparable input iterator" (comparable only to its sentinel type, but not to itself)
> 
> This isn't the only difference between //Cpp17InputIterator// and `std::input_iterator`.
> 
> > I suggest you leave everything the way it is in main but then add the new C++20-only, sentinel-comparison-only test iterator
> 
> That leaves ambiguity as to whether or not `input_iterator` refers to a //Cpp17InputIterator// or is a strict `std::input_iterator`. It needs to be renamed to clear up that ambiguity.
> This isn't the only difference

Obligatory "What are the differences you care about, then?"
No two things in the world are ever exactly the same. That doesn't mean we make libc++ PRs about it. :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101242