Skip to content
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

Address async issues #1027

Merged
merged 10 commits into from
Feb 7, 2024
Merged

Conversation

BillWagner
Copy link
Member

@BillWagner BillWagner commented Dec 7, 2023

Each fix is in a single commit. In order:

Fixes #857: produces a result vs. returns a value.

For an async method, say "produces a result value" vs. "return a value" when describing the rules for the returned "task type".

Fixes #878

Add a note that the builder pattern supports identity convertible types. I made this a note, rather than normative language. I read the existing text to allow the example in the issue (as does the roslyn implementation).

Fixes #775

Add a bullet to the list specifying that an OperationCanceledException puts the returned task in the cancelled state.

This meant adding OperationCanceledException to the types in Annex C.

I chose this fix because all known task builders conform to this behavior. It seems reasonable to standardize it.

Fixes #879

Clarify the required accessibility of any task builder type and its required members. The members of the task builder type must have declared public accessibility. The accessibility of the task builder type must match the accessibility of its corresponding task type.

Note that we will need to update this text once dotnet/roslyn#71283.

The fifth commit adds a note clarifying the meaning of the accessibility requirements for the task builder type and its corresponding task type.

Fixes dotnet#857

For an async method, say "produces a result value" vs. "return a value" when describing the rules for the returned "task type".
Fixes dotnet#878

I made this a note, rather than normative language. I read the existing text to allow the example in the issue (as does the roslyn implementation).
Fixes dotnet#775

Add a bullet to the list specifying that an `OperationCanceledException` puts the returned task in the cancelled state.

This meant adding `OperationCanceledException` to the types in Annex C.

I chose this fix because all known task builders conform to this behavior. It seems reasonable to standardize it.
Clarify the required accessibility of any task builder type and its required members.

Note that we will need to update this text once dotnet/roslyn#71283 is fixed and merged.
The normative language on exactly matching accessibility is all that's strictly needed. But, I added a note about the implications for nested types and internal types.
@BillWagner BillWagner marked this pull request as ready for review December 15, 2023 19:37
@BillWagner BillWagner requested a review from jskeet December 15, 2023 19:37
@BillWagner BillWagner added the meeting: discuss This issue should be discussed at the next TC49-TG2 meeting label Dec 15, 2023
Copy link
Contributor

@jskeet jskeet left a comment

Choose a reason for hiding this comment

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

Mostly fine (and thanks!), a few comments/questions.

Copy link
Contributor

@jskeet jskeet left a comment

Choose a reason for hiding this comment

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

LGTM again, after discussion.

@BillWagner BillWagner merged commit 03c5cdf into dotnet:draft-v8 Feb 7, 2024
7 checks passed
@BillWagner BillWagner deleted the address-async-issues branch February 7, 2024 20:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
meeting: discuss This issue should be discussed at the next TC49-TG2 meeting
Projects
None yet
3 participants