Skip to content

[Concurrency] Alternative Task API. #37495

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 6 commits into from
May 21, 2021

Conversation

DougGregor
Copy link
Member

@DougGregor DougGregor commented May 18, 2021

The Task type has oscillated somewhat from being purely a namespace,
to having instances that are used (albeit rarely), back to purely
being a namespace that isn't used for all that many names. Many of the
names that used to be on Task have already been moved out, e.g., for
creating new detached tasks, creating new task groups, adding
cancellation handlers, etc.

Collapse Task.Handle<Success, Failure> into Task<Success, Failure>.
Task.Handle is the type that is most frequently referenced in the
concurrency library, so giving it the short name Task is most
appropriate. Replace the top-level async/detach functions with a
Task initializer and Task.detached, respectively.

The Task type can still act as a namespace for static operations
such as, e.g., Task.isCancelled. Do this with an extension of the
form:

extension Task where Success == Never, Failure == Never { ... }

We've been accruing a number of compatibility shims. Move them all
into their own source file, deprecate them, and make them
always-emit-into-client so they don't have any ABI impact.

This also covers other changes that came up in the review:

  • Switch get() to value as an async property
  • Switch getResult() to result as an async property
  • Switch order/names of withTaskCancellationHandler parameters
  • Update names of TaskPriority members, and make it a RawRepresentable struct

The `Task` type has oscillated somewhat from being purely a namespace,
to having instances that are used (albeit rarely), back to purely
being a namespace that isn't used for all that many names. Many of the
names that used to be on Task have already been moved out, e.g., for
creating new detached tasks, creating new task groups, adding
cancellation handlers, etc.

Collapse `Task.Handle<Success, Failure>` into `Task<Success, Failure>`.
`Task.Handle` is the type that is most frequently referenced in the
concurrency library, so giving it the short name `Task` is most
appropriate. Replace the top-level async/detach functions with a
`Task` initializer and `Task.detached`, respectively.

The `Task` type can still act as a namespace for static operations
such as, e.g., `Task.isCancelled`. Do this with an extension of the
form:

    extension Task where Success == Never, Failure == Never { ... }

We've been accruing a number of compatibility shims. Move them all
into their own source file, deprecate them, and make them
always-emit-into-client so they don't have any ABI impact.
@DougGregor
Copy link
Member Author

@swift-ci please test

@ktoso ktoso self-requested a review May 18, 2021 22:35
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.

Looking good! Thanks Doug, pretty happy with the final names of things.

Minor change to be done in priorities still, you can do this here or later.

Based on feedback from the second review, we decided to go with
high/default/low/background, with aliases for the Dispatch-inspired
names. While here, make TaskPriority be backed by a UInt8 to better
describe the actual restrictions, and start removing userInteractive,
because clients shouldn't be able to specify it.
Swap the order of the arguments so we now have:

    await withTaskCancellationHandler {
      // do stuff
    } onCancel: {
      // ..
    }
@DougGregor
Copy link
Member Author

@swift-ci please smoke test

@DougGregor
Copy link
Member Author

@swift-ci please build toolchain

@DougGregor DougGregor merged commit fb2197c into swiftlang:main May 21, 2021
@DougGregor DougGregor deleted the task-api-rework branch May 21, 2021 04:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants