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

[Hui via Phabricator via libcxx-commits]

huixie90 added inline comments.


================
Comment at: libcxx/include/__ranges/all.h:42
       noexcept(noexcept(_LIBCPP_AUTO_CAST(std::forward<_Tp>(__t))))
+      -> decltype(_LIBCPP_AUTO_CAST(std::forward<_Tp>(__t)))
     {
----------------
EricWF wrote:
> huixie90 wrote:
> > 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
> Oh, based on the name I had assumed it did something entirely different. Like forward<T&&>(t), but without the need to write the type.
> Maybe it needs a better name.
> 
> Also, this is a lesson in using `auto` return types. I think it's clear that:
> 
> 1. We shouldn't use them in any place the standard doesn't mandate.
> 1.  We really shouldn't use them in any place we also use `noexcept(noexcept(...))`, because it can poison an entire overload set as your discovering.
yeah, i was expecting a `__decay_copy`  function but it turned out to have this macro. I guess the name was inspired by the C++23 `auto(x)` syntax which is almost the same as this macro except when `x` is a prvalue, this macro will  do an  extra move.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128281