-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
Conversation
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.
@jcouv @RikkiGibson @agocke please review |
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` |
There was a problem hiding this comment.
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>
?
There was a problem hiding this comment.
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.
There was a problem hiding this 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.
There was a problem hiding this 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)
@jcouv @RikkiGibson for another look please; I removed hoisted local, as per dotnet/runtime#112918. |
There was a problem hiding this 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)
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 ofTask
, since the user could write that themselves and the runtime will have to handle it.