[libcxx-commits] [PATCH] D128281: [libc++] fix views::all hard error on lvalue move only views instead of SFINAE

[Konstantin Varlamov via Phabricator via libcxx-commits]

var-const added inline comments.


================
Comment at: libcxx/test/std/ranges/range.adaptors/range.all/all.pass.cpp:153
     static_assert(!std::is_invocable_v<decltype(std::views::all), RandomAccessRange, RandomAccessRange>);
+    static_assert(!std::is_invocable_v<decltype(std::views::all), MoveOnlyView&>);
   }
----------------
huixie90 wrote:
> jloser wrote:
> > var-const wrote:
> > > Is it important for the view to be move-only? From the patch description, I got the impression that any reference should fail, but perhaps I'm missing something?
> > The call operator already requires the template type be a view, so an lvalue reference to a view is the simplest case to make it go boom AFAICT.
> "move-only" is important here because `_LIBCPP_AUTO_CAST` (which is used in the all's function body, and noexcept specification) is going to make a copy.
> 
> see definition here
> ```
> #define _LIBCPP_AUTO_CAST(expr) static_cast<typename decay<decltype((expr))>::type>(expr)
> ```
> 
> If we pass an lvalue reference to a copyable `view`, the expression above is just making a copy of the `view`, which is fine. If the `view` is move-only, the above expression is ill-formed and cause `noexcept(decay-copy(v))` go boom
Thanks for explaining! Can you please add a condensed form of this explanation as a comment here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128281