From 0675ef8472f5c18314cc811b0d6060bd0e9ac48b Mon Sep 17 00:00:00 2001 From: juangamnik Date: Sat, 16 Jul 2016 19:15:16 +0200 Subject: [PATCH 1/3] Create dispatching-of-reified-types.md --- proposals/dispatching-of-reified-types.md | 135 ++++++++++++++++++++++ 1 file changed, 135 insertions(+) create mode 100644 proposals/dispatching-of-reified-types.md diff --git a/proposals/dispatching-of-reified-types.md b/proposals/dispatching-of-reified-types.md new file mode 100644 index 000000000..b9dba7c31 --- /dev/null +++ b/proposals/dispatching-of-reified-types.md @@ -0,0 +1,135 @@ +# Dispatching of reified types + +* **Type**: Design proposal +* **Author**: Johannes Neubauer +* **Status**: Pull request initiated +* **Prototype**: *not yet* + +## Feedback + +Discussion of this proposal has been done in [this issue](https://youtrack.jetbrains.com/issue/KT-12897) +so far. An issue in the KEEP project has to be opened. + +## Summary + +Support dispatching of method calls for reified types in inline functions. + +## Motivation + +Currently, the function body is copied as is, but since kotlin has a multipass compiler it should be possible to recheck the +type after copying. For now, you need to do the dispatch manually (which is error-prone). + +## Description + +The following example shows, that the current behavior is different from non-inline functions. The scenario takes place in +the context of libGDX in an android application: + +```kotlin +object Actions { + // other stuff ... + inline fun action(): T { + val pool = Pools.get(T::class.java) + val action = pool.obtain() + action.pool = pool + return action + } +} + +inline fun T.copy(): T { + val copyAction = Actions.action() + println(""" + |The reified types are: ${T::class.java.name}, + | ${this.javaClass.name}, + | ${copyAction.javaClass.name} + |""".trimMargin()) + // this leads to the UnsupportedOperationException + // (although the output is: "The reified types are: \ + // com.badlogic.gdx.scenes.scene2d.actions.SequenceAction\n \ + // com.badlogic.gdx.scenes.scene2d.actions.SequenceAction"): + this copyTo copyAction + // Using the following instead works ("that's good" is printed out): + // (this as SequenceAction) copyTo (copyAction as SequenceAction) + return copyAction +} + +/** + * This is the "else"-case for `a copyTo b` since we use it in + * `copyInternal()` generically on `T: Action`. + */ +infix fun Action.copyTo(action: Action) { + throw UnsupportedOperationException( + """ |Tried to copy an (unsupported) action type + |'${this.javaClass.kotlin.simpleName}' + |""".trimMargin() + ) +} + +// there are more overloaded methods for copyTo + +infix fun ParallelAction.copyTo(action: ParallelAction) { + action.actions.addAll(this.actions) +} + +infix fun SequenceAction.copyTo(action: SequenceAction) { + println("that's good") + this as ParallelAction copyTo action +} + +// here it is called: +fun someMethod() { + val sequenceAction = SequenceAction() + sequenceAction.copy() +} +``` + +No macros are necessary for implementing this. The compiler should be able to inline the function body +and recheck without a macro since it is a multi-pass compiler. Naively, the inlining could be done in a +preprocessing step (not a macro) on the code-level and then compile it. + +## Open Questions + +*No, for the time being.* + +## Alternatives + +Use a big when-expression. But this is error-prone, since you have to conform to the order of inheritance. For example +`SequenceAction extends ParallelAction`, which means that all `SequenceAction`s are converted to `ParallelAction`s +unintendedly during copying: + +```kotlin +inline fun T.copy(): T { + return when (this) { + is AlphaAction -> { + val copyAction = Actions.action() + this copyTo copyAction + copyAction as T + } + + is ParallelAction -> { + val copyAction = Actions.action() + this copyTo copyAction + copyAction as T + } + is SequenceAction -> { + val copyAction = Actions.action() + this copyTo copyAction + copyAction as T + } + is VisibleAction -> { + val copyAction = Actions.action() + this copyTo copyAction + copyAction as T + } + else -> throw UnsupportedOperationException(""" + |Tried to copy an (unsupported) action type + |'${this.javaClass.kotlin.simpleName}' + |""".trimMargin() + ) + } + +} +``` + +Another alternative would be to use the visitor pattern or the sealed classes. But in an example like the one above, +i.e. you do not have access to the types, you cannot add the visitor pattern or sealed classes without reproducing the +whole type hierarchy. In order to add the visitor pattern, we would need dynamic dispatch of extension methods. From 84324c4bb42b3e523708808f9c01ec4b18d410e0 Mon Sep 17 00:00:00 2001 From: juangamnik Date: Thu, 25 Aug 2016 15:56:39 +0200 Subject: [PATCH 2/3] Update and rename dispatching-of-reified-types.md to type-checking-of-reified-types.md Updated proposal with respect to discussions on https://github.com/Kotlin/KEEP/pull/35 --- proposals/dispatching-of-reified-types.md | 135 ------------- proposals/type-checking-of-reified-types.md | 209 ++++++++++++++++++++ 2 files changed, 209 insertions(+), 135 deletions(-) delete mode 100644 proposals/dispatching-of-reified-types.md create mode 100644 proposals/type-checking-of-reified-types.md diff --git a/proposals/dispatching-of-reified-types.md b/proposals/dispatching-of-reified-types.md deleted file mode 100644 index b9dba7c31..000000000 --- a/proposals/dispatching-of-reified-types.md +++ /dev/null @@ -1,135 +0,0 @@ -# Dispatching of reified types - -* **Type**: Design proposal -* **Author**: Johannes Neubauer -* **Status**: Pull request initiated -* **Prototype**: *not yet* - -## Feedback - -Discussion of this proposal has been done in [this issue](https://youtrack.jetbrains.com/issue/KT-12897) -so far. An issue in the KEEP project has to be opened. - -## Summary - -Support dispatching of method calls for reified types in inline functions. - -## Motivation - -Currently, the function body is copied as is, but since kotlin has a multipass compiler it should be possible to recheck the -type after copying. For now, you need to do the dispatch manually (which is error-prone). - -## Description - -The following example shows, that the current behavior is different from non-inline functions. The scenario takes place in -the context of libGDX in an android application: - -```kotlin -object Actions { - // other stuff ... - inline fun action(): T { - val pool = Pools.get(T::class.java) - val action = pool.obtain() - action.pool = pool - return action - } -} - -inline fun T.copy(): T { - val copyAction = Actions.action() - println(""" - |The reified types are: ${T::class.java.name}, - | ${this.javaClass.name}, - | ${copyAction.javaClass.name} - |""".trimMargin()) - // this leads to the UnsupportedOperationException - // (although the output is: "The reified types are: \ - // com.badlogic.gdx.scenes.scene2d.actions.SequenceAction\n \ - // com.badlogic.gdx.scenes.scene2d.actions.SequenceAction"): - this copyTo copyAction - // Using the following instead works ("that's good" is printed out): - // (this as SequenceAction) copyTo (copyAction as SequenceAction) - return copyAction -} - -/** - * This is the "else"-case for `a copyTo b` since we use it in - * `copyInternal()` generically on `T: Action`. - */ -infix fun Action.copyTo(action: Action) { - throw UnsupportedOperationException( - """ |Tried to copy an (unsupported) action type - |'${this.javaClass.kotlin.simpleName}' - |""".trimMargin() - ) -} - -// there are more overloaded methods for copyTo - -infix fun ParallelAction.copyTo(action: ParallelAction) { - action.actions.addAll(this.actions) -} - -infix fun SequenceAction.copyTo(action: SequenceAction) { - println("that's good") - this as ParallelAction copyTo action -} - -// here it is called: -fun someMethod() { - val sequenceAction = SequenceAction() - sequenceAction.copy() -} -``` - -No macros are necessary for implementing this. The compiler should be able to inline the function body -and recheck without a macro since it is a multi-pass compiler. Naively, the inlining could be done in a -preprocessing step (not a macro) on the code-level and then compile it. - -## Open Questions - -*No, for the time being.* - -## Alternatives - -Use a big when-expression. But this is error-prone, since you have to conform to the order of inheritance. For example -`SequenceAction extends ParallelAction`, which means that all `SequenceAction`s are converted to `ParallelAction`s -unintendedly during copying: - -```kotlin -inline fun T.copy(): T { - return when (this) { - is AlphaAction -> { - val copyAction = Actions.action() - this copyTo copyAction - copyAction as T - } - - is ParallelAction -> { - val copyAction = Actions.action() - this copyTo copyAction - copyAction as T - } - is SequenceAction -> { - val copyAction = Actions.action() - this copyTo copyAction - copyAction as T - } - is VisibleAction -> { - val copyAction = Actions.action() - this copyTo copyAction - copyAction as T - } - else -> throw UnsupportedOperationException(""" - |Tried to copy an (unsupported) action type - |'${this.javaClass.kotlin.simpleName}' - |""".trimMargin() - ) - } - -} -``` - -Another alternative would be to use the visitor pattern or the sealed classes. But in an example like the one above, -i.e. you do not have access to the types, you cannot add the visitor pattern or sealed classes without reproducing the -whole type hierarchy. In order to add the visitor pattern, we would need dynamic dispatch of extension methods. diff --git a/proposals/type-checking-of-reified-types.md b/proposals/type-checking-of-reified-types.md new file mode 100644 index 000000000..344706f3d --- /dev/null +++ b/proposals/type-checking-of-reified-types.md @@ -0,0 +1,209 @@ +# Type-checking of reified types + +* **Type**: Design proposal +* **Author**: Johannes Neubauer +* **Status**: Shepherded +* **Shepherd**: [@dnpetrov](https://github.com/dnpetrov) +* **Prototype**: *not yet* + +## Feedback + +Discussion of this proposal has been done in [this issue](https://youtrack.jetbrains.com/issue/KT-12897) and in [this pull request](https://github.com/Kotlin/KEEP/pull/35). An issue in the KEEP project has to be opened. + +## Summary + +Support type-checking for reified types in inline functions. + +## Motivation + +Currently, the byte code of the function body of inline functions is copied as is. Since Kotlin has a multipass compiler it should be possible to recheck the type after copying. For now, you need to do the type-check manually (via a `when`-expression, the visitor-pattern, or sealed classes which is error-prone). + +## Description + +The following example shows, that the current behavior is different from non-inline functions. The scenario takes place in +the context of libGDX in an android application: + +```kotlin +object Actions { + // other stuff ... + inline fun action(): T { + val pool = Pools.get(T::class.java) + val action = pool.obtain() + action.pool = pool + return action + } +} + +inline fun T.copy(): T { + val copyAction = Actions.action() + println(""" + |The reified types are: ${T::class.java.name}, + | ${this.javaClass.name}, + | ${copyAction.javaClass.name} + |""".trimMargin()) + // this leads to the UnsupportedOperationException + // (although the output is: "The reified types are: \ + // com.badlogic.gdx.scenes.scene2d.actions.SequenceAction\n \ + // com.badlogic.gdx.scenes.scene2d.actions.SequenceAction"): + this copyTo copyAction + // Using the following instead works ("that's good" is printed out): + // (this as SequenceAction) copyTo (copyAction as SequenceAction) + return copyAction +} + +/** + * This is the "else"-case for `a copyTo b` since we use it in + * `copyInternal()` generically on `T: Action`. + */ +infix fun Action.copyTo(action: Action) { + throw UnsupportedOperationException( + """ |Tried to copy an (unsupported) action type + |'${this.javaClass.kotlin.simpleName}' + |""".trimMargin() + ) +} + +// there are more overloaded methods for copyTo + +infix fun ParallelAction.copyTo(action: ParallelAction) { + action.actions.addAll(this.actions) +} + +infix fun SequenceAction.copyTo(action: SequenceAction) { + println("that's good") + this as ParallelAction copyTo action +} + +// here it is called: +fun someMethod() { + val sequenceAction = SequenceAction() + sequenceAction.copy() +} +``` + +Currently, even this expression (replacing the respective line in `T.copy(): T`) uses the upper bound `Action` instead of the (reified) type used at call-site: + +```kotlin +(this as T) copyTo (copyAction as T) +``` + +The Kotlin documentation reads: +> We qualified the type parameter with the reified modifier, now it’s accessible +> inside the function, **almost as if it were a normal class.** + +-- https://kotlinlang.org/docs/reference/inline-functions.html#reified-type-parameters + +But in this case it does not behave as if it were a normal class although `return copyAction as T` is allowed. + +No macros are necessary for implementing this proposal. The compiler should be able to inline the function body +and recheck without a macro since it is a multi-pass compiler. Naively, the inlining could be done in a +preprocessing step (not a macro) on the code-level and then compile it. + +## Intended Behavior + +The concrete type of the reified type should be evaluated using the type at *call-site*. + +## Conflict Resolution + +There are cases where conflicts arise regarding the overload resolution in inline methods. The conflicts should be propagated to the *call-site*: + +```kotlin +// inline function declaration-site +interface A +fun A.foo() {} + +inline fun invokeFooOn(instance: T) = instance.foo() + +// inline function call-site +interface B +fun B.foo() {} + +class C : A, B + +fun test() { + C().foo() // throws an error: ambigous call + (C() as A).foo() // OK. + + invokeFooOn(C()) // (should) throw(s) the same error: ambigous call + invokeFooOn(C() as A) // OK. +} +``` + +Inline functions are inlined at compile-time. Operations involving reified type parameters are annotated with special bytecode instructions. Bytecode is post-processed at call-site, where corresponding type arguments are known. Hence, all information is available at compile-time to produce this error although one might argue that this results in leaking abstraction. + +## Open Questions + +1. Is the proposed conflict resolution (throwing an error that has been caused inside the implementation of the inline function) leak abstraction or is it a straightforward solution? +2. Is this change a "bug fix" (see the excerpt of Kotlin documentation above) or silently breaking a lot of code? + +## Alternatives + +Use a big when-expression. But this is error-prone, since you have to conform to the order of inheritance. For example +`SequenceAction extends ParallelAction`, which means that all `SequenceAction`s are converted to `ParallelAction`s +unintendedly during copying: + +```kotlin +inline fun T.copy(): T { + return when (this) { + is AlphaAction -> { + val copyAction = Actions.action() + this copyTo copyAction + copyAction as T + } + + is ParallelAction -> { + val copyAction = Actions.action() + this copyTo copyAction + copyAction as T + } + is SequenceAction -> { + val copyAction = Actions.action() + this copyTo copyAction + copyAction as T + } + is VisibleAction -> { + val copyAction = Actions.action() + this copyTo copyAction + copyAction as T + } + else -> throw UnsupportedOperationException(""" + |Tried to copy an (unsupported) action type + |'${this.javaClass.kotlin.simpleName}' + |""".trimMargin() + ) + } + +} +``` + +Another alternative would be to use the visitor pattern or sealed classes. But in an example like the one above, +i.e., you do not have access to the types, you cannot add the visitor pattern or sealed classes without reproducing the +whole type hierarchy (which is error-prone). In order to add the visitor pattern, we would need dynamic dispatch on extension methods. + +Further alternatives are to use the dispatching of the *implicit extension receiver*, but this way the type hierarchy has to be rebuild, too: + +```kotlin +interface CopyableActionExtension { + fun create(): T + fun T.copyTo(action: T) +} + +object CopyableTemporalAction : CopyableActionExtension { + // ... + + // uses implicit receiver + fun TemporalAction.copyTo(action: TemporalAction): TemporalAction { + // ... + } +} + +// rebuild type hierarchy with one CopyableXYZAction per XYZAction + +fun CopyableActionExtension.copyOf(action: T): T { + val newAction = create() + action.copyTo(newAction) + return newAction +} + +... CopyableTemporalAction.copyOf(temporalAction) ... +``` From cc515917511ba2b03fe12fd5d9405c190ac4759f Mon Sep 17 00:00:00 2001 From: juangamnik Date: Fri, 16 Sep 2016 09:53:58 +0200 Subject: [PATCH 3/3] Update type-checking-of-reified-types.md Added section Realization. Added discussion on relation to "overriding extension methods" --- proposals/type-checking-of-reified-types.md | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/proposals/type-checking-of-reified-types.md b/proposals/type-checking-of-reified-types.md index 344706f3d..b62572711 100644 --- a/proposals/type-checking-of-reified-types.md +++ b/proposals/type-checking-of-reified-types.md @@ -95,6 +95,19 @@ The Kotlin documentation reads: But in this case it does not behave as if it were a normal class although `return copyAction as T` is allowed. +This proposal is not a solution for [proposal 46](https://github.com/Kotlin/KEEP/pull/46), since calling the copy-method above (even with the new behavior) would lead to unwanted results: + +```kotlin +// here it is called: +fun someMethod() { + val sequenceAction: Action = SequenceAction() + // `T` will be of type `Action` although I would like to copy a `SequenceAction` + sequenceAction.copy() +} +``` + +So, independent from this proposal ["overriding extension methods"](https://github.com/Kotlin/KEEP/pull/46) has its own right to exist. + No macros are necessary for implementing this proposal. The compiler should be able to inline the function body and recheck without a macro since it is a multi-pass compiler. Naively, the inlining could be done in a preprocessing step (not a macro) on the code-level and then compile it. @@ -131,6 +144,10 @@ fun test() { Inline functions are inlined at compile-time. Operations involving reified type parameters are annotated with special bytecode instructions. Bytecode is post-processed at call-site, where corresponding type arguments are known. Hence, all information is available at compile-time to produce this error although one might argue that this results in leaking abstraction. +## Realization + +Since the naive solution mentioned above (just copy the code to inline before compilation) needs access to the source code of the inline function it is not feasible. The current solution uses special bytecode instructions in the body of inline functions, which are post-processed after inlining. This approach can be enhanced to replace any call to methods on instances of the reified type by one typechecked at call-site. + ## Open Questions 1. Is the proposed conflict resolution (throwing an error that has been caused inside the implementation of the inline function) leak abstraction or is it a straightforward solution?