[libcxx-commits] [PATCH] D100275: [libcxx] adds `forward_iterator` and `forward_range`

[Christopher Di Bella via Phabricator via libcxx-commits]

cjdb added inline comments.


================
Comment at: libcxx/test/std/iterators/iterator.requirements/iterator.concepts/iterator.concept.forward/forward_iterator.compile.pass.cpp:156
+
+static_assert(!std::forward_iterator<noncopyable_input_iterator>);
+
----------------
miscco wrote:
> Feel free to disregard: That line gets drowned a bit. Should we add a banner above it? 
Nice observation. I'll move this test (and the one below) above the `namespace standard_types` the next time I visit this commit (too busy to rebase this instant).


================
Comment at: libcxx/test/std/iterators/iterator.requirements/iterator.concepts/iterator.concept.forward/forward_iterator.compile.pass.cpp:158
+
+struct not_input_iterator {
+  using difference_type = std::ptrdiff_t;
----------------
miscco wrote:
> I do not really like that name. We should say *why* it is not a input iterator without checking all the methods.
> 
> I think here it is `not_eq_comparable_iterator`. Ditto my wish for porting the iterator test machinery
> 
> 
> I do not really like that name. We should say *why* it is not a input iterator without checking all the methods.

The salient feature here isn't why the type is not an input iterator: only that it isn't an input iterator (since all forward iterators are input iterators, a non-input iterator cannot be a forward iterator). Either way, I consider it important to assert that `input_iterator` isn't satisfied, to document that this case has been checked.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100275