Skip to content

Update RuntimeHelpers.Await rules #77957

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Apr 29, 2025
Merged

Conversation

333fred
Copy link
Member

@333fred 333fred commented Apr 1, 2025

These rules now clarify the extensibility point we're looking to establish with the runtime for Await helpers. Generally speaking, these are just ordinary methods that a user is free to call, so there's no issue with us using them for subtypes of Task, since the user could write that themselves and the runtime will have to handle it.

These rules now clarify the extensibility point we're looking to establish with the runtime for `Await` helpers. Generally speaking, these are just ordinary methods that a user is free to call, so there's no issue with us using them for subtypes of `Task`, since the user could write that themselves and the runtime will have to handle it.
@333fred 333fred requested a review from a team as a code owner April 1, 2025 21:01
@ghost ghost added Area-Compilers untriaged Issues and PRs which have not yet been triaged by a lead labels Apr 1, 2025
@333fred 333fred requested review from RikkiGibson, jcouv and agocke April 1, 2025 21:05
@jcouv jcouv self-assigned this Apr 2, 2025
@333fred
Copy link
Member Author

333fred commented Apr 10, 2025

@jcouv @RikkiGibson @agocke please review

@RikkiGibson RikkiGibson self-assigned this Apr 10, 2025
2. There is an identity or implicit reference conversion from `E` to the type of `P`.
4. Otherwise, if `Mi` has a generic arity of 1 with type param `Tm`, all of the following must be true, or `Mi` is removed:
1. The return type is `Tm`
2. There is an identity or implicit reference conversion from `E`'s unsubstituted definition to `P`
Copy link
Member

Choose a reason for hiding this comment

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

I'm unsure what "E's unsubstituted definition" means. Is it something like, if E is Task<C> then its unsubstituted definition is Task<T>?

Copy link
Member Author

Choose a reason for hiding this comment

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

Correct. I also struggled to find a better wording, but couldn't come up with one.

Copy link
Member

@RikkiGibson RikkiGibson left a comment

Choose a reason for hiding this comment

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

Done review pass. I found the parameter matching requirement in the generic case a little unclear. Otherwise looks good.

Copy link
Member

@jcouv jcouv left a comment

Choose a reason for hiding this comment

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

LGTM Thanks (iteration 3)

@333fred
Copy link
Member Author

333fred commented Apr 29, 2025

@jcouv @RikkiGibson for another look please; I removed hoisted local, as per dotnet/runtime#112918.

Copy link
Member

@jcouv jcouv left a comment

Choose a reason for hiding this comment

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

LGTM Thanks (iteration 4)

@333fred 333fred merged commit 927b5c6 into dotnet:main Apr 29, 2025
4 of 5 checks passed
@333fred 333fred deleted the update-runtime-async-doc branch April 29, 2025 20:11
@dotnet-policy-service dotnet-policy-service bot added this to the Next milestone Apr 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Compilers Feature - Runtime Async untriaged Issues and PRs which have not yet been triaged by a lead
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants