seastar
seastar copied to clipboard
scylladb
Make `.then(e)` preserve the value category of `e`?
This code does not compile:
auto f = std::bind_front([] (std::unique_ptr<int>) { return make_ready_future<>(); }, std::make_unique<int>(0));
(void)make_ready_future<>().then(std::move(f));
because then(std::move(f)) tries to call f(), and the expression f() is ill-formed; it tries to copy unique_ptr from the bind_front object into the lambda parameter, but unique_ptr does not have a copy constructor.
Instead, then should preserve the value category of the passed-in function object when calling it. In our example, it should do the equivalent of std::move(f)() (note: std::move(f)() causes the unique_ptr to be moved).
"preserve the value category" means future::then should behave differently depending on the value category of its parameter (i.e. also remember if it was passed an ordinary reference). I think that's too much complication for an already complicated interface, also one that is being discouraged in most places in favor of coroutines.
I agree your example should compile. I think we can unconditionally call Func::operator()() &&. The func object is owned by future::then, and won't be used again, so it's fine to std::move() it before calling its operator()().
@avikivity
"preserve the value category" means future::then should behave differently depending on the value category of its parameter (i.e. also remember if it was passed an ordinary reference). I think that's too much complication for an already complicated interface (...).
In my opinion, it's the opposite. Changing the value category is what introduces complexity. Indeed, if we preserve the value category, then would call the function as if the user of then called it directly. So if the user decides to use std::move(f), then does the same. If the user wants f, then does the same. I think right now the behavior of then is surprising, it does not preserve the intention of the user of then.
The func object is owned by future::then, and won't be used again, so it's fine to std::move() it before calling its operator()().
I don't think it's true. There's nothing that prevents a function object from being used after then, or even suggests that it shouldn't be used. Indeed, then takes a forwarding reference, so it's possible to pass both lvalues and rvalues. If we wanted, we could restrict the interface to take rvalue references only, but we didn't. And suppose I have a stateless function object, with pure operator(). Why should it be illegal to use it in multiple then calls? Consider:
future<> test() {
struct func {
future<> operator()() {
return make_ready_future<>();
}
};
func f{};
return make_ready_future<>().then(f).then(std::move(f));
}
It should indeed be illegal to use f again after the last then because I passed an rvalue-reference to it to then. But I don't see a reason why it would be illegal to use it after the first then (btw. please ignore the use-after-free, I could fix the example but let's keep it simple).
Alternative approach would be to restrict then to take rvalue references only. That would be consistent with co_await, because there's no way to reuse the continuation after co_await.
I think restricting to rvalues makes the most sense (and by restricting, I mean having a deprecation period where lvalues still "work", but emit a warning).
By far the most common usage of future::then() is to pass rvalues. I think it's wrong to elaborate the API to handle the very uncommon lvalue use case. If someone wants to pass lvalues, that's absolutely okay, but they can annotate them with std::ref() as a way of saying "I understand that I am now responsible for the lifetime of this object".
Should we start preserving the value category during the deprecation period?
Or should we instantly switch to using std::move everywhere - and always call the rvalue-reference version of operator() if there is one?
About which operator() should we call - I don't have an intuition. I suppose calling operator()(...) && will decay to operator()(...) & or operator()(...) const & if just those are provided, so it won't break anything.