[libcxx-commits] [PATCH] D103493: [libcxx][ranges] Add concepts in range.utility.helpers.

[Zoe Carver via Phabricator via libcxx-commits]

zoecarver added inline comments.


================
Comment at: libcxx/test/libcxx/ranges/range.utility.helpers/simple_view.compile.pass.cpp:29-32
+struct NoConstView : std::ranges::view_base {
+  friend int* begin(NoConstView&);
+  friend int* end(NoConstView&);
+};
----------------
Quuxplusone wrote:
> zoecarver wrote:
> > Quuxplusone wrote:
> > > I suggest you add a positive example `OnlyConstView`, with only `friend const int* begin(const OnlyConstView&)`.
> > > And then you can eliminate lines 40 and 42 from `DifferentSentinel`.
> > This isn't a view. A view requires that T is movable (i.e., not const) and that we can do `ranges::begin(declval<T&>())`. Therefor, we need all four members.
> @zoecarver: I believe you've misunderstood. I'm asking that you add this test case:
> ```
> struct OnlyConstView : std::ranges::view_base {
>   friend int* begin(const OnlyConstView&);
>   friend int* end(const OnlyConstView&);
> };
> static_assert(std::ranges::__simple_view<OnlyConstView>);
> ```
> And then modify `DifferentSentinel` as follows:
> ```
> struct DifferentSentinel : std::ranges::view_base {
>   friend int* begin(DifferentSentinel const&);
>   friend sentinel_wrapper<int*> end(DifferentSentinel const&);
> };
> static_assert(!std::ranges::__simple_view<DifferentSentinel>);
> ```
No, I understood you. Maybe the following snippet will help me explain what I'm trying to say:
```
struct OnlyConstView : std::ranges::view_base {
  friend int* begin(const OnlyConstView&);
  friend int* end(const OnlyConstView&);
};
static_assert(!std::ranges::range<OnlyConstView>);
static_assert( std::ranges::range<const OnlyConstView>);
static_assert(!std::ranges::view<OnlyConstView>);
static_assert(!std::ranges::view<const OnlyConstView>);
static_assert(!std::ranges::__simple_view<OnlyConstView>);
static_assert(!std::ranges::__simple_view<const OnlyConstView>);
```

Here, you can see that only `const OnlyConstView` is a `range`. This is because a range requires us to be able to do `ranges::begin(declval<T&>())` (so `T` must be const). You'll also notice that in no case is this a view, because a view requires `T` to be movable, so it cannot be const. And `__simple_view` requires `T` to be a view, so this ins't a `__simple_view` either. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103493