libunifex icon indicating copy to clipboard operation
libunifex copied to clipboard

Reference value types and when_all

Open apmccartney opened this issue 3 years ago • 1 comments

Current behavior

when_all decays the value types from upstream operations to determine it's storage.

Expected behavior

Reference values passed by upstream to be "stored" by reference_wrapper, using pointers, or the like, and unwrapped prior to being passed to downstream operations.

Why is this a problem?

That means that lvalue references sent from upstream operations are copy constructed into storage.

Consequently,

  1. surprising (and potentially expensive) copies are made
  2. senders of references to non-copyable types cannot be used with when_all.

Minimum Reproducing Example

Compiler explorer link: https://godbolt.org/z/Wacxf3

#include  <unifex/just.hpp>
#include  <unifex/transform.hpp>
#include  <unifex/when_all.hpp>

#include <iostream>

int main() {
  struct foo {
    foo() = default;
    foo(const foo&) = delete;
  };
  
  struct receiver {
    
    void set_value(foo&) noexcept { std::cout << "success" << std::endl; }

    [[noreturn]] void set_error(std::exception_ptr) noexcept {
      std::terminate();
    }

    [[noreturn]] void set_done() noexcept {
      std::terminate();
    }
  };

  foo instance;

  auto s = unifex::when_all(unifex::just(&instance) | unifex::transform([](foo* ptr) -> foo& { return *ptr; }));
  auto r = receiver{};

  auto os = unifex::connect(std::move(s), std::move(r));
  unifex::start(os);
};

apmccartney avatar Feb 23 '21 13:02 apmccartney

Yes, I can see how this would be problematic for some use-cases.

We can probably be somewhat less aggressive at decaying lvalues in the when_all algorithm.

There can be difficulties trying to apply different semantics to xvalue and prvalue value_types though, and there can even be ambiguities here if a sender can send either an xvalue or a prvalue of the same type as we can't tell which overload of set_value() was intended to be called - they will both resolve to the same overload of set_value.

If a sender sends an xvalue result then we don't necessarily know whether it is safe to store that result as a reference or not. It may only be a temporary passed to set_value() and may not remain alive until the end of the when_all() operation. So I think we'd still need to decay-copy xvalue results, but it should be possible to avoid decay-copying lvalue results in when_all().

A temporary workaround for now would be to wrap the lvalue reference result in std::ref() to return a reference_wrapper.

e.g. I think it might work if you change the return *ptr; to return std::ref(*ptr);

lewissbaker avatar Mar 25 '21 23:03 lewissbaker