From a732e8f5fe81370eed14d4a153bc1028c2cef882 Mon Sep 17 00:00:00 2001 From: Mark Punzalan Date: Wed, 4 Mar 2020 23:00:07 -0800 Subject: [PATCH] [JVM IR] Ensure there is one accessor for each `super` access from a subclass when there are multiple subclasses in a file. --- .../ir/FirBlackBoxCodegenTestGenerated.java | 5 + .../jvm/lower/SyntheticAccessorLowering.kt | 102 ++++++++++-------- .../superCallFromMultipleSubclasses.kt | 20 ++++ .../superCallFromMultipleSubclasses.kt | 27 +++++ .../codegen/BlackBoxCodegenTestGenerated.java | 5 + .../BlackBoxInlineCodegenTestGenerated.java | 5 + ...otlinAgainstInlineKotlinTestGenerated.java | 5 + .../LightAnalysisModeTestGenerated.java | 5 + .../ir/IrBlackBoxCodegenTestGenerated.java | 5 + .../IrBlackBoxInlineCodegenTestGenerated.java | 5 + ...otlinAgainstInlineKotlinTestGenerated.java | 5 + .../IrJsCodegenBoxTestGenerated.java | 5 + .../semantics/JsCodegenBoxTestGenerated.java | 5 + 13 files changed, 154 insertions(+), 45 deletions(-) create mode 100644 compiler/testData/codegen/box/syntheticAccessors/superCallFromMultipleSubclasses.kt create mode 100644 compiler/testData/codegen/boxInline/syntheticAccessors/superCallFromMultipleSubclasses.kt diff --git a/compiler/fir/fir2ir/tests/org/jetbrains/kotlin/codegen/ir/FirBlackBoxCodegenTestGenerated.java b/compiler/fir/fir2ir/tests/org/jetbrains/kotlin/codegen/ir/FirBlackBoxCodegenTestGenerated.java index 0f5dbfc644c..87fec79f94f 100644 --- a/compiler/fir/fir2ir/tests/org/jetbrains/kotlin/codegen/ir/FirBlackBoxCodegenTestGenerated.java +++ b/compiler/fir/fir2ir/tests/org/jetbrains/kotlin/codegen/ir/FirBlackBoxCodegenTestGenerated.java @@ -26617,6 +26617,11 @@ public class FirBlackBoxCodegenTestGenerated extends AbstractFirBlackBoxCodegenT runTest("compiler/testData/codegen/box/syntheticAccessors/protectedFromLambda.kt"); } + @TestMetadata("superCallFromMultipleSubclasses.kt") + public void testSuperCallFromMultipleSubclasses() throws Exception { + runTest("compiler/testData/codegen/box/syntheticAccessors/superCallFromMultipleSubclasses.kt"); + } + @TestMetadata("syntheticAccessorNames.kt") public void testSyntheticAccessorNames() throws Exception { runTest("compiler/testData/codegen/box/syntheticAccessors/syntheticAccessorNames.kt"); diff --git a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/lower/SyntheticAccessorLowering.kt b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/lower/SyntheticAccessorLowering.kt index edfbca414f2..400e0e644be 100644 --- a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/lower/SyntheticAccessorLowering.kt +++ b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/lower/SyntheticAccessorLowering.kt @@ -38,7 +38,6 @@ import org.jetbrains.kotlin.ir.visitors.transformChildrenVoid import org.jetbrains.kotlin.load.java.JavaVisibilities import org.jetbrains.kotlin.load.java.JvmAbi import org.jetbrains.kotlin.name.Name -import org.jetbrains.kotlin.synthetic.isVisibleOutside import org.jetbrains.kotlin.utils.addToStdlib.safeAs internal class SyntheticAccessorLowering(val context: JvmBackendContext) : IrElementTransformerVoidWithContext(), FileLoweringPass { @@ -71,7 +70,7 @@ internal class SyntheticAccessorLowering(val context: JvmBackendContext) : IrEle } } - private val functionMap = mutableMapOf() + private val functionMap = mutableMapOf, IrFunctionSymbol>() private val getterMap = mutableMapOf() private val setterMap = mutableMapOf() @@ -88,14 +87,64 @@ internal class SyntheticAccessorLowering(val context: JvmBackendContext) : IrEle callee is IrConstructor && callee.isOrShouldBeHidden -> handleHiddenConstructor(callee).symbol - !expression.symbol.isAccessible(withSuper, thisSymbol) -> - functionMap.getOrPut(expression.symbol) { - when (val symbol = expression.symbol) { + !expression.symbol.isAccessible(withSuper, thisSymbol) -> { + // Find the right container to insert the accessor. Simply put, when we call a function on a class A, + // we also need to put its accessor into A. However, due to the way that calls are implemented in the + // IR we generally need to look at the type of the dispatchReceiver *argument* in order to find the + // correct class. Consider the following code: + // + // fun run(f : () -> Int): Int = f() + // + // open class A { + // private fun f() = 0 + // fun g() = run { this.f() } + // } + // + // class B : A { + // override fun g() = 1 + // fun h() = run { super.g() } + // } + // + // We have calls to the private methods A.f from a generated Lambda subclass for the argument to `run` + // in class A and a super call to A.g from a generated Lambda subclass in class B. + // + // In the first case, we need to produce an accessor in class A to access the private member of A. + // Both the parent of the function f and the type of the dispatch receiver point to the correct class. + // In the second case we need to call A.g from within class B, since this is the only way to invoke + // a method of a superclass on the JVM. However, the IR for the call to super.g points directly to the + // function g in class A. Confusingly, the `superQualifier` on this call also points to class A. + // The only way to compute the actual enclosing class for the call is by looking at the type of the + // dispatch receiver argument, which points to B. + // + // Beyond this, there can be accessors that are needed because other lowerings produce code calling + // private methods (e.g., local functions for lambdas are private and called from generated + // SAM wrapper classes). In this case we rely on the parent field of the called function. + // + // Finally, we need to produce accessors for calls to protected static methods coming from Java, + // which we put in the closest enclosing class which has access to the method in question. + val symbol = expression.symbol + val dispatchReceiverType = expression.dispatchReceiver?.type + val parent = symbol.owner.accessorParent(dispatchReceiverType?.classOrNull?.owner ?: symbol.owner.parent) + + // The key in the cache/map needs to be BOTH the symbol of the function being accessed AND the parent + // of the accessor. Going from the above example, if we have another class C similar to B: + // + // class C : A { + // override fun g() = 2 + // fun i() = run { super.g() } + // } + // + // For the call to super.g in function i, the accessor to A.g must be produced in C. Therefore, we + // cannot use the function symbol (A.g in the example) by itself as the key since there should be + // one accessor per dispatch receiver (i.e., parent of the accessor). + functionMap.getOrPut(symbol to parent) { + when (symbol) { is IrConstructorSymbol -> symbol.owner.makeConstructorAccessor().also(pendingAccessorsToAdd::add).symbol - is IrSimpleFunctionSymbol -> symbol.owner.makeSimpleFunctionAccessor(expression as IrCall).symbol + is IrSimpleFunctionSymbol -> symbol.owner.makeSimpleFunctionAccessor(expression as IrCall, parent).symbol else -> error("Unknown subclass of IrFunctionSymbol") } } + } else -> return super.visitFunctionAccess(expression) } @@ -216,46 +265,9 @@ internal class SyntheticAccessorLowering(val context: JvmBackendContext) : IrEle copyAllParamsToArgs(it, accessor) } - private fun IrSimpleFunction.makeSimpleFunctionAccessor(expression: IrCall): IrSimpleFunction { + private fun IrSimpleFunction.makeSimpleFunctionAccessor(expression: IrCall, parent: IrDeclarationParent): IrSimpleFunction { val source = this - // Find the right container to insert the accessor. Simply put, when we call a function on a class A, - // we also need to put its accessor into A. However, due to the way that calls are implemented in the - // IR we generally need to look at the type of the dispatchReceiver *argument* in order to find the - // correct class. Consider the following code: - // - // fun run(f : () -> Int): Int = f() - // - // open class A { - // private fun f() = 0 - // fun g() = run { this.f() } - // } - // - // class B : A { - // override fun g() = 1 - // fun h() = run { super.g() } - // } - // - // We have calls to the private methods A.f from a generated Lambda subclass for the argument to `run` - // in class A and a super call to A.g from a generated Lambda subclass in class B. - // - // In the first case, we need to produce an accessor in class A to access the private member of A. - // Both the parent of the function f and the type of the dispatch receiver point to the correct class. - // In the second case we need to call A.g from within class B, since this is the only way to invoke - // a method of a superclass on the JVM. However, the IR for the call to super.g points directly to the - // function g in class A. Confusingly, the `superQualifier` on this call also points to class A. - // The only way to compute the actual enclosing class for the call is by looking at the type of the - // dispatch receiver argument, which points to B. - // - // Beyond this, there can be accessors that are needed because other lowerings produce code calling - // private methods (e.g., local functions for lambdas are private and called from generated - // SAM wrapper classes). In this case we rely on the parent field of the called function. - // - // Finally, we need to produce accessors for calls to protected static methods coming from Java, - // which we put in the closest enclosing class which has access to the method in question. - val dispatchReceiverType = expression.dispatchReceiver?.type - val parent = source.accessorParent(dispatchReceiverType?.classOrNull?.owner ?: source.parent) - return buildFun { origin = JvmLoweredDeclarationOrigin.SYNTHETIC_ACCESSOR name = source.accessorName(expression.superQualifierSymbol) @@ -267,7 +279,7 @@ internal class SyntheticAccessorLowering(val context: JvmBackendContext) : IrEle pendingAccessorsToAdd.add(accessor) accessor.copyTypeParametersFrom(source, JvmLoweredDeclarationOrigin.SYNTHETIC_ACCESSOR) - accessor.copyValueParametersToStatic(source, JvmLoweredDeclarationOrigin.SYNTHETIC_ACCESSOR, dispatchReceiverType) + accessor.copyValueParametersToStatic(source, JvmLoweredDeclarationOrigin.SYNTHETIC_ACCESSOR, expression.dispatchReceiver?.type) accessor.returnType = source.returnType.remapTypeParameters(source, accessor) accessor.body = IrExpressionBodyImpl( diff --git a/compiler/testData/codegen/box/syntheticAccessors/superCallFromMultipleSubclasses.kt b/compiler/testData/codegen/box/syntheticAccessors/superCallFromMultipleSubclasses.kt new file mode 100644 index 00000000000..51debfd2805 --- /dev/null +++ b/compiler/testData/codegen/box/syntheticAccessors/superCallFromMultipleSubclasses.kt @@ -0,0 +1,20 @@ +// IGNORE_BACKEND_FIR: JVM_IR +open class A { + open fun test(s: String) = s +} + +object B : A() { + override fun test(s: String) = "fail" + + val doTest = { super.test("O") } +} + +object C : A() { + override fun test(s: String) = "fail" + + val doTest = { super.test("K") } +} + +fun box(): String { + return B.doTest() + C.doTest() +} diff --git a/compiler/testData/codegen/boxInline/syntheticAccessors/superCallFromMultipleSubclasses.kt b/compiler/testData/codegen/boxInline/syntheticAccessors/superCallFromMultipleSubclasses.kt new file mode 100644 index 00000000000..af640fd4a92 --- /dev/null +++ b/compiler/testData/codegen/boxInline/syntheticAccessors/superCallFromMultipleSubclasses.kt @@ -0,0 +1,27 @@ +// FILE: 1.kt + +package test + +open class A { + open fun test(s: String) = s +} + +object B : A() { + override fun test(s: String) = "fail" + + inline fun doTest(s: String) = super.test(s) +} + +object C : A() { + override fun test(s: String) = "fail" + + inline fun doTest(s: String) = super.test(s) +} + +// FILE: 2.kt + +import test.* + +fun box(): String { + return B.doTest("O") + C.doTest("K") +} diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java index 51758095d9e..d8c6c3752aa 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java @@ -28133,6 +28133,11 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest { runTest("compiler/testData/codegen/box/syntheticAccessors/protectedFromLambda.kt"); } + @TestMetadata("superCallFromMultipleSubclasses.kt") + public void testSuperCallFromMultipleSubclasses() throws Exception { + runTest("compiler/testData/codegen/box/syntheticAccessors/superCallFromMultipleSubclasses.kt"); + } + @TestMetadata("syntheticAccessorNames.kt") public void testSyntheticAccessorNames() throws Exception { runTest("compiler/testData/codegen/box/syntheticAccessors/syntheticAccessorNames.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxInlineCodegenTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxInlineCodegenTestGenerated.java index d84ffccacb4..3d95d97ad8b 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxInlineCodegenTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxInlineCodegenTestGenerated.java @@ -4333,6 +4333,11 @@ public class BlackBoxInlineCodegenTestGenerated extends AbstractBlackBoxInlineCo runTest("compiler/testData/codegen/boxInline/syntheticAccessors/superCall.kt"); } + @TestMetadata("superCallFromMultipleSubclasses.kt") + public void testSuperCallFromMultipleSubclasses() throws Exception { + runTest("compiler/testData/codegen/boxInline/syntheticAccessors/superCallFromMultipleSubclasses.kt"); + } + @TestMetadata("superProperty.kt") public void testSuperProperty() throws Exception { runTest("compiler/testData/codegen/boxInline/syntheticAccessors/superProperty.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/CompileKotlinAgainstInlineKotlinTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/CompileKotlinAgainstInlineKotlinTestGenerated.java index d600a5cd28e..917260b524b 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/CompileKotlinAgainstInlineKotlinTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/CompileKotlinAgainstInlineKotlinTestGenerated.java @@ -4333,6 +4333,11 @@ public class CompileKotlinAgainstInlineKotlinTestGenerated extends AbstractCompi runTest("compiler/testData/codegen/boxInline/syntheticAccessors/superCall.kt"); } + @TestMetadata("superCallFromMultipleSubclasses.kt") + public void testSuperCallFromMultipleSubclasses() throws Exception { + runTest("compiler/testData/codegen/boxInline/syntheticAccessors/superCallFromMultipleSubclasses.kt"); + } + @TestMetadata("superProperty.kt") public void testSuperProperty() throws Exception { runTest("compiler/testData/codegen/boxInline/syntheticAccessors/superProperty.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java index 6124a44d5c5..6313b271d00 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java @@ -26950,6 +26950,11 @@ public class LightAnalysisModeTestGenerated extends AbstractLightAnalysisModeTes runTest("compiler/testData/codegen/box/syntheticAccessors/protectedFromLambda.kt"); } + @TestMetadata("superCallFromMultipleSubclasses.kt") + public void testSuperCallFromMultipleSubclasses() throws Exception { + runTest("compiler/testData/codegen/box/syntheticAccessors/superCallFromMultipleSubclasses.kt"); + } + @TestMetadata("syntheticAccessorNames.kt") public void testSyntheticAccessorNames() throws Exception { runTest("compiler/testData/codegen/box/syntheticAccessors/syntheticAccessorNames.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java index 5bd252c3176..8528e335302 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java @@ -26617,6 +26617,11 @@ public class IrBlackBoxCodegenTestGenerated extends AbstractIrBlackBoxCodegenTes runTest("compiler/testData/codegen/box/syntheticAccessors/protectedFromLambda.kt"); } + @TestMetadata("superCallFromMultipleSubclasses.kt") + public void testSuperCallFromMultipleSubclasses() throws Exception { + runTest("compiler/testData/codegen/box/syntheticAccessors/superCallFromMultipleSubclasses.kt"); + } + @TestMetadata("syntheticAccessorNames.kt") public void testSyntheticAccessorNames() throws Exception { runTest("compiler/testData/codegen/box/syntheticAccessors/syntheticAccessorNames.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxInlineCodegenTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxInlineCodegenTestGenerated.java index d105f139e4d..1cdb785dd8d 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxInlineCodegenTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxInlineCodegenTestGenerated.java @@ -4068,6 +4068,11 @@ public class IrBlackBoxInlineCodegenTestGenerated extends AbstractIrBlackBoxInli runTest("compiler/testData/codegen/boxInline/syntheticAccessors/superCall.kt"); } + @TestMetadata("superCallFromMultipleSubclasses.kt") + public void testSuperCallFromMultipleSubclasses() throws Exception { + runTest("compiler/testData/codegen/boxInline/syntheticAccessors/superCallFromMultipleSubclasses.kt"); + } + @TestMetadata("superProperty.kt") public void testSuperProperty() throws Exception { runTest("compiler/testData/codegen/boxInline/syntheticAccessors/superProperty.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrCompileKotlinAgainstInlineKotlinTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrCompileKotlinAgainstInlineKotlinTestGenerated.java index 9a05bdf064d..c7089756397 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrCompileKotlinAgainstInlineKotlinTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrCompileKotlinAgainstInlineKotlinTestGenerated.java @@ -4068,6 +4068,11 @@ public class IrCompileKotlinAgainstInlineKotlinTestGenerated extends AbstractIrC runTest("compiler/testData/codegen/boxInline/syntheticAccessors/superCall.kt"); } + @TestMetadata("superCallFromMultipleSubclasses.kt") + public void testSuperCallFromMultipleSubclasses() throws Exception { + runTest("compiler/testData/codegen/boxInline/syntheticAccessors/superCallFromMultipleSubclasses.kt"); + } + @TestMetadata("superProperty.kt") public void testSuperProperty() throws Exception { runTest("compiler/testData/codegen/boxInline/syntheticAccessors/superProperty.kt"); diff --git a/js/js.tests/test/org/jetbrains/kotlin/js/test/ir/semantics/IrJsCodegenBoxTestGenerated.java b/js/js.tests/test/org/jetbrains/kotlin/js/test/ir/semantics/IrJsCodegenBoxTestGenerated.java index 379f4cac8a4..5d663b8c9da 100644 --- a/js/js.tests/test/org/jetbrains/kotlin/js/test/ir/semantics/IrJsCodegenBoxTestGenerated.java +++ b/js/js.tests/test/org/jetbrains/kotlin/js/test/ir/semantics/IrJsCodegenBoxTestGenerated.java @@ -21622,6 +21622,11 @@ public class IrJsCodegenBoxTestGenerated extends AbstractIrJsCodegenBoxTest { public void testProtectedFromLambda() throws Exception { runTest("compiler/testData/codegen/box/syntheticAccessors/protectedFromLambda.kt"); } + + @TestMetadata("superCallFromMultipleSubclasses.kt") + public void testSuperCallFromMultipleSubclasses() throws Exception { + runTest("compiler/testData/codegen/box/syntheticAccessors/superCallFromMultipleSubclasses.kt"); + } } @TestMetadata("compiler/testData/codegen/box/throws") diff --git a/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/JsCodegenBoxTestGenerated.java b/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/JsCodegenBoxTestGenerated.java index b2af7e6ed43..9a2c698963b 100644 --- a/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/JsCodegenBoxTestGenerated.java +++ b/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/JsCodegenBoxTestGenerated.java @@ -21682,6 +21682,11 @@ public class JsCodegenBoxTestGenerated extends AbstractJsCodegenBoxTest { public void testProtectedFromLambda() throws Exception { runTest("compiler/testData/codegen/box/syntheticAccessors/protectedFromLambda.kt"); } + + @TestMetadata("superCallFromMultipleSubclasses.kt") + public void testSuperCallFromMultipleSubclasses() throws Exception { + runTest("compiler/testData/codegen/box/syntheticAccessors/superCallFromMultipleSubclasses.kt"); + } } @TestMetadata("compiler/testData/codegen/box/throws")