Skip to content

Temporary sender in transform_sender may dangle #1402

Open

Description

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

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions