SE-0304: Structured Concurrency

[ktoso (Konrad 'ktoso' Malawski 🐟🏴‍☠️)]

Thanks for the review comments Chris, diving in then:

---

@Chris_Lattner3

The call to Task.unsafeCurrent is not itself unsafe, it is the returned UnsafeConcurrentTask that is and the possibility of it escaping.

Yes, that's correct. @John_McCall strongly suggested we call the function to reach for it unsafe though as often times you may not even see the returned type, e.g. if you'd close over it from a different task and touch it from another task etc... It felt important for the name to imply users have to be careful here.

@Chris_Lattner3

For sync clients that don't need the unsafe stuff, I think we should something like:

extension Task { 
  static var unsafeCurrent: UnsafeCurrentTask? { get }
  static var current: Task? { unsafeCurrent?.task }   // safe for sync callers!
}

That would be nice, but it'll cause us to overload a property with a different return type depending if it's async or not... I'm not completely up to date what our plan with overloads is, it kept changing back and forth, but today we can't write this:

Task.swift:541:21: error: invalid redeclaration of 'current'
  public static var current: Task? {
                    ^
Task.swift:52:21: note: 'current' previously declared here
  public static var current: Task { get async {
                    ^

We also can't add the current computed synchronous variable, even if we kept the async function:

Task.swift:540:21: error: invalid redeclaration of 'current'
  public static var current: Task? {
                    ^
Task.swift:52:22: note: 'current()' previously declared here
  public static func current() async -> Task {
                     ^

So yeah, interesting idea but today we can't write the synchronous current because of redeclaration issues. @Douglas_Gregor would know what our end goal plan with those is.

@Chris_Lattner3

Also, shouldn't current() be a property now that getters can be async?

Yes, and the proposal itself has a comment about this:

static func current() async -> Task { get }
// If async computed properties proposal is accepted, this can become a static property

It is one of those proposal inter-dependence things... The async properties are still in pitch phase [Pitch] Effectful Read-only Properties but yes, the intent is for current to be an async getter.

I think it's best to keep the proposal in sync with the implementation shape for now, until the async properties land at least, to not confuse people about what API is "real".

@Chris_Lattner3

I imagine that the async version may be more efficient (?) and if so it seems these should be overloadable given the rules in the async/await proposal.

In this specific case it does not matter for efficiency if the func is async or sync -- both end up in the same actual implementation (which eventually hits a thread local or other platform specific storage for it).

Yeah we're missing a way to express the overload, Doug should know our plans with overloading, I'm not sure about those.

---

@Chris_Lattner3

In Detatched Tasks, I think this example:

let dinnerHandle: Task.Handle<Meal, Error> = ...
try await eat(dinnerHandle)

should be try await eat(dinnerHandle.get()) .

That's actually correct, it shows you can pass around a handle (for better or worse), because the eat is declared as:

func eat(mealHandle: Task.Handle<Meal, Error>) {

I will have added the missing mealHandle: label, that'll explain things better.

---

@Chris_Lattner3

In Task Groups, it looks like withGroup always throws. Wouldn't it make sense to support a body that doesn't throw, either by overloading this or by using rethrows? Is the type system not powerful enough (because it would have to flow down to Group.add) to model what we need here?

Oh that's a great catch, seems we are able to express this - thanks! A rethrows does the job here, given the current shape of the groups APIs, hooray for non-escaping body.

I'll adjust that right away.

@Chris_Lattner3

Should Task.Group have a generic error argument to mirror Task.Handle and more naturally allow for typed errors in the future? The existing withGroup could always force Error , but this would permit other withGroup -like members in the future.

I guess we might have to if other APIs under review are doing the same now - i.e. the UnsafeContinuation APIs moved towards a typed error even through it only ever is Error.

I guess it'll be an extra childTaskThrowing: Failure = Error.self parameter
WDYT @Joe_Groff ?

---

Thanks for the review, plenty useful feedback here