Version overloading

[faizilham]

This is an issue for discussion of version overloading proposal. The full text of the proposal is here.

Please use this issue for the discussion on the substance of the proposal. For minor corrections to the text, please open comment directly in the PR #423.

[udalov]

functions with @JvmOverloads annotation may not have version annotated parameter.

Why? It seems beneficial to have deprecated-hidden $default methods generated for those too.

[faizilham]

Why? It seems beneficial to have deprecated-hidden $default methods generated for those too.

Mainly because of the implementation clashes due to the different use cases.

The current implementation of @JvmOverloads does not copy the default values of the optional parameters to the generated overloads, while the version overloading does. As far as I know, this is done in JvmOverloads lowering to prevent problems with optional parameters that depend on other parameters:

@JvmOverloads
fun xyz(a: Int = b, b: Int = 1) { ... }

The generated overload is fun xyz(a: Int), without the default value of a. This is not a problem for JvmOverloads since the main use case is Java interoperability.

For version overloading though we need to copy the default values since the use case is compatibility with older (mostly Kotlin) binaries. In this case, the compiler can copy the default value as long as b has older or the same version annotation as a, otherwise it reports an error.

fun xyz(@IntroducedAt("2") a: Int = b, b: Int = 1) { ... } // deprecated overload: fun xyz(b: B = 1)

fun xyz(a: Int = b, @IntroducedAt("2") b: Int = 1) { ... } // error! 'a' depends on a "future" parameter 'b'

There are also problems with the overload deprecation. JvmOverloads does not deprecate-hide the generated overloads to allow discoverability from Java source code, while version overloading deprecates to prevent users from using older signatures.

The implementations will also have to be tightly coupled in order to prevent overload signatures clash, but this might be a lesser concern.

[udalov]

So, @JvmOverloads and @IntroducedAt solve different use cases. I agree with that, and that's why I think they should be orthogonal features, so you can use either or both of them on the same declaration. Otherwise, as a library author, I can't use this feature for an already existing @JvmOverloads-annotated declaration.

There are also problems with the overload deprecation. JvmOverloads does not deprecate-hide the generated overloads to allow discoverability from Java source code, while version overloading deprecates to prevent users from using older signatures.

Maybe I'm not seeing something, but this should be solved by lowering @IntroducedAt before @JvmOverloads, and tweaking the @JvmOverloads lowering to avoid generating declarations with an already existing signature.

[faizilham]

I just realized the example fun xyz(a: Int = b, b: Int = 1) already won't compile because b is seen as uninitialized. The signature clash problem is still not yet solved though.

[...] this should be solved by lowering @IntroducedAt before @JvmOverloads, and tweaking the @JvmOverloads lowering to avoid generating declarations with an already existing signature.

This could work, although I'm not sure how we should handle cases like this:

@JvmOverloads
fun xyz(
    a: Int = 1,
    @IntroducedAt("3") a1: Int = 2,
    @IntroducedAt("2") b : Int = 3,
) { }

/* Which ones to choose?
IntroducedAt overloads:
  1. @Deprecated fun xyz(a: Int = 1)
  2. @Deprecated fun xyz(a: Int = 1, b: Int = 3)

JvmOverloads overloads: 
  3. fun xyz()
  4. fun xyz(a: Int)             // clash with 1
  5. fun xyz(a: Int, a1: Int)    // clash with 2
*/

If we choose the overloads 1, 2, and 3, it preserves binary compatibility but breaks the expected behavior of JvmOverloads.

Let's say we call xyz(1, 2) from Java. This won't work directly because it is deprecated-hidden, but even if it is not hidden, it would actually call xyz(a=1, b=2) instead of the expected xyz(a=1, a1=2).

This is very similar with the source compatibility problem mentioned in the text which can be solved by providing arguments by name, but we can't do that in Java.

This might still be a reasonable trade-off, but I don't know how acceptable it is to most users. What do you think?

[hrach]

I'm somewhat puzzled by the format requirements, which somehow suggest using the exact release name ("1.1.0.1") of the change. But I'd say that is often unknown during the development time. So I'm thinking that practically, I should rather resort to simple "incremental" single-digit versioning.

How do you think the workflow should look? Do you expect actual release version usage, or is it generally open to whatever suits the author? Do you have any feedback e.g. from Google that Compose will use it somehow?

