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

Allow TaskGroup's ChildTaskResult Type To Be Inferred #2494

Conversation

rlziii
Copy link
Contributor

@rlziii rlziii commented Jun 18, 2024

Copy link
Contributor

@ktoso ktoso left a comment

Choose a reason for hiding this comment

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

Looks good to me! Minor ideas to improve proposal and we can ask the core team for a review 👍

@rlziii
Copy link
Contributor Author

rlziii commented Jun 19, 2024

Looks good to me! Minor ideas to improve proposal and we can ask the core team for a review 👍

Thank you for looking over everything. I have implemented the suggestions and updated a spots of grammar. Should I go ahead and tag this as Ready for review?

@ktoso
Copy link
Contributor

ktoso commented Jun 20, 2024

Yeap, please mark as ready to review! :)

@rlziii rlziii marked this pull request as ready for review June 20, 2024 13:53
@rjmccall rjmccall added the LSG Contains topics under the domain of the Language Steering Group label Jun 24, 2024
…the first sentence of the Design Details section.

## Detailed design

Because type inference is top-down, it relies on the first statement that uses `group` to infer determine the generic for `ChildTaskResult`. Therefore, it is possible to get a compiler error by creating a task group where the first use of `group` does not use `addTask(...)`, like so:
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Because type inference is top-down, it relies on the first statement that uses `group` to infer determine the generic for `ChildTaskResult`. Therefore, it is possible to get a compiler error by creating a task group where the first use of `group` does not use `addTask(...)`, like so:
Because type inference is top-down, it relies on the first statement that uses `group` to infer the generic arguments for `ChildTaskResult`. Therefore, it is possible to get a compiler error by creating a task group where the first use of `group` does not use `addTask(...)`, like so:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the suggestion.

Copy link
Member

Choose a reason for hiding this comment

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

Thank you! I'll kick the review off shortly.

// Expect `ChildTaskResult` to be `Int`...
withTaskGroup(of: Int.self) { group in
group.cancelAll()
group.addTask { 42 }
Copy link
Member

Choose a reason for hiding this comment

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

I think it would be beneficial to have some more realistic examples. For example, there are small-but-realistic examples at https://www.avanderlee.com/concurrency/task-groups-in-swift/ and https://www.kodeco.com/books/modern-concurrency-in-swift/v1.0/chapters/7-concurrent-code-with-taskgroup that you could reference (with attribution!) and will give reviewers a better feel for how this impacts use of the feature.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have updated the examples to use more real-world scenarios. Thank you for the suggestion. 👍

@DougGregor DougGregor merged commit eec5f1e into swiftlang:main Jul 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
LSG Contains topics under the domain of the Language Steering Group
Projects
None yet
4 participants