Temporary sender in `transform_sender` may dangle
jiixyj opened this issue · 0 comments
transform_sender
is defined like this:
struct __transform_sender {
template <class _Self = __transform_sender, class _Domain, class _Sender, class... _Env>
STDEXEC_ATTRIBUTE((always_inline))
/*constexpr*/
decltype(auto)
operator()(_Domain __dom, _Sender&& __sndr, const _Env&... __env) const
noexcept(__nothrow_callable<__transform_sender_1, _Domain, _Sender, const _Env&...>) {
using _Sender2 = __call_result_t<__transform_sender_1, _Domain, _Sender, const _Env&...>;
// If the transformation doesn't change the sender's type, then do not
// apply the transform recursively.
if constexpr (__decay_same_as<_Sender, _Sender2>) {
return __transform_sender_1()(__dom, static_cast<_Sender&&>(__sndr), __env...);
} else {
// We transformed the sender and got back a different sender. Transform that one too.
return _Self()(
__dom,
__transform_sender_1()(__dom, static_cast<_Sender&&>(__sndr), __env...),
__env...);
}
}
};
In general it takes care to "perfectly backward" the value category of any custom transform_sender
implementation it calls by using decltype(auto)
.
But I think there is a lifetime bug in the last else
clause. What happens if the return value of __transform_sender_1
is a prvalue? This is given as an argument to the call _Self()(...)
and therefore materializes as an xvalue. Thus, _Self()(...)
possibly returns a xvalue as well, as inside, it cannot distinguish between xvalue and prvalue. But now we are returning a reference to a temporary! Shouldn't the "prvalue-ness" of the return value of __transform_sender_1
be "perfectly backwarded" with something like this:
if constexpr (std::is_reference_v<decltype(__transform_sender_1()(
__dom, static_cast<_Sender&&>(__sndr), __env...))>) {
return _Self()(
__dom,
__transform_sender_1()(__dom, static_cast<_Sender&&>(__sndr), __env...),
__env...);
} else {
return auto(_Self()(
__dom,
__transform_sender_1()(__dom, static_cast<_Sender&&>(__sndr), __env...),
__env...));
}
This is the relevant place in the draft: https://eel.is/c++draft/exec#snd.transform-1
Let transformed-sndr be the expression dom.transform_sender(std::forward(sndr), env...)
if that expression is well-formed; otherwise, default_domain().transform_sender(std::forward(sndr), env...)
Let final-sndr be the expression transformed-sndr if transformed-sndr and sndr have the same type ignoring cv-qualifiers; otherwise, it is the expression transform_sender(dom, transformed-sndr, env...).
I think it should say something like:
Let transformed-sndr be the expression dom.transform_sender(std::forward(sndr), env...)
if that expression is well-formed; otherwise, default_domain().transform_sender(std::forward(sndr), env...)
Let final-sndr be the expression transformed-sndr if transformed-sndr and sndr have the same type ignoring cv-qualifiers; otherwise, it is the expression auto(transform_sender(dom, transformed-sndr, env...)) if transformed-sndr is a prvalue, and transform_sender(dom, transformed-sndr, env...) otherwise.
Also, in the sender adaptors like https://eel.is/c++draft/exec#then-3 for example, it should probably say auto(transform_sender(get-domain-early(sndr), make-sender(then-cpo, f, sndr)))
, right? make-sender
is a prvalue, and this should be preserved.