[libcxx-commits] [PATCH] D116894: [libc++] [ranges] Implement P2415R2 owning_view

[Louis Dionne via Phabricator via libcxx-commits]

ldionne added inline comments.


================
Comment at: libcxx/test/std/ranges/range.req/range.refinements/viewable_range.compile.pass.cpp:120
 static_assert(!std::ranges::viewable_range<void>);
 static_assert(!std::ranges::viewable_range<int>);
 static_assert(!std::ranges::viewable_range<int (*)(char)>);
----------------
Quuxplusone wrote:
> ldionne wrote:
> > In the paper, they mention this:
> > 
> > >  Editor's note: `remove_reference_t` rather than `remove_cvref_t` because we need to reject const `vector<int>&&` from being a `viewable_range`
> > 
> > I don't think we have a test that a `Range const&&` isn't a `viewable_range` -- could we add one? In other words, I'd like a test that fails if you had used `movable<remove_cvref_t<_Tp>>` instead of `movable<remove_reference_t<_Tp>>`.
> > 
> My understanding is that this test relies on the ["clever" use of `T` to mean `T&&`](https://reviews.llvm.org/D116991#inline-1119810) that I've been railing against all this week for some reason. ;)
> So when you see `static_assert(!std::ranges::viewable_range<const T6>);` above, that's basically the `const vector<int>&&` case — it's testing that a (range, non-view, const, rvalue) is not a viewable_range.
> 
> However, if you ask me to rewrite all the above `<X>`s to `<X&&>`s, or to add `<X&&>`s in addition to the `<X>`s, I'll be reasonably happy to do so!
I think we should add `X&&` in addition to `X`. That would make it clear that we cover the case I mentioned above (and which is called out explicitly in the paper).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116894