@SignatureDiscriminator review - #503
InversionSpaces wants to merge 3 commits into
Conversation
|
Please merge your proposal directly to the 'main' branch before opening a public KEEP discussion. |
There was a problem hiding this comment.
What is our stance on non-migration usages of @SignatureDiscriminator?
The following should be allowed and not generate a warning?
private fun foo(a: String) {}
@SignatureDiscriminator("foo-b")
private fun foo(b: String) {}I guess it could be useful for swift import possibly.
EDIT: Oh, I see, just started to read the interop KEEP.
|
As a general comment, the fact that Export model is separate from the "core proposal" feels very weird to me. In particular, I would expect the problem of open members to play a much more important role. Currently the text only states:
But I find it very hard to understand why this is the case. |
|
It would be nice if the proposal also outlined which restrictions could be dropped from the language once |
|
Am I correct that apart from |
| On other platforms the backend encodes it in the declaration's binary identity. | ||
|
|
||
| For example, on the JVM, the method descriptor has no field for a discriminator, so the compiler encodes it in the JVM method name. | ||
| Conceptually, the emitted name consists of the Kotlin source name followed by a stable encoding of the discriminator, while Kotlin metadata continues to expose the original source name. |
There was a problem hiding this comment.
So, this function would be not callable from java? Or how exactly should it looks like?
There was a problem hiding this comment.
Yes, if you want the function which uses a Kotlin-specific discriminator to be nicely callable from Java, you'd need to use the proper export tooling (or the useAsExportName shortcut if it stays in the final design)
| } | ||
| ``` | ||
|
|
||
| but is applied to all derived entities: property itself, backing field, getter and setter, etc. |
There was a problem hiding this comment.
But we can apply it specifically to the getter/setter (but not field btw). What does it mean?
There was a problem hiding this comment.
I didn't really intend for @SignatureDiscriminator to be applicable for getters/setters separately, considering how tightly coupled our resolution is for properties (that's why it does not have these targets as available). Do you see a use-case for that? Or am I simply missing this restriction being explicitly stated?
| For example, these Objective-C selectors could be imported with distinct synthesized discriminators even though their Kotlin parameter types are identical: | ||
|
|
||
| ```kotlin | ||
| @SignatureDiscriminator("objc:tableView:titleForHeaderInSection:") // synthesized |
There was a problem hiding this comment.
It's a bit confusing in example, that tableView is both function and first parameter name. not really clear which of two is used in the string.
And as far as I remember it replaces name, not adds to it, so I'm not sure if we can compatibly switch to new behavior.
There was a problem hiding this comment.
Good point, I'll update to the tableView(of:..., title...) shape to make the generated selectors unambiguous (aka the first param is ignored).
And as far as I remember it replaces name, not adds to it, so I'm not sure if we can compatibly switch to new behavior.
Didn't fully get that part, what replaces name?
There was a problem hiding this comment.
It's a bit confusing in example, that tableView is both function and first parameter name.
In fact, the real platform libraries have exactly this: tableView functions with tableView as the first parameter name, e.g.
public open fun tableView(tableView: platform/UIKit/UITableView, titleForHeaderInSection: kotlin/Long /* = platform/darwin/NSInteger^ */): kotlin/String?
And that's what the real platform SDK has: https://developer.apple.com/documentation/uikit/uitableviewdatasource/tableview(_:titleforheaderinsection:)?language=objc.
So, the current version with tableView(of: is also confusing.
|
TL;DR version of the update
|
@serras not really, IIRC the main problem with internal members on interfaces is that they are weird. A public interface with an abstract internal member cannot be implemented in another module, it can only be used there. Which makes it a strange flavor of Signature discriminators do nothing here. |
|
@ice-phoenix How to implement them abstract internal members is part of the problem. But I think we also do some mangling of non-abstract members, so that they don't clash with potential future implementors outside that module. This is the part where I though signature discriminator could somehow help. |
| The discriminator value could become the JVM name, JavaScript export name, Objective-C selector base, and WebAssembly export name. | ||
|
|
||
| > Note: in essence, this is the previously proposed `@BinarySignatureName` design. |
There was a problem hiding this comment.
Is it?
The previously proposed design that I know of says:
The platform interoperability uses different mechanisms and is not affected by the specified signature name. The exported name of declaration for JavaScript, C/C++, Objective-C, and Swift is still derived from the source name of declaration and could be customized, if needed, by separate platform-specific annotations.
| For example, these Objective-C selectors could be imported with distinct synthesized discriminators even though their Kotlin parameter types are identical: | ||
|
|
||
| ```kotlin | ||
| @SignatureDiscriminator("objc:tableView:titleForHeaderInSection:") // synthesized |
There was a problem hiding this comment.
It's a bit confusing in example, that tableView is both function and first parameter name.
In fact, the real platform libraries have exactly this: tableView functions with tableView as the first parameter name, e.g.
public open fun tableView(tableView: platform/UIKit/UITableView, titleForHeaderInSection: kotlin/Long /* = platform/darwin/NSInteger^ */): kotlin/String?
And that's what the real platform SDK has: https://developer.apple.com/documentation/uikit/uitableviewdatasource/tableview(_:titleforheaderinsection:)?language=objc.
So, the current version with tableView(of: is also confusing.
| } | ||
| ``` | ||
|
|
||
| #### Why not implicitly inherit the platform name? |
There was a problem hiding this comment.
I think @ObjCName is inherited implicitly at the moment and can't even be used on an override.
Does this proposal suggest changing that?
| With `useAsExportName = false`, no implicit `@JsName` is applied. | ||
| The core proposal scheme is used then. | ||
|
|
||
| #### Kotlin/Native: Objective-C and Swift export |
There was a problem hiding this comment.
What about C Export with its @CName?
Even if we don't derive it from the discriminator in this proposal, we should at least mention that.
| // Objective-C selector: stringValueIndex: | ||
| ``` | ||
|
|
||
| The Swift-facing name remains derived from the Kotlin source name, so the same declaration is presented as `value(index:)` through `NS_SWIFT_NAME`. |
There was a problem hiding this comment.
What is the motivation behind this decision?
|
|
||
| ### Synthesized discriminators | ||
|
|
||
| A platform import may attach a synthesized discriminator when two native declarations would otherwise merge to the same Kotlin declaration signature. |
There was a problem hiding this comment.
Why not always use a discriminator when a clash is possible? Synthesizing conditionally might be a bit harmful.
Imagine we have an imported declaration
open class C {
fun foo(x: Int)
}Now, the new version of the platform SDK gets foo(y: Int), so we have
open class C {
@SignatureDiscriminator("fooX")
fun foo(x: Int)
@SignatureDiscriminator("fooY")
fun foo(y: Int) {}
}With the added discriminators, per "may attach a synthesized discriminator when two native declarations would otherwise merge".
Which means that Kotlin callsites of foo(x: Int) become binary broken when updating the SDK.
| ### Objective-C import | ||
|
|
||
| This is most useful for Objective-C import, where parameter names (selector) are an important part of a callable signature. | ||
| The importer records a selector-derived discriminator for imported functions that require one. |
There was a problem hiding this comment.
that require one.
What does that mean?
|
|
||
| Existing source compatibility is preserved as follows. | ||
|
|
||
| When an override has exactly one compatible imported Objective-C member, the compiler adopts the inherited selector-derived discriminator implicitly. |
There was a problem hiding this comment.
compatible
Important to note here is that this compatibility doesn't follow Kotlin rules at the moment. Instead, it always checks parameter names, even when there is no clash.
5 participants
No description provided.