Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Binary Signature Name #302

Open
elizarov opened this issue Jun 2, 2022 · 7 comments
Open

Binary Signature Name #302

elizarov opened this issue Jun 2, 2022 · 7 comments

Comments

@elizarov
Copy link
Contributor

elizarov commented Jun 2, 2022

This proposal introduces @BinarySignatureName annotation that serves as a cross-platform variant of @JvmName
annotation that is used in Kotlin/JVM and is designed to solve various library migration and Object-C interoperability use-cases.
This annotation affects overload matching and linking of cross-platform Kotlin libraries.

For convenience of Object-C interoperability, a helper @ObjCSignature annotation is introduced that makes Objective-C
interoperability more straightforward as shown later.

See binary-signature.md for the full text of the proposal, use-cases, detailed design, and open issues. Use PR #303 for minor corrections (like typos and text style) and use this issue for the discussion on the substance of the proposal.

@lounres
Copy link
Contributor

lounres commented Jun 4, 2022

Hello! I have several questions about the proposal:

  1. What does phrase "@BinarySignatureName cannot be used via a typealias." mean? (It's in section "Detailed design > Details on BinarySignatureName annotation".)
  2. Look at such snippet:
    interface PolynomialRing<C, P> {
         operator fun C.plus(other: C): C = TODO()
         operator fun C.plus(other: P): P = TODO()
         operator fun P.plus(other: C): P = TODO()
         operator fun P.plus(other: P): P = TODO()
    }
    All of the plus functions are well disambiguated during call resolution. But they have the same binary signature on Kotlin/JVM public abstract plus(Ljava/lang/Object;Ljava/lang/Object;)Ljava/lang/Object. And (if I understand the proposal right) there is no ambiguity on other platforms. So I don't quite understand what the proposal advises me to do in this case?

@elizarov
Copy link
Contributor Author

elizarov commented Jun 6, 2022

lounres

What does phrase "@BinarySignatureName cannot be used via a typealias." mean? (It's in section "Detailed design > Details on BinarySignatureName annotation".)

It means that you cannot have, for example, typealias BSN = BinarySignatureName and then annotate your code with @BSN.

  1. Look at such snippets interface PolynomialRing<C, P> ...

You cannot currently compile this code on JVM, and you cannot work around the problem with @JvmName, because those are all open functions. With this proposal, though, you'll be able to write:

// Compiles fine with this proposal
interface PolynomialRing<C, P> {
     @BinarySignatureName("plusCC")
     operator fun C.plus(other: C): C = TODO()
     @BinarySignatureName("plusCP")
     operator fun C.plus(other: P): P = TODO()
     @BinarySignatureName("plusPC")
     operator fun P.plus(other: C): P = TODO()
     @BinarySignatureName("plusPP")
     operator fun P.plus(other: P): P = TODO()
}

@sfs
Copy link

sfs commented Jun 27, 2022

How does this feature interact with special bridge methods for builtins with different binary names on the JVM?

If I understand the proposal correctly then users will have to add manual bridge methods when changing binary names. E.g.,

interface A<T> {
  fun foo(x: T)
}

class B : A<String> {
  @BinarySignatureName("bar")
  open fun foo(x: String) {}
  
  @Deprecated("Binary compatibility", level = DeprecationLevel.HIDDEN)
  override fun foo(x: String) { this.foo(x) }
}

The same issue already exists for Kotlin collection methods on the JVM, where we have Kotlin functions with different names compared to the binary names in (Java) interfaces. In this case we generate such bridge methods implicitly.

I think we should disable automatic bridge generation in the presence of @BinarySignatureName. One complication here is that the issue with conflicting fake overrides already exists in Kotlin collections on the JVM.

For example, java.util.Collection<T> has a method contains(x: Any?): Boolean while the Kotlin Collection<T> has a method contains(x: T): Boolean. Both methods compile to the same binary signature, which has visible effects for Kotlin implementations of Collection.

In this case there is no way to produce the correct bridge methods manually. Even if it was, these bridges are fragile. For example, in order for the resulting class to be subclassable from Java we need to ensure that we don't generate a generic signature.

@sfs
Copy link

sfs commented Jul 26, 2022

I think there are two ways to deal with renamed declarations from the Kotlin/JVM standard library. In order to allow any signatures on overrides we should add an additional override matching signature corresponding to the binary signature. This has to be present in addition to matching the declarations without signatures which is what we currently do.

Now the only choice is whether this becomes a special case on the JVM - which I would strongly prefer - or if we want to have the same corner case on all platforms.

For the latter we could just add something like the following text immediately after Objective-C overrides in Kotlin/Native

#### Standard library declarations with multiple override matching signatures

For Java compatibility, the Kotlin standard library on Kotlin/JVM contains methods with implicitly renamed binary signatures.
In order to allow these methods to be overridden without introducing additional bridge methods the following declarations have additional override matching signatures.

| Class                                  | Declaration                         | Additional BinarySignatureName |
| -------------------------------------- | ----------------------------------- | ------------------------------ |
| `kotlin.collections.MutableList<T>`    | `fun removeAt(index: Int): T`       | `remove`                       |
| `kotlin.Number`                        | `fun toByte(): Byte`                | `byteValue`                    |
| `kotlin.Number`                        | `fun toShort(): Short`              | `shortValue`                   |
| `kotlin.Number`                        | `fun toInt(): Int`                  | `intValue`                     |
| `kotlin.Number`                        | `fun toLong(): Long`                | `longValue`                    |
| `kotlin.Number`                        | `fun toFloat(): Float`              | `floatValue`                   |
| `kotlin.Number`                        | `fun toDouble(): Double`            | `doubleValue`                  |
| `kotlin.CharSequence`                  | `fun get(index: Int): Char`         | `charAt`                       |
| `kotlin.CharSequence`                  | `val length: Int`                   | `length`                       |
| `kotlin.collections.Collection<out T>` | `val size: Int`                     | `size`                         |
| `kotlin.collections.Map<K, out V>`     | `val size: Int`                     | `size`                         |
| `kotlin.collections.Map<K, out V>`     | `val keys: Set<K>`                  | `keySet`                       |
| `kotlin.collections.Map<K, out V>`     | `val values: Collection<V>`         | `values`                       |
| `kotlin.collections.Map<K, out V>`     | `val entries: Set<Map.Entry<K, V>>` | `entrySet`                     |

However, this creates additional implementation concerns for non-JVM platforms for no good reason.

Instead I would propose that as part of this KEEP we update JvmName to essentially be an alias of BinarySignatureName with the following caveats:

  • If a declaration has both a JvmName and a BinarySignatureName then the JvmName takes precedence on Kotlin/JVM. Otherwise it is ignored anyway. Additionally we have to extend the override matching rule to state that both the binary signature name and the jvm name must match on overrides.
  • The list of renamed builtin declarations above has an additional implicit JvmName rather than an additional BinarySignatureName for the purposes of override matching. This means that these declarations can be overridden with an additional JvmName but it's not possible to add a BinarySignatureName override.

Since JvmName is currently restricted to final declarations this extension should not affect any current users. This seems like a reasonable trade-off to me and the ability to have platform specific binary names is probably useful in other cases as well.

@elizarov
Copy link
Contributor Author

@sfs Thanks for the notice on special bridges. Indeed, we must take them into account in the BinarySignatureName design. We need to clarify how @BinarySignatureName interacts with special bridges on JVM. We need to specify what happens if you use @BinarySignatureName on a specially-bridged declaration.

However, I don't see how your suggestions on "implicit JvmName" is going to help here. See, the way these special bridges work is through the generation of a bridge. E.g., if you override Number.toByte() the corresponding JVM name of the corresponding method is still toByte and it still implements, for example, any Kotlin interface with the corresponding toByte function. A bridge is an additional byteValue method that gets generated. In contrast, both JvmName and BinrarySignatureName are designed to change the underlying JVM of a declaration; no bridge is generated.

Can you explain a little bit more here a specific problem with special bridges that you are trying to solve, please?

@sfs
Copy link

sfs commented Aug 9, 2022

@elizarov Maybe I'm reading the specification incorrectly, because from your response it looks like the intention is that declarations with BinarySignatureNames can override standard library methods with different (binary) names?

My reading was that you cannot override a specially-bridged declaration with a function with an @BinarySignatureName annotation, since the override signatures won't match.

Can you explain a little bit more here a specific problem with special bridges that you are trying to solve, please?

The main problem with special bridge methods is that they are generated as final (to avoid users overriding one but not the other from Java). When we migrate existing Java libraries to Kotlin we typically have to avoid binary incompatible changes and changing a method to final is a binary incompatible change. It would be useful to have some mechanism to opt-out of the special bridge machinery for a given class.

In KT-52897 I suggested relaxing the restrictions on JvmName annotations to avoid special bridge generation. According to my reading of the current specification we cannot use BinarySignatureName to do this, because the standard library methods in question do not have BinarySignatureName annotations. To be precise, I think we would reject the following code.

abstract class A : Collection<String> {
  @get:BinarySignatureName("size")
  abstract override val size: Int
}

The size property does not override the val size: Int declaration in Collection due to different override signatures.

This gets more complicated when we inherit implementations with potentially different names. Consider the following use case.

open class A {
  @get:BinarySignatureName("size")
  val size: Int
    get() = ...
}

class B : A(), Collection<String> { ... }

At the moment we would have two size properties in B. The one from A has an explicit override signature name size, while the one in Collection has an override signature name getSize. Since the signatures are different, the two properties are different and we don't consider A.size as an implementation for Collection.size.

After bridge generation on the JVM this code would result in a platform declaration clash, since we would try to generate a size bridge in B for the generated getSize method.

The first issue here is that outside of the JVM this code is fine and this is the correct behavior. The standard library function in question has a binary name of getSize and if we allow an "override" with binary name size we would now need to introduce (final) bridge methods on the non-JVM backends. This does not seem like a good idea, which is why I would prefer not to assign any special behavior to BinarySignatureName for standard library methods that happen to have different binary names on the JVM.

On the other hand, on the JVM it would be extremely useful to be able to override these standard library methods without introducing additional bridge methods and for that we would need to consider, e.g., a declaration of the form @JvmName("byteValue") override fun toByte(): Byte = ... as a valid override of the corresponding declaration in Number. That's what I meant by having "additional JvmName" annotations on these declarations.

With such an override matching rule in place we would not generate a bridge, because both the declaration in the standard library and its implementation would have the same binary name.

@elizarov
Copy link
Contributor Author

@sfs The main problem with special bridge methods is that they are generated as final (to avoid users overriding one but not the other from Java). When we migrate existing Java libraries to Kotlin, we typically have to avoid binary incompatible changes and changing a method to final is a binary incompatible change. It would be useful to have some mechanism to opt-out of the special bridge machinery for a given class.

The problem is more complicated than opting out of special bridge generation. Ideally, you should be able to manipulate those special bridges without any hard-coded compiler magic. That is, all the specially-bridged collection declarations shall be marked with some new kind of JVM-specific annotations that would drive the generation of the special bridges, as opposed to them being hard-coded into the compiler. And their design, indeed, shall also include an ability to opt-out of the final bridge generation to write it manually.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants