To clarify, I'm not talking about adding a new type, just adding a new function and reusing the existing TaksGroup type. The difference between what is currently propose and my proposal is basically
keeping the types all the same but just removing discardResults parameter and instead adding Discarding to the method name. The amount of new API surface would be exactly the same as the current proposal i.e. two new functions. Note that in the internal implementation of TaskGroup we will likely still need to use a boolean flag of some kind.
Yeah, I was clarifying that either way it is a new method, not just adding a parameter, but yeah form an user's perspective that's different.
The new name could be good as it makes the signature strictly correct again, though it pushes us towards actually making a ResultDiscarding Task Group IMHO, it's a bit weird that we have a new method, and chance to do the right thing with not offering next() but don't do so. The good news is that the underlying C++ implementation would be shared, and no new builtins (well, one which we need anyway), would be added... making this much leaner than the initial "complete new type with complete new impl" approach we had early on.
We'll have to see what the language and concurrency team would prefer. Adding a new type for this was considered too heavy, but perhaps given all those things and a better name for it it may be on the table after all?
If I’m following the discussion correctly and there’s agreement that the child task result type should be Void, and the group result type is ignored, shouldn’t this just be…
…which seems pretty nice, and overall an even more elegant solution than having a Boolean toggle.
Now, if it simply must be that we need new “variants” of TaskGroup for “discarding-ness” just like for “throwing-ness,” say for the purpose of eliminating nonsensical uses of next(), my personal two cents are that that’s fine.
It’s actually quite a different end result than the originally pitched idea of a distinct notion of a “task pool” type with its own separate hierarchy of “throwing-ness,” even if the end result is having additional types.
I agee, we should drop the of childTaskResultType: ChildTaskResult.Type parameter if we constrain it to Void anyway (which I think we should). FWIW Swift 6 will not even allow a same-type constraint in this case and the compiler already produces a warning today: Same-type requirement makes generic parameter 'ChildTaskResult' non-generic; this is an error in Swift 6.
I'm not sure if removing the returning returnType: GroupResult.Type = GroupResult.self parameter is a good idea though. It can still be useful to eventually return something from within the body function e.g. a count of created child tasks.
This is what I suggested at the end of the pitch thread. Given that the throwing version must be throws instead of rethrows, I think it makes even more sense. Writing it all out, it would be:
This design really seems much more intuitive to me. Having a type parameter that must always be used with the same type feels like an annoyance at best, and an attractive nuisance at worst.
The number of differences between an existing task group and this discarding-results-task group is larger than it appeared while this was discussed in the pitch phase, so it's worth revisiting the idea of a new type. Personally, I find the idea of calling it DiscardingTaskGroup to go along with withDiscardingTaskGroup much more appealing than the originally-proposed TaskPool. My main complaints at pitch phase were (1) almost complete overlap in functionality (which is less true now) and (2) introducing the "pool" name alongside "group".
As for API surface... there isn't a whole lot of API on this once it becomes its own type, so the duplication isn't that bad. And it doesn't seem like there's much utility in (say) adding a protocol to unify the two types, because I doubt anyone would want to write generic functions based on that protocol.
+1 I have been chatting with @ktoso about this today already where he explained me all the little differences in behavior between the discarding and non discarding version. My fear was that it will become very hard to reason about Code and teach these nuances to new people. A separate type should make this way clearer.
Apologies, I'm working on a comprehensive writeup about the things we learnt in course of this review and addressing the feedback in the implementation My chat with Franz was a bit of a rant while implementing the edge cases around cancellation/error handling
I'll have an update for this thread as well as the proposal ready soon enough, hopefully today already! Thank you very much for the input everyone, we're arriving at a much nicer end result I think.
Short version / preview:
new type sounds good after all, hooray
new method sounds good, the naming discussions have resolved the concerns we had about the new type - thank you everyone for the feedback
cancellation and error handling behavior is far more complex than we thought at first, since the usual group patterns don't work due to the disappearance of try await next()
cancelling "siblings" needs special handling since we cannot rely on the next() throwing out of the group body anymore
I believe we should be able to amend/extend this review round, as the overall consensus so far has been in that direction.
I'd be interested in feedback about the error handling and group cancellation aspect specifically, which is explained in the new "Error propagation and group cancellation" section in the updated proposal. We believe those are the right semantics, but would love to hear what people think the error thrown in this situation should be:
withThrowingDiscardingTaskGroup() { group in
group.addTask { try boom(1) }
try boom(2)
}
The boom(1) and boom(2) are racing here, and it is probably the most consistent to keep close to the promise that the "first" error thrown will be propagated out of the group -- as we promise this in general already for child tasks.
We are not considering adding the "error reducer" functionality in this take though.
Paging @Douglas_Gregor for review manager duties, thanks in advance!
I would expect boom(2) to be thrown here, but only because I expect the outermost block here to run synchronously for lack of explicit await. In other words, I expect the order of operations to be:
Task group is created
try boom(1) is scheduled, but not run
try boom(2) is run and throws an error
try boom(1) is cancelled and its error discarded
The error from try boom(2) is rethrown.
In practice I wouldn’t complain if boom(1) wins. I also don’t have any expectations either way here:
try await withThrowingDiscardingTaskGroup { group in
group.addTask {
await …
try boom(1)
}
await …
try boom(2)
}
unless I can reason about the await statements themselves in a way that makes this no longer a race (e.g. one is sleep(1000) and the other is sleep(2000)).
This assumption is invalid, and it's always invalid in concurrent code.
It's entirely possible for the CPU scheduler to decide to put the thread running the outermost block to sleep immediately after addTask returns, and then run the child task to the point that it errors. That can allow it to throw first, even though there's no await.
Can you guys update the proposal that ThrowingDiscardingTaskGroup actually has a generic Failure: Error type. While parsing the proposal I had the impression that error type was completely discarded as well. I had to look up the actual implementation to find out that it's still there.
I am happy that this proposal has gone in this direction - the original distinction between a TaskGroup and a TaskPool would have been obscure whereas the new naming makes the intention clear. There is enough conceptual difference (along with practical behavioural differences, particularly around error handling) to warrant a different type; most use cases will likely want to operate on a particular type of group.
The error behaviour makes sense to me and provides consistency. As stated, an "error reducer" seems potentially useful but can be left as a future extension to support more advanced use cases if and as they become clearer.
Small nickpick: There is an inconsistency between Discarding[Throwing]TaskGroup and [Throwing]DiscardingTaskGroup
Not much to add from my side, I had 2 concerns (confusion by using the same type, and the doc not being clear enough about auto awaiting) and both have been cleared up!
I think the separate type offers a much better and safe (as in not being easily miss-usable) than the previous version.
My chat with Franz was a bit of a rant while implementing the edge cases around cancellation/error handling 
