Skip to content

Add utilities as_awaitable() and with_awaitable_senders<>#180

Merged
ericniebler merged 10 commits intomainfrom
senders-are-awaitable
Sep 30, 2021
Merged

Add utilities as_awaitable() and with_awaitable_senders<>#180
ericniebler merged 10 commits intomainfrom
senders-are-awaitable

Conversation

@ericniebler
Copy link
Collaborator

This makes it trivial for coroutine type authors to make senders awaitable within their coroutines. It also provides a default implementation of unhandled_done() that makes a done signal behave like an uncatchable exception.


- `execution::set_error(r, e)` is expression-equivalent to `(result_ptr_->emplace<2>(e), continuation_.resume())`.

- `execution::set_done(r)` is expression-equivalent to `continuation_.promise().unhandled_done().resume()`.
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

coroutine_handle::resume is potentially throwing, but it wouldn't seem like there's a way to ensure correct behavior here if we can't resume the coroutine. It would be as if a function failed to return to its caller. Likewise for the set_error case above it.

@lewissbaker am I missing anything? If not, letting the exception hit the noexcept and terminate the program seems like a reasonable thing to do.


- If `sender_traits<remove_cvref_t<S>>::value_types<Tuple, Variant>` would have the form `Variant<Tuple<T>>`, then <code><i>single-sender-value-type</i>&lt;S></code> is an alias for type `T`.

- Otherwise, if `sender_traits<remove_cvref_t<S>>::value_types<Tuple, Variant>` would have the form `Variant<Tuple<>>` or `Variant<>`, then <code><i>single-sender-value-type</i>&lt;S></code> is an alias for type `void`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if the result is Variant<> whether we should specify that the await_resume() method of the returned awaitable will be tagged as [[noreturn]]?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not going to sweat this for now.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Filed as #210

<i>sender-awaitable</i>(S&& s, P& p);
bool await_ready() const noexcept { return false; }
void await_suspend(coro::coroutine_handle&lt;>) noexcept { start(state_); }
value_t await_resume();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If value_types<Tuple, Variant> has form Variant<> then maybe this should be marked [[nodiscard]]?

And if error_types<Variant> has form Variant<> then maybe this should be marked noexcept?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deferring this.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If value_types<Tuple, Variant> has form Variant<> then maybe this should be marked [[nodiscard]]?

@lewissbaker, wouldn't that mean that this awaitable will never produce a value, and so await_resume will never be called? What would be the point of declaring it [[nodiscard]]?

And if error_types<Variant> has form Variant<> then maybe this should be marked noexcept?

Filed as #213

public:
<i>sender-awaitable</i>(S&& s, P& p);
bool await_ready() const noexcept { return false; }
void await_suspend(coro::coroutine_handle&lt;>) noexcept { start(state_); }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am still a little concerned about the potential for unbounded recursion and stack-overflow if someone awaits a sender that completes synchronously in a loop with this formulation.

Maybe this should conditionally schedule itself onto a trampoline_scheduler if the operation may potentially complete synchronously (e.g. if the get_blocking() CPO from P2257 was adopted and calling it returned something other than asynchronous_completion_t).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method should take a coroutine_handle<P> to force that the coroutine-type it is being awaited in has exactly type P.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am still a little concerned about the potential for unbounded recursion and stack-overflow if someone awaits a sender that completes synchronously in a loop with this formulation.

This has to be future work. I can't open that can of worms here.

- <i>Effects:</i> equivalent to:

<pre highlight="c++">
if constexpr (<i>is-awaitable</i>&lt;Value>)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Normally we'd expect to have a call to the await_transform() customisation point here.
Is it guaranteed that we'll be able to add such a customisation in here later without breaking anything?
What's the rationale for not adding in the await_transform() CPO at this point?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The rationale is that an await_transform CPO should really be its own paper. I worry that putting it in P2300 will raise eyebrows. This is the smallest change that works, and at this point I think the Committee will be scared off by anything other than small changes.

else if constexpr (<i>single-typed-sender</i>&lt;Value>)
return as_awaitable(static_cast&lt;Value&&>(value), static_cast&lt;Promise&>(*this));
else
return (Value&&) value;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This case would never expect to be reached in a well-formed program, right?
The is-awaitable concept check should in theory catch and cover all cases where the type should be awaitable.
If we get to the point here where we are returning value unchanged but it is not awaitable then the program is going to be ill-formed anyway...

Having said that, I wonder if we should be doing a stricter check than is-awaitable above - we do know the promise_type at this point so we can correctly construct the appropriate coroutine_handle<P> argument to await_suspend().

Copy link
Collaborator Author

@ericniebler ericniebler Sep 29, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The is-awaitable concept check should in theory catch and cover all cases where the type should be awaitable.

True. The question is one of diagnostics. By returning the non-awaitable thing unchanged, the user will get a compiler error telling them that they are trying to await a thingy that isn't awaitable. That's a much better usability story.

we do know the promise_type at this point so we can correctly construct the appropriate coroutine_handle<P> argument to await_suspend().

That's a fair point, but again, if a type has await_ready and await_resume (or an operator co_await), chances are they are really trying to make an awaitable type. Ignoring that and trying to interpret the type as a sender might make for lousy diagnostics.

Then again, it would in all likelihood not be a sender either, in which case this function would return the object unchanged, and the compiler will tell the user what they did wrong. Hrm.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we should be doing a stricter check than is-awaitable above - we do know the promise_type at this point so we can correctly construct the appropriate coroutine_handle<P> argument to await_suspend().

Filed as #214

Base automatically changed from awaitables-are-senders to main September 29, 2021 19:43
@ericniebler ericniebler merged commit 0461e3a into main Sep 30, 2021
@ericniebler ericniebler deleted the senders-are-awaitable branch September 30, 2021 16:43
Copy link
Collaborator Author

@ericniebler ericniebler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Following up on outstanding issues here, and a question for @lewissbaker


- If `sender_traits<remove_cvref_t<S>>::value_types<Tuple, Variant>` would have the form `Variant<Tuple<T>>`, then <code><i>single-sender-value-type</i>&lt;S></code> is an alias for type `T`.

- Otherwise, if `sender_traits<remove_cvref_t<S>>::value_types<Tuple, Variant>` would have the form `Variant<Tuple<>>` or `Variant<>`, then <code><i>single-sender-value-type</i>&lt;S></code> is an alias for type `void`.
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Filed as #210

<i>sender-awaitable</i>(S&& s, P& p);
bool await_ready() const noexcept { return false; }
void await_suspend(coro::coroutine_handle&lt;>) noexcept { start(state_); }
value_t await_resume();
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If value_types<Tuple, Variant> has form Variant<> then maybe this should be marked [[nodiscard]]?

@lewissbaker, wouldn't that mean that this awaitable will never produce a value, and so await_resume will never be called? What would be the point of declaring it [[nodiscard]]?

And if error_types<Variant> has form Variant<> then maybe this should be marked noexcept?

Filed as #213

else if constexpr (<i>single-typed-sender</i>&lt;Value>)
return as_awaitable(static_cast&lt;Value&&>(value), static_cast&lt;Promise&>(*this));
else
return (Value&&) value;
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we should be doing a stricter check than is-awaitable above - we do know the promise_type at this point so we can correctly construct the appropriate coroutine_handle<P> argument to await_suspend().

Filed as #214

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

Successfully merging this pull request may close these issues.

2 participants