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

[Louis Dionne via Phabricator via libcxx-commits]

ldionne added inline comments.


================
Comment at: libcxx/include/__ranges/concepts.h:38
+
+  // `iterator_t` defined in <__ranges/begin.h>
+
----------------
cjdb wrote:
> Quuxplusone wrote:
> > 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.
> > FWIW, I'd look at the definitions on cppreference:
> 
> Having written a fair chunk of the ranges section of cppreference, I can confidently say it's designed for consumers of C++, not for implementers, so different groupings make sense.
> 
> > * `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`
> 
> This is a good suggestion (even though cppreference doesn't group them like this), but I'd prefer to group them as per the standard's synopsis.
> 
> > The other four typedefs probably might as well go into `<ranges>` until we have a reason to move them elsewhere.
> 
> That would make using them in `<ranges/*>` very awkward.
Hmm, I think I buy Arthur's argument. Either do that or leave as-is, non blocking as far as I'm concerned. Thanks for explaining why I can't just have what I asked for in the first place, though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100269