Binary Signature Name proposal #303
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
lounres
reviewed
Jun 4, 2022
Comment on lines
+430
to
+432
| Objective-C has relaxed rules on overrides. In particular, it is possible to change the parameter types in Object-C on override. | ||
| These relaxed checks are implemented only in Kotlin/Native for Kotlin code that is produced as a result of ObjC-interop tooling and | ||
| their actual specifications are out of the scope of this KEEP. |
There was a problem hiding this comment.
Objective-C is called "Objective-C" (the 1st sentence), "Object-C" (the 1st sentence) and "ObjC" (the 2nd sentence) at the same time. Is it a typo or intended?
There was a problem hiding this comment.
Object-C is a typo, while an ObjC is a proper abbreviation.
|
|
||
| In general, it is not in Kotlin style to have functions that different only in parameter name like it is customary | ||
| in Objective-C and Swift, for example. In Kotlin you'd give these functions different names. However, it is conceivable | ||
| that some domain-specific DSLs might want to have this kind of feature. The following code does not compile: |
There was a problem hiding this comment.
There is phrase "domain-specific DSLs" (the 3rd sentence) which is equal to "domain-specific domain-specific languages". Is the doubling a typo or intended?
There was a problem hiding this comment.
Typo. Should be just DSLs.
lounres
reviewed
Jun 4, 2022
Co-authored-by: Gleb Minaev <43728100+lounres@users.noreply.github.com>
udalov
reviewed
Sep 2, 2022
| } | ||
|
|
||
| class Derived : Base() { | ||
| @SingatureName("foo") |
There was a problem hiding this comment.
Suggested change
| @SingatureName("foo") | |
| @BinarySignatureName("foo") |
udalov
reviewed
Sep 2, 2022
| } | ||
|
|
||
| class Derived : Base() { | ||
| @SingatureName("bar") |
There was a problem hiding this comment.
Suggested change
| @SingatureName("bar") | |
| @BinarySignatureName("bar") |
ice-phoenix
reviewed
Sep 8, 2022
Comment on lines
+194
to
+195
| Normally, such overloads, that are different only in parameter names, are not allowed in Kotlin, so a special | ||
| Objective-C specific rules are currently used for Objective-C interoperability. However, these Objective-C-specific |
There was a problem hiding this comment.
Suggested change
| Normally, such overloads, that are different only in parameter names, are not allowed in Kotlin, so a special | |
| Objective-C specific rules are currently used for Objective-C interoperability. However, these Objective-C-specific | |
| Normally, such overloads, that are different only in parameter names, are not allowed in Kotlin, so special | |
| Objective-C specific rules are currently used for Objective-C interoperability. However, these Objective-C-specific |
An indefinite article is almost never used with plurals, it can be dropped
| Kotlin uses different signatures in different stages of compilation process: | ||
|
|
||
| * [Call resolution signature](#call-resolution-signature) | ||
| * [Override matchine signature](#override-matching-signature) |
There was a problem hiding this comment.
Suggested change
| * [Override matchine signature](#override-matching-signature) | |
| * [Override matching signature](#override-matching-signature) |
| The change of parameter names is binary compatible in Kotlin, but not, in general, source compatible. | ||
| * It includes a return type. That makes it different from the call resolution signature, too. | ||
| While the change of the return type to a more specific type can be mostly source compatible, it is not a binary compatible change. | ||
| * It use the name specified in `@BinarySignatureName`, |
There was a problem hiding this comment.
Suggested change
| * It use the name specified in `@BinarySignatureName`, | |
| * It uses the name specified in `@BinarySignatureName`, |
| #### Override matching check | ||
|
|
||
| Declarations are grouped by BOTH a source name and a signature name to detect conflicting | ||
| overloads using the current Kotlin rules. That is, in each, group pairs of declarations are checked w.r.t. their |
There was a problem hiding this comment.
Suggested change
| overloads using the current Kotlin rules. That is, in each, group pairs of declarations are checked w.r.t. their | |
| overloads using the current Kotlin rules. That is, in each group, pairs of declarations are checked w.r.t. their |
| The specified signature name affects linking of the resulting binary and thus has the following platform-specific effects: | ||
|
|
||
| * **Kotlin/JVM**: uses JAR file format as the binary artifact produced by the compiler, so the signature name is used as | ||
| a default value of `@JvmName` to ensure that it has the desired effect on the resulting binary signature. |
There was a problem hiding this comment.
Suggested change
| a default value of `@JvmName` to ensure that it has the desired effect on the resulting binary signature. | |
| a default value of `@JvmName` to ensure that it has the desired effect on the resulting binary signature. |
In Markdown, double space at line end forces a hard line break
| The Kotlin standard library uses `@SinceKotlin("...")` and `@DeprecatedSinceKotlin(hiddenSince = "...")` to hide | ||
| declarations conditionally, based on the current API version in use (the value of the `-api-version` argument | ||
| when compiling the module where the usage is located). For the purposes of **call resolution** check, | ||
| they will taken into account, too: |
There was a problem hiding this comment.
Suggested change
| they will taken into account, too: | |
| they are taken into account, too: |
| they will taken into account, too: | ||
|
|
||
| * Declarations with `@SinceKotlin(version)`, when the current API version is below the specified version, are excluded from the check. | ||
| * Declarations with `@DeprecatedSinceKotlin(hiddenSince = version)`, when the current API version is the same or more recent as the version, are excluded from the check. |
There was a problem hiding this comment.
Suggested change
| * Declarations with `@DeprecatedSinceKotlin(hiddenSince = version)`, when the current API version is the same or more recent as the version, are excluded from the check. | |
| * Declarations with `@DeprecatedSinceKotlin(hiddenSince = version)`, when the current API version is the same or more recent as the specified version, are excluded from the check. |
| fun addSection(footer: String) { /* ... */ } | ||
| ``` | ||
|
|
||
| > In fact, only them is required to have `@BinarySginatureName` name annotation to compile this code. |
There was a problem hiding this comment.
Suggested change
| > In fact, only them is required to have `@BinarySginatureName` name annotation to compile this code. | |
| > In fact, only one of them is required to have `@BinarySginatureName` name annotation to compile this code. |
| ``` | ||
|
|
||
| The `@BinarySignatureName` annotation affects the resolution algorithm in the compiler on a deep level, so this annotation, | ||
| by itself, has to be resolved before everything else. Hence, there is an additional restrictions on usage of this annotation: |
There was a problem hiding this comment.
Suggested change
| by itself, has to be resolved before everything else. Hence, there is an additional restrictions on usage of this annotation: | |
| by itself, has to be resolved before everything else. Hence, there are additional restrictions on usage of this annotation: |
| a great deal of type information that is otherwise available in Kotlin. It means, that a valid change during separate | ||
| compilation of different libraries can produce a pair of libraries that link together successfully, yet are not | ||
| consistent from the Kotlin's type-system point of view. Trying to build those after linking into a platform binary | ||
| would either crash the compiler or produce a broken executable. To avoid that, we would need to specify an implement |
There was a problem hiding this comment.
Suggested change
| would either crash the compiler or produce a broken executable. To avoid that, we would need to specify an implement | |
| would either crash the compiler or produce a broken executable. To avoid that, we would need to specify and implement |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Please, use this PR for minor corrections (like typos and text style) and use KEEP-302 for the discussion on the substance of the proposal.