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

Hui via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Tue Jun 21 11:55:40 PDT 2022

huixie90 marked 3 inline comments as done.
huixie90 added inline comments.

Comment at: libcxx/include/__ranges/all.h:42
+      -> decltype(_LIBCPP_AUTO_CAST(std::forward<_Tp>(__t)))
EricWF wrote:
> var-const wrote:
> > I wonder if we have any other cases where we don't write the return type 3 times and consequently don't get SFINAE.
> Is this a case of "not getting SFINAE" or "auto" return types not deducing to reference types? Does this work if we use `decltype(auto)` instead?
> Also this changes the return type from being by-value to being by-reference, which is the intended change I assume?
This function always return by value because `_LIBCPP_AUTO_CAST` always yield the decayed type

#define _LIBCPP_AUTO_CAST(expr) static_cast<typename decay<decltype((expr))>::type>(expr)

In the spec, it says expression equivalent to `decay-copy(E)`, and  `_LIBCPP_AUTO_CAST` is the libc++ way of implementing `decay-copy`

In any case, the return type should be the same as `decay-copy` (expression equivalent), and this is why `decltype(_LIBCPP_AUTO_CAST(...))` is correct

`decltype(auto)` won't work because `decltype(auto)` doesn't have any effect on SFINAE

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&>);
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

  rG LLVM Github Monorepo



More information about the libcxx-commits mailing list