[libcxx-commits] [PATCH] D100269: [libcxx][ranges] adds `ranges::range`, `ranges::common_range`, and range aliases

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

Quuxplusone added inline comments.


================
Comment at: libcxx/include/__ranges/concepts.h:38
+
+  // `iterator_t` defined in <__ranges/begin.h>
+
----------------
cjdb wrote:
> ldionne wrote:
> > That's kinda weird, would it be possible to define it here instead?
> Your options are:
>   * it lives here exclusively, `__ranges/access.h` (`__ranges/end.h` in current D100255) needs to use `decltype(ranges::begin(declval<_Rp&>()))` wherever it currently uses `iterator_t`
>   * it lives in `__ranges/access.h` exclusively, comment remains
>   * it lives in both
FWIW, I'd look at the definitions on cppreference:
- `iterator_t` is the type of `ranges::begin`, so it belongs next to `begin`
- `sentinel_t` is the type of `ranges::end`, so it belongs next to `end`
- `range_size_t`is the type of `ranges::size`, so it belongs next to `size`
The other four typedefs probably might as well go into `<ranges>` until we have a reason to move them elsewhere.


================
Comment at: libcxx/include/ranges:37-38
     using iterator_t = decltype(ranges::begin(declval<T&>()));
+  template<class T>
+    using iterator_t = decltype(ranges::begin(declval<T&>()));
+  template<range R>
----------------
This is listed twice.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100269