Rework CompletionHandler to avoid subclassing a functional type

[dkhalanskyjb]

Workaround for KT-64075

[dkhalanskyjb]

I don't know why Lincheck tests fail with a LinkageError. @ndkoval, could you help?

[dkhalanskyjb]

Status: it seems like the fix should be on the side of Lincheck. We probably won't have to change the approach taken in this PR.

[qwwdfsad]

Great job, thank you for taking care of this nuisance.

Looks good, all my remarks are trivialities about the doc

[qwwdfsad]

Nice doc 👍

[qwwdfsad]

Should we?

I see this type alias (or maybe even I would interpret it as an opaque type if it was possible) as a perfect fit -- it is a functional type, but with very specific semantics that is a property of this "type", not of the callsite (invokeOn* and coroutine builders), and the documentation to this type is a perfect place to spell it out.

WDYT? As usual, this particular endpoint might be not the most important one, but it's nice to align our mental models here

[dkhalanskyjb]

Here's my reasoning behind the TODO.

tl;dr: this function's contract is defined by the places that accept the function, not the other way around, so these other places should define what they expect. Also, the latest commit that fixed the explanation of what the argument means, and this type looks even more unnatural now.

Long version:

I know of two use cases for binding an arbitrary function to a name:

• Passing it somewhere so that that other place calls it,
• Storing it somewhere so that someone calls it later.

For the second use case, introducing a type alias is clearly beneficial: when you have a var handler: CompletionHandler, there is some contract that should connect the parties. If I see a CompletionHandler, I may only invoke it so or so, and return, it promises me this and that, and vice versa. But what we have here is purely the first use case.

but with very specific semantics that is a property of this "type", not of the callsite

This surprises me greatly: I think it's exactly the opposite.

The contract on the handler is exactly the combination of the contracts of the functions that accept the handler. Here's the deciding factor for me: if the behavior of invokeOnCompletion changed and the contract on CompletionHandler could somehow be relaxed, we wouldn't define a RelaxedCompletionHandler (supertype of CompletionHandler), we would just change the documentation of CompletionHandler. So, CompletionHandler is a "fake" type, a product of circumstances, only useful for passing it to the specific call sites.

[qwwdfsad]

Thanks for the elaborate answer.

we would just change the documentation of CompletionHandler.
The contract on the handler is exactly the combination of the contracts of the functions that accept the handler.

Indeed. It appears I just think about this one inside out -- its definition/contract is driven bu the need of its "clients" (invokeOnCompletion), yet the moment the contract is established -- it's the property of CompletionHandler that it does not throw, not the vise versa -- "in general, CompletionHandler is whatever you want it to be, but it shouldn't throw if passed to Job.invokeOn...()"

[dkhalanskyjb]

If I got your intention correctly, you're touching on a deep concept here, called the native type theory. In essence, we can take any dynamically typed function and define its type as a function that accepts all values that don't make it misbehave. By this logic, fun LocalDate(year: Int, monthNumber: Int, dayOfMonth: Int) is equivalent to a total function of the type fun LocalDate(year: Year<TypeLevelYearNumber>, monthNumber: MonthNumber<TypeLevelMonthNumber>, dayOfMonth: DayOfMonth<MonthNumber = TypeLevelMonthNumber, YearNumber = TypeLevelYearNumber>).

I think the reasons CompletionHandler is not worth introducing are the same reasons typealias MonthNumber = Int isn't worth introducing. I can elaborate on them if you want.

[qwwdfsad]

It's quite close. I don't see it fit e.g. for an arbitrary function, but here we have a multitude of functions that all accept the same kind of parameter with the same kind of constraints. These constraints are significant enough to document, and broad enough to get tired of repeating it (NB: here, I focus not on "repeating the text" but on "repeating the conceptual complexity of the entity accepting this parameter and draining our complexity budget").

It's just lightweight and specific enough so it doesn't deserve a dedicated nominal type.

(please note that this discussion is not a blocker of this PR by any means)

[dkhalanskyjb]

here we have a multitude of functions that all accept the same kind of parameter with the same kind of constraints

Let's consider two scenarios separately:

• There's a CompletionHandler and, separately, a CancellationHandler;
• The two types of handlers stay merged into one, just because they have the same type.

I'm confident the second option is a no-go because the constraints and the behavior are actually different between invokeOnCompletion and invokeOnCancellation. Knowing just the CancellationHandler type, you can't make a judgement about how it will behave, you need to know which of the two endpoints ends up actually receiving the handler.

The first option also seems strange to me: then, the contract on CompletionHandler would simply completely mirror the contract on invokeOnCompletion, and I don't see the benefits of introducing a separate entry point when we can just say "will be passed to invokeOnCompletion and must satisfy the same requirements" and in fact, must either do so or direct people to the documentation of CompletionHandler explicitly, because no one realistically is going to notice, "Aha, this function accepts not just any lambda but one with a special type, let me read the docs."

[qwwdfsad]

Would be nice to mention the cause semantics and when it's a null?

[dkhalanskyjb]

Done. Now the abstraction of CompletionHandler looks even more strange.

[qwwdfsad]

Yeah, it's a mess. Luckily, it's a documented mess right now and a constant reminder we probably should change it :)