Deprecate abstract coroutine context keys

[dkhalanskyjb]

Additionally, removed the outdated opt-ins into experimental standard library APIs, as those are all stable now.

Fixes #4333

[murfel]

Just to clarify, you will also make a twin PR into stdlib to deprecate AbstractCoroutineContextKey, once you figure out the process for it?

[dkhalanskyjb]

@murfel, yes, but these are independent tasks.

[qwwdfsad]

That's a bit of a nuisance.

I would consider coupling this change with introduction of .dispatcher and .dispatcherOrNull extensions on CoroutineContext (not sure about the CoroutineScope).

[dkhalanskyjb]

I see what you mean, but this pattern is very niche anyway: https://grep.app/search?f.lang=Kotlin&regexp=true&q=%5B%5E+%5D%5C%5BCoroutineDispatcher%5C%5D

In addition, in most cases, using ContinuationInterceptor instead of CoroutineDispatcher doesn't make any functional difference (or even makes the code a bit more flexible), which means directing the few existing cases to .dispatcher is not ideal.

[qwwdfsad]

I've figured, but still see it as a good idea:

• That's, generally, an informal respect to backwards compatibility and migration paths (i.e. not how we did it with capitalize) for users who do use that
• The cost of this addition is negligible
• In the spirit of the standard library, it is a small but really convenient addition when you need it. I believe even I was in need of that quite a few times (I was also pretty sure I created an issue for that, but I failed to find it)

[LouisCAD]

My 2 cents:

The fact that something is niche shouldn't be the reason to dismiss/remove it.
It should only be a secondary factor, if a main reason justifies the removal/dismissal of something, IMO.

Also, beware that grep.app isn't finding all usages that GitHub search would.

For this case, your link leads to 66 usages, while I got 338 with public-only GitHub search: https://github.com/search?q=%22t%5BCoroutineDispatcher%5D%22+language%3AKotlin&type=code

If the measuring tool you use is minimizing usage of stuff, you'll end believing things are much more niche than they actually are.

That would risk churning more Kotlin devs than you would think if actions are made based on flawed data.

[dkhalanskyjb]

Hi Louis,

Since you're raising the topic of grep.app's reliability for the second time, I assume you'd appreciate an extended description of the methodology.

Yes, you are right, if something is barely used, it's not a reason to remove it or make it intentionally inconvenient. The criterion for inclusion of an API is that it should either solve a common need or not be easily expressible, but for removal, the harm of the API must outweigh its popularity. A terribly harmful API may get modified or removed even at the cost of far-reaching changes, whereas to remove an API that's just non-idiomatic and has a better alternative, it's enough for it to be niche.

grep.app is indeed not finding all usages. Neither does github, as there is a lot of code that's not on github at all. This is not a problem for our analysis, as the goal is not to evaluate how many codebases will be affected, but what fraction of them correspond to which measure of popularity. https://grep.app/search?q=kotlinx.coroutines.Job or https://grep.app/search?q=kotlinx.coroutines.flow.combine are examples of widely used APIs. https://grep.app/search?q=kotlinx.coroutines.channels.toList is niche.

The distribution of hits on grep.app is a bit different from the one on GitHub, as grep.app filters out duplicate files. If a user copy-pastes a library into their project, it results in additional hits on GitHub but not on grep.app. This means that not only absolute numbers will be different between GitHub and grep.app, but also the relative ones. I link to grep.app, as I believe that copies of libraries are not important for our analysis, since users are unlikely to maintain them on their own, so grep.app's results are more representative.

Another reason to use grep.app is to understand the usage patterns. This is often much more significant than it is to just count usages, as that way, we can learn more about why people are using an API at all and whether we can provide them with a better alternative. Removal of duplicates on the grep.app's side helps us more conveniently sift through the files.

To sum it up, yes, we are aware of the tool's limitations, which is why we would never say "this change will affect 100 codebases." We have no way of knowing. What we can fairly confidently say is, "the usage is dominated by uncommon requirements of a few large foundational codebases that have the resources to fix this."

Please let me know if this answers your doubts.

[LouisCAD]

Hello Dmitry,
Thanks for the answer!
I appreciate it, and it makes sense to me.

[murfel]

If I may chip in my 2 cents, I believe we should prioritise moving faster on the minor occasions, rather than being excessively thorough. So I vote not to do anything and merge this already :)

[dkhalanskyjb]

I decided against adding the .dispatcher extension, as the typical use case of duplicating another coroutine's dispatcher doesn't need to distinguish between ContinuationInterceptor and CoroutineDispatcher, which means the extension would be .continuationInterceptor.
Otherwise, .dispatcher[OrNull] would hide a subtle bug, as the code would read as, "This context has no dispatcher", when in fact, there is a continuation interceptor, even if it's not actually a dispatcher.
.continuationInterceptor doesn't seem any better than [ContinuationInterceptor] to me, so I don't see the point.

[murfel]

Use the explicit ContinuationInterceptor.Key, as per the deprecation advice? Or, alternatively, fix the advice to use the implicit ContinuationInterceptor and fix all usages to be implicit.

I'd mildly prefer all usages in the codebase to be either all ContinuationInterceptor xor all ContinuationInterceptor.Key, not just the ones you touch in this PR.

I don't have a strong opinion, just noticing a tiny inconsistency. Feel free to discard if it makes some sense to keep them different.

[dkhalanskyjb]

The specific syntax doesn't matter here and can be chosen abritrarily, but for the deprecation message, there is an important distinction.

A ContinuationInterceptor is the general version of a CoroutineDispatcher: something that controls what happens to a coroutine when it's resumed. In this PR, the problem is with ways of referring to a continuation interceptor. Saying "use ContinuationInterceptor instead of a CoroutineDispatcher" is meaningless, as a CoroutineDispatcher is a ContinuationInterceptor, we are already using one. As objects, however, ContinuationInterceptor is equal to ContinuationInterceptor.Key but distinct from CoroutineDispatcher[.Key].

We could say "use the ContinuationInterceptor companion object", or "use the ContinuationInterceptor key", but I prefer ContinuationInterceptor.Key over that, as this gives the user a clear reference to look up.

[murfel]

If I may chip in my 2 cents, I believe we should prioritise moving faster on the minor occasions, rather than being excessively thorough. So I vote not to do anything and merge this already :)