[faizilham]

I think this should be left to the library authors. If the authors find simple incremental versioning to be more convenient, then by all means they can keep it. A simple incremental number still fulfill the format requirement after all. However, I think the format should be flexible enough to also allow exact release version number if the authors want to use that instead. An exact release version number is also quite useful for documentation.

I also want to mention that relabeling version number is possible as long as it does not change the chronological order of when the parameters are introduced. While it might not be the ideal solution, this can be used as a workaround. Let's say a library's latest released version is 1.2.5 and the soonest possible next version is 1.2.6. The library authors have some choices when introducing a new parameter:

1. Simply use 1.2.6, but if it is delayed then relabel it to the correct version number just before release.
2. Use a version number that is not canonical to the library's, such as 1.2.5.999, and relabel it to a canonical one in later patch.
3. Abandon using library version and relabel all the version numbers to simple incremental number while keeping the correct chronological order.

Do you have any feedback e.g. from Google that Compose will use it somehow?

We are also still waiting for feedback from them.

[hrach]

Yes, thank you. I understand the relabeling is possible, and I'm not against the current proposal in any way. I was just curious how you (Jetbrains) expect this to be used, if you have any internal decisions (e.g., IntelliJ will use platform releases as the version marker), etc. :) Thx.

[nikitabobko]

Notice that if the version numbers appear not in ascending order, it might lead to source incompatibility

The current proposal doesn't allow adding new optional parameters in the middle, only to the end.

What if we sorted the parameters in the binary signature in the following way? params.sortedWith(compareBy({ it.introducedAt }, { it.parameterIndex }))

This way, we could allow to add parameters in the middle.

Downsides?

(1) This approach kinda assumes that Java is a second class citizen. Java can only understand parameters by their order, and the order of parameters in the binary signature is no longer optimal from the programmers point of view but it's optimal from the binary signature point of view.

For example:

fun poorJava(
    textFont: Int = 1,
    @IntroducedAt("2") textSize: Int = 1,
    @IntroducedAt("1") background: Int = 1,
) {} // binary signature: poorJava(textFont, background, textSize)

Although, it's more optimal and logical to put textFont and textSize together, they are separate, and Java will see them separately.

[nikitabobko]

Thinking out loud. I disregard the downside, because we are in the territory of binary compatibility. We did our best. In the binary signature, it's not possible to add textSize in the 2rd version in the middle, period.

If Kotlin users care about consistency between Java and Kotlin clients, the users should just always add IntroducedAt optional parameters at the end. If Kotlin users don't care about the consistency, but prefer to make life of their Kotlin clients slightly better - they can add optional parameters annotated with IntroducedAt however they like.

[faizilham]

The current proposal doesn't allow adding new optional parameters in the middle, only to the end.

This is an implementation detail, but this can be a suppressible warning instead of an error. If authors think it is worth the risk of source incompatibility, they can choose to suppress the warning. When the enforced named parameter feature is fully implemented, we can change this to only allow named-only parameters when adding new parameters in the middle.

Another idea is to add a function annotation to allow adding the parameters in the middle, and add a checker to check that such functions must have its arguments provided by names. However, I think this overlaps too much with the named parameters proposal for a temporary solution.

What if we sorted the parameters in the binary signature in the following way? [...]

Wouldn't this also be a problem to new Kotlin sources which call the function without named arguments? Since we currently can't enforce providing arguments by name, if they call poorJava(1, 2, 3), in the binary it would mean background=2, textSize=3, while in the source it would imply textSize=2, background=3

[nikitabobko]

This is an implementation detail, but this can be a suppressible warning instead of an error. If authors think it is worth the risk of source incompatibility, they can choose to suppress the warning

Yes, sure.

Sorry, I realized that I wasn't clear on what benefits does my suggestion have over the current proposal. I don't have really a good example, my intuition says that it's better to add new parameters at the end.

To demonstrate the intuition, let's look at the feature interaction with @JvmOverloads

@JvmOverloads
fun baz(
    a: Int,
    @IntroducedAt("1") b: Int = 1,
    c: Int = 1,
) = Unit
// Binary signatures according to the current proposal:
// baz(a)
// baz(a, c)
// baz(a, b, c)
//
// Binary signatures according to my suggestion:
// baz(a)
// baz(a, c)
// baz(a, c, b)

