[libcxx-commits] [PATCH] D119620: [libc++][ranges] Implement Ranges changes to `istream{, buf}_iterator`.

[Louis Dionne via Phabricator via libcxx-commits]

ldionne accepted this revision as: ldionne.
ldionne added a comment.

This LGTM. I'm fine with not guarding `default_sentinel_t` under `_LIBCPP_HAS_NO_CONCEPTS`, since (1) that macro will soon die and (2) it technically doesn't need concepts.

Leaving final approval to Arthur since he had comments.



================
Comment at: libcxx/test/std/iterators/stream.iterators/istream.iterator/istream.iterator.cons/default.pass.cpp:57
+    {
+    typedef std::istream_iterator<int> T;
+
----------------
var-const wrote:
> Because these constructors (default constructor and `default_sentinel_t` constructor) are defined in the same clause, I'm leaning towards testing them in the same test file.
I have a very slight preference for different files since they are really different constructors (except they happen to have the same effect). But definitely not a blocking comment.


================
Comment at: libcxx/test/std/iterators/stream.iterators/istream.iterator/istream.iterator.ops/equal.pass.cpp:11
 
 // class istream_iterator
 
----------------
If you add the test here (I'd weakly prefer another file), please update the synopsis. This applies in a couple other tests too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119620