Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Temporary sender in transform_sender may dangle #1402

Open
jiixyj opened this issue Aug 24, 2024 · 0 comments
Open

Temporary sender in transform_sender may dangle #1402

jiixyj opened this issue Aug 24, 2024 · 0 comments

Comments

@jiixyj
Copy link

jiixyj commented Aug 24, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant