I think this only applies to Escapable types, because non-escapable types can't be stored in a class instance. It's also possible to just store a ~Discardable escapable value in a global variable to leak it. Where ~Discardable could actually offer new guarantees is for non-escapable types (if non-escapable values remain unable to be stored in a class instance, and therefore can't be leaked in a reference cycle), where one could guarantee that a ~Discardable non-escapable value will be consumed prior to its lifetime ending. For example, a ~Discardable non-escapable value with a mutating lifetime dependency on a variable could safely leave that variable temporarily uninitialized.
The fact that any escapable value can be leaked and therefore ~Discardable can only be effectively a lint for escapable types, I think reduces the negative impact of Continuation being Discardable. However, I think it would be better for Continuation's deinitializer to just log a warning instead of crashing the program.
Given that deinitializers are implicitly called, and the precise point at which they are called is nondeterministic, I think it's generally been established that deinitializers should just free up resources, and their effects should be (at least mostly) invisible to the caller. I think the fact that Continuation's deinitializer would have such a visible effect, by crashing the program, is unprecedented.
I'm also concerned that it could lead to "Hyrum's law" issues. That is, people will come to rely on certain APIs not discarding values, even though that isn't enforced by the type system, and even if the API author didn't intend for that to be guaranteed. I think an analogous scenario is if ~Copyable wasn't introduced to the type system, and some types just had a "copy constructor" that crashes the program.
I also don't think there's much of a practical benefit. While discarding a Continuation is a programmer error, it's harmless except for wasting resources, and I think unlikely to be indicative of a more serious problem. It's less similar to things like integer overflow and out-of-bounds indexing, and more similar to things like reference cycles. Additionally, crashing the program can be disruptive, such as if the program is a long-running server process. And logging a warning, I think, would be almost as effective as crashing the program for making the developer aware of the issue. So I think the tradeoffs weigh towards logging a warning being preferable.
I disagree with the characterization that leaking a Continuation is harmless. While it doesn’t result in undefined behavior it often means that the program is stuck or at least parts of the program are stuck. I would rather have my service crash than it being in a state where it can’t make forward progress.
I think that both halting a thread and producing a fatal error at runtime are both bad developer experiences. Halting the thread, while it doing really break anything, just silently ignores the error while possibly causing performance issues. A fatal error, even if it is easy to find, is still an issue, as the code path that discards the continuation is not always immediately obvious and may not always be tested thoroughly or caught in a code review.
Personally, I think that if an invalid use is obvious at compile time, we should at least do a best effort attempt to diagnose it at compile-time. Even if we don’t want to wait for a possible future ~Discardable, we could at least make a special case specifically for Continuation that diagnoses the simplest cases (i.e. not in an array). This way, it may even be possible to make it ~Discardable in the future without breaking source in a lot of cases.
Just a small nit, I think the failure type parameter on the withContinuation function should be named throwing rather than throws to stay consistent with the naming in the _Concurrency module.
I agree with the argument that we shouldn't record the source location of the withContinuation call in order to report it in the crash.
For one, this ends up being a non-trivial overhead that I don't think we want to impose on people who are using this API correctly.
For another, I think it's usually the wrong location to report for the crash. Continuation is a non-copyable type, which means the code at fault in the crash is always the code that didn't properly handle/resume the continuation. When the continuation is held locally, the buggy code is the code that called deinit, which means the stack trace of the crash tells you exactly who's at fault. If the continuation is held by some abstract container (e.g. it's stored in a Dictionary or an actor field), the bug might be more indirect; e.g. it might actually be that some previous bit of code failed to properly consume the continuation out of its container. However, in this case, the stack trace should still reliably identify the container, and there it is likely to be just as helpful for identifying the bug as the source location of the withContinuation call would have been.
Finally, the debugger (and similar tools) ought to be able to report the full stack trace of a task that is suspended on an unresumed continuation, which should be strictly more information than the withContinuation source location as long as the tool has line tables for the process.
Thanks for the review discussion, everyone! The language steering group has decided to accept this proposal with revisions, changing the name of withContinuation(of:throws:) to withContinuation(of:throwing:).