[Docs][Concurrency] Fix documentation about cancellation, copy docs to a few methods #63960
Conversation
| @@ -262,7 +258,7 @@ public struct TaskGroup<ChildTaskResult: Sendable> { | |||
| /// Adds a child task to the group. | |||
| /// | |||
| /// - Parameters: | |||
| /// - overridingPriority: The priority of the operation task. | |||
| @@ -491,15 +487,17 @@ public struct TaskGroup<ChildTaskResult: Sendable> { | |||
|
|
|||
| /// Cancel all of the remaining tasks in the group. | |||
| /// | |||
| /// After cancellation, | |||
| /// any new results from the tasks in this group | |||
| /// are silently discarded. | |||
There was a problem hiding this comment.
This isn't true, cancelled tasks run as usual and can be collected.
FYI @amartini51 in case we used this assumption somewhere else in some docs?
| /// A discarding task group eagerly discards and releases its child tasks as | ||
| /// soon as they complete. This allows for the efficient releasing of memory used | ||
| /// by those tasks, which are not retained for future `next()` calls, as would | ||
| /// be the case with a ``TaskGroup``. |
There was a problem hiding this comment.
Wrong group to have those docs
| @@ -916,15 +912,17 @@ public struct ThrowingTaskGroup<ChildTaskResult: Sendable, Failure: Error> { | |||
|
|
|||
| /// Cancel all of the remaining tasks in the group. | |||
| /// | |||
| /// After cancellation, | |||
| /// any new results or errors from the tasks in this group | |||
| /// are silently discarded. | |||
There was a problem hiding this comment.
ditto again wrong cancellation semantics assumption, also removed and clarified below.
| /// | ||
| /// Since a `TaskGroup` is a structured concurrency primitive, cancellation is | ||
| /// - when ``cancelAll()`` is invoked on it, | ||
| /// - when an error is thrown out of the `withThrowingTaskGroup(...) { }` closure, |
There was a problem hiding this comment.
throwing groups have this semantic, non throwing dont. Technically they could, but the APIs are fully non throwing today in those, including the with closure
| /// - priority: The priority of the operation task. | ||
| /// Omit this parameter or pass `.unspecified` | ||
| /// to set the child task's priority to the priority of the group. | ||
| /// - operation: The operation to execute as part of the task group. | ||
| @_alwaysEmitIntoClient |
There was a problem hiding this comment.
missed to copy docs on this occurrence of the method
There was a problem hiding this comment.
I think it was intentional that we didn't copy/paste the documentation onto deprecated versions.
| /// to set the child task's priority to the priority of the group. | ||
| /// - operation: The operation to execute as part of the task group. | ||
| /// - Returns: `true` if the child task was added to the group; | ||
| /// otherwise `false`. |
There was a problem hiding this comment.
missed to copy docs on this occurrence of the method
| /// - Parameters: | ||
| /// - operation: The operation to execute as part of the task group. | ||
| /// - Returns: `true` if the child task was added to the group; | ||
| /// otherwise `false`. |
There was a problem hiding this comment.
missed to copy docs on this occurrence of the method
ktoso
added
the
concurrency
|
@swift-ci please smoke test |
While adding documentation in #63956 I noticed that some of the docs are wrong, e.g. the docs mentioned that "cancelled tasks are silently discarded" which isn't true, they run as usual.
This does a cleanup pass wrt. cancellation semantics and makes it clearer and correct.
Added docs in a few spots where we didn't copy them over; sadly this API is riddled with tons of overloads which makes such mistakes possible.