Hello Gor,
thank you for taking the time to answer my comments.
>> Given that having from_promise is allowing for symmetry with untyped promise, [...]
Well, address() and from_address() is not strictly necessary either, right? It can always be replaced by the following pattern:
void do_something_async(void (*callback) (void *), void *argument);
/* ... */
auto operator await(/* ... */) {
struct awaitable {
void await_suspend(coroutine_handle<> handle) {
_handle = handle;
do_something_async([] (void *argument) {
auto self = static_cast<awaitable *>(argument);
self->_handle.resume();
}, this);
// The awaitable object is alive until the
// await-expression completes, so this is fine.
}
/* ... */
coroutine_handle<> _handle;
}
return awaitable(/* ... */);
}
This pattern is more general than address() / from_address(): It allows to store additional parameters the callback receives in the awaitable object can return them from await_resume. This enables users to implement non-void await regardless of the underlying promise type; something which is not possible with address() / from_address()! Why does the proposal include an explicit address() / from_address() instead of promoting the use of this store-handle-in-awaitable pattern?
I see, this is indeed a much better idea than what I was suggesting.
Let me ask you another question: Why does the proposal _only_ provide type-erased coroutine_handles? A call to resume() / done() / destroy() results in an indirect jump. I agree that the compiler should be able to devirtualize this jump in many cases; I also agree that having the coroutine_handle of the prosal is important. But why don't we do something like the following:
Define a `suspend-point handle` as a type that has the same interface as coroutine_handle but has weaker guarantees: resume() / done() and destroy() must only be called while the underlying coroutine is suspended in exactly the same execution of the await-expression it was constructed for. Add an implicit conversion from each `suspend-point handle` type to coroutine_handle. Pass a `suspend-point handle` instead of a coroutine_handle to await_suspend().
This way we can write await_suspend like this:
template<typename H>
void await_suspend(H handle);
// This is specialized for each await-expression, so that the compiler can
// avoid using indirect calls for resume() / done() / destory() here.
void await_suspend(coroutine_handle<> handle);
// Still works the same as before. Uses the implicit conversion to construct
// a coroutine_handle<>.
I feel like doing this has zero downsides. Sure, it is a code size for speed trade-off but it is opt-in so that await operators for commonly used types like std::future would still be able to use the more conservative coroutine_handle. It has a niche for code where the failure to devirtualize an indirect jump would result in a noticeable performance overhead: Consider using await to read from a fast in-memory cache. The fast-path of such a operation might be a single atomic load-acquire (aka a usual non-atomic load on x86_64) and a indirect call would dominate the run time of the whole operation. Also note that devirtualization is a relatively complex optimization and might not be possible in some cases (e.g. when code in a different translation unit is invoked and the compiler is unable to prove that this code does not change the target of the jump (e.g. by resuming the coroutine and suspending again); or just when compiling in debug mode). Using this technique resume() would be so fast (it would always be inlined and often be completely removed by an optimizing compiler) that one could decide to abolish await_ready.
Note than in order to enable the store-handle-in-awaitable pattern from above together with this technique the handle would need to be passed to operator await (and not to await_suspend). This would allow us to convert existing C-style callbacks to await in a very natural way and without using any indirect jumps or address() magic:
template<typename H>
auto operator await(std::chrono::seconds t, H handle) {
struct awaitable {
awaitable(std::chrono::seconds t, H handle)
: _t(t), _handle(handle) { }
void await_suspend() {
call_in_n_seconds(_t.count(), [] (void *argument) {
auto self = static_cast<awaitable *>(argument);
self->_handle.resume();
}, this);
}
std::chrono::seconds _t;
H _handle;
};
return awaitable(t, handle);
}
Many thanks,
Alexander