Both of the approached generate binary compatible signatures. Both of approaches generate source compatible signatures for clients from Java. But which one is more readable and causes less surprise for Java developers is a good question, for which I don't have an immediate answer

On one hand, the current proposal is better, because in the fully expanded form, the parameters go in the exactly the same order as they do in the Kotlin source file.

On the other hand, signatures in my suggestion look better from the evolution point of view.

(BTW, I think the proposal would benefit from a separate section on feature interaction with @JvmOverloads)

But I do have one practical advantage of my suggestion!

The current proposal is not always source compatible for Java clients, but my suggestion is always source compatible for Java clients (it's still not source compatible for Kotlin clients, but this will be resolved in future by named parameters feature).

Example:

// Kotlin
// FILE: file.kt
fun bar(
    @IntroducedAt("1") b: Int = 1,
    vararg a: Int,
) = Unit
// Binary signatures according to my suggestion:
// bar(vararg a)
// bar(vararg a, b)
//
// Binary signatures according to the current proposal:
// bar(vararg a)
// bar(b, vararg a)

In version 0, Java compiles fine. In version 1, Java will fail with reference to bar is ambiguous message.

Wouldn't this also be a problem to new Kotlin sources which call the function without named arguments?

My suggestion is to reorder parameters only in the binary signature, but keep the source signature intact (we can only do it for Kotlin sources, Java will always follow the binary signature). In the source, in Kotlin, poorJava(1, 2, 3) means textFont = 1, textSize = 2, background = 3, but in Java, poorJava(1, 2, 3) would mean textFont = 1, background = 2, textSize = 1

[faizilham]

The easier compatibility with JvmOverloads is indeed a plus point, although I'm still concerned with how it affect the non-deprecated signature though. In the current proposal, adding or removing IntroducedAt annotation never affect the "active" signature, and it only adds or removes hidden overloads.

I do not know how common this would be, but I can imagine that some library authors only use this feature for a temporary measure to bridge binary compatibility, and they might want to remove the annotation with a very old version numbers to reduce the number of hidden overloads and binary size.

Of course removing the annotation will always break binary compatibility, but in the current proposal the effect is limited to some hidden overloads and never break the active signature.

(BTW, I think the proposal would benefit from a separate section on feature interaction with @JvmOverloads)

yeah, I agree.

[lelandrichardson]

Thank you for this proposal, I believe it would be helpful in reducing some of the maintenance pain we experience with Compose.

Version String

My biggest bit of feedback is around the constraints on the version string. The proposed format requirements would likely work just fine for Compose, however it seems like an unnecessary constraint to put on the parameter, and some libraries which might want to specify more specific versions which may have alpha characters shouldn't be prevented from using this feature unnecessarily in that case. I believe having all strings be valid seems desirable.

Related, however, is how the version strings are ordered if you open them up to be unconstrained. I think the main requirement here would be to preserve the "expected" behavior for version strings like "1.9.0" being earlier in the sequence than something like "1.10.0", (so not a standard alphanumeric sort). The linked Maven sort order would work fine, but I think the main requirement here is that it is well specified / well known and resolves cases like the one above "correctly".

Source vs Binary Compatibility

As a broader point, in Compose, we have a strict binary compatibility requirement, but no such source compatibility requirement. Of course we strive for source compatibility, however it is often impractical or impossible. A big reason for this is due to call sites being able to take the form of named/unnamed arguments, with the last parameter being able to be specified with trailing lambda syntax for lambdas or fun interfaces. We have also done this as a result of the overloads being otherwise ambiguous due to the types of two semantically different parameters being the same. It is our hope that the required named parameters proposal will help dramatically in being able to ensure source compatibility more often.

For this reason, I suspect Validation (5) and (6) would have a few instances in the Compose code base where they would need to be suppressed if we were to refactor manually written deprecated overloads in favor of a single API with @IntroducedAt annotations across the code base. Despite that, I am not opposed to these being warnings or errors as long as they are suppressible, especially with the potential future where the use of "require named" would be encouraged when these conditions were violated.

[faizilham]

Hi, thank you for your feedback!

Version string

My initial reservation about allowing all strings to be valid is to prevent confusion about the ordering and possible mistakes like writing 1..1 instead of 1.1. I also still don't fully see a good example where you want to version a parameter with something like 1.2-beta instead of just the intended release version 1.2. However, if the community think that the proposed constraint is too restrictive, I think the Maven comparable version is a good compromise:

1. All strings are valid Maven version (for example, 1..1 is equivalent to 1.0.1)
2. There is a specified ordering, and it can handle cases like 1.9 < 1.10 < 1.99
3. It can also handle cases with common qualifiers, such as 1.2-alpha9 < 1.2-alpha10 < 1.2-beta < 1.2-rc < 1.2
4. There is already a well-tested implementation in the compiler

We can also have more extensive explanations and ordering examples in the documentation to disambiguate the ordering.

Source compatibility
Validation (5) and (6) indeed give suppressible warning, so at least there is a work around for it. There is an argument to be made for disabling these until forced-named parameter feature is implemented, although I lean more into suppressible warning to ease future migration to forced-named parameter.

Although I'm curious regarding your point about validation (5), do you have an example where you want to have a named optional parameter preceding a non-optional, non-trailing-lambda parameter?

[lelandrichardson]

I think i misread Validation (5) initially. I think the situation I was interpreting this as beforehand was the case for adding @IntroducedAt parameters before some parameters which were "version zero". So something like:

fun xyz(x: X, y: @IntroducedAt("2") Y = Y(), z: Z: Z())

I don't have an example off the top of my head, but I am pretty sure there have been somewhat nuanced cases where we have decided to introduce a new parameter "in the middle", where the result is backwards compatible but less source compatible than it could otherwise be.

For example, say the last parameter is a lambda

fun example(a: A, b: B = B(), c: C = C(), d: () -> Unit = {})

In this case, if we want to add a new optional lambda parameter, we are a little bit stuck. We have the following possibilities:

1. Add it to the end:

fun example(a: A, b: B = B(), c: C = C(), d: () -> Unit = {})
fun example(a: A, b: B = B(), c: C = C(), d: () -> Unit, e: () -> Unit = {})

2. Add it before trailing lambda

fun example(a: A, b: B = B(), c: C = C(), d: () -> Unit = {})
fun example(a: A, b: B = B(), c: C = C(), e: () -> Unit = {}, d: () -> Unit = {})

3. Add it "in the middle":

fun example(a: A, b: B = B(), c: C = C(), d: () -> Unit = {})
fun example(a: A, b: B = B(), e: () -> Unit = {}, c: C = C(), d: () -> Unit = {})

We can assume that the following types of call sites already existed:

// A
example(a,b,c,d)
// B
example(a,b,c) { ... }
// C
example(a = a, b = b, c = c) { }
// D
example(a = a, b = b, c = c, d = d)

All of the options (1), (2), and (3) are binary compatible, so it's a question of source compatibility and behavioral compatibility. Source compatibility is "does it still compile", but there is also behavior compatibility: "does it still compile to the intent of the original code"?

Option (1) is source compatible with A and D, but only behavior compatible with D
Option (2) is source compatible with A, B, C, and D, but NOT behavior compatible with A
Option (3) is source compatible with C and D, and behavior compatible with C and D

This is a difficult set of tradeoffs. Basically we view behavior compatibility here as much more important than source compatibility, because source compatibility will trigger an error and allow for a developer to do an easy fix. The behavior compatibility will easily go unnoticed and cause bugs.

There's also the likelihood of each call site. This is API dependent, but I would say that C is the most common for composable APIs, with A being somewhat common as a result of "wrapping" functions that attempt to clone the API and just wholesale pass most of their arguments:

fun fancyExample(a, b, c, d) {
  example(makeFancy(a), b, c, d)
}

Basically, Option (3) starts to look appealing because it is never source compatible but behavior incompatible at the same time.

This example might seem a bit contrived, but I know we've hit this at least once, though i think it was probably a few times. There might be a lesson here of avoiding nullable trailing lambda parameters.

[lelandrichardson]

Note that the proposal to require named parameters solves the source incompatibility completely with the above example

[faizilham]

Either options are fine, so I think this choice should be left to the library authors. Option (3) will generate a warning until we have forced-named parameter, but as we discussed before the warning can be suppressed.