From 2fa4c28e0f157c734085db453967bc7a4f089eba Mon Sep 17 00:00:00 2001 From: Mikhael Bogdanov Date: Wed, 6 Sep 2017 18:03:12 +0200 Subject: [PATCH] Properly reorder arguments in anonymous object super class constructor #KT-18356 Fixed --- .../kotlin/codegen/ExpressionCodegen.java | 2 +- .../ObjectSuperCallArgumentGenerator.kt | 42 +++++++++++++------ .../codegen/state/KotlinTypeMapper.java | 11 ++--- .../argumentOrderInObjectSuperCall.kt | 10 +++++ .../argumentOrder/argumentOrderInSuperCall.kt | 11 +++++ .../box/super/superConstructor/kt18356.kt | 8 ++++ .../box/super/superConstructor/kt18356_2.kt | 8 ++++ .../superConstructor/objectExtendsInner.kt | 14 +++++++ .../objectExtendsLocalInner.kt | 17 ++++++++ .../ir/IrBlackBoxCodegenTestGenerated.java | 36 ++++++++++++++++ .../codegen/BlackBoxCodegenTestGenerated.java | 36 ++++++++++++++++ .../LightAnalysisModeTestGenerated.java | 36 ++++++++++++++++ .../semantics/JsCodegenBoxTestGenerated.java | 36 ++++++++++++++++ 13 files changed, 249 insertions(+), 18 deletions(-) create mode 100644 compiler/testData/codegen/box/argumentOrder/argumentOrderInObjectSuperCall.kt create mode 100644 compiler/testData/codegen/box/argumentOrder/argumentOrderInSuperCall.kt create mode 100644 compiler/testData/codegen/box/super/superConstructor/kt18356.kt create mode 100644 compiler/testData/codegen/box/super/superConstructor/kt18356_2.kt create mode 100644 compiler/testData/codegen/box/super/superConstructor/objectExtendsInner.kt create mode 100644 compiler/testData/codegen/box/super/superConstructor/objectExtendsLocalInner.kt diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/ExpressionCodegen.java b/compiler/backend/src/org/jetbrains/kotlin/codegen/ExpressionCodegen.java index 6e6a0e0ef66..1cd085dd4b9 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/ExpressionCodegen.java +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/ExpressionCodegen.java @@ -1072,7 +1072,7 @@ public class ExpressionCodegen extends KtVisitor impleme List valueArguments = new ArrayList<>(params); List valueParameters = new ArrayList<>(params); List mappedTypes = new ArrayList<>(params); - for (ValueParameterDescriptor parameter : superValueParameters) { + for (ValueParameterDescriptor parameter : superCall.getValueArguments().keySet()) { ResolvedValueArgument argument = superCall.getValueArguments().get(parameter); if (!(argument instanceof DefaultValueArgument)) { valueArguments.add(argument); diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/ObjectSuperCallArgumentGenerator.kt b/compiler/backend/src/org/jetbrains/kotlin/codegen/ObjectSuperCallArgumentGenerator.kt index 3c72178851c..fd93e56a971 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/ObjectSuperCallArgumentGenerator.kt +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/ObjectSuperCallArgumentGenerator.kt @@ -16,24 +16,40 @@ package org.jetbrains.kotlin.codegen -import org.jetbrains.kotlin.descriptors.ConstructorDescriptor -import org.jetbrains.kotlin.resolve.calls.model.DefaultValueArgument -import org.jetbrains.kotlin.resolve.calls.model.ExpressionValueArgument -import org.jetbrains.kotlin.resolve.calls.model.ResolvedCall -import org.jetbrains.kotlin.resolve.calls.model.VarargValueArgument -import org.jetbrains.kotlin.resolve.jvm.jvmSignature.JvmMethodParameterSignature -import org.jetbrains.org.objectweb.asm.Type -import org.jetbrains.org.objectweb.asm.commons.InstructionAdapter - import org.jetbrains.kotlin.codegen.AsmUtil.pushDefaultValueOnStack +import org.jetbrains.kotlin.descriptors.CallableDescriptor +import org.jetbrains.kotlin.descriptors.ConstructorDescriptor +import org.jetbrains.kotlin.resolve.calls.model.* +import org.jetbrains.kotlin.resolve.jvm.jvmSignature.JvmMethodParameterSignature +import org.jetbrains.org.objectweb.asm.commons.InstructionAdapter internal class ObjectSuperCallArgumentGenerator( private val parameters: List, private val iv: InstructionAdapter, - private var offset: Int, + offset: Int, superConstructorCall: ResolvedCall ) : ArgumentGenerator() { + private val offsets = IntArray(parameters.size) { -1 } + + init { + var currentOffset = offset + superConstructorCall.valueArguments.forEach { + (descriptor, argument) -> + if (argument !is DefaultValueArgument) { + val index = descriptor.index + offsets[index] = currentOffset + currentOffset += parameters[index].asmType.size + } + } + } + + override fun generate( + valueArgumentsByIndex: List, + actualArgs: List, + calleeDescriptor: CallableDescriptor? + ): DefaultCallArgs = super.generate(valueArgumentsByIndex, valueArgumentsByIndex, calleeDescriptor) + public override fun generateExpression(i: Int, argument: ExpressionValueArgument) { generateSuperCallArgument(i) } @@ -49,8 +65,10 @@ internal class ObjectSuperCallArgumentGenerator( private fun generateSuperCallArgument(i: Int) { val type = parameters[i].asmType - iv.load(offset, type) - offset += type.size + if (offsets[i] == -1) { + throw AssertionError("Unknown parameter value at index $i with type $type") + } + iv.load(offsets[i], type) } override fun reorderArgumentsIfNeeded(args: List) { diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/state/KotlinTypeMapper.java b/compiler/backend/src/org/jetbrains/kotlin/codegen/state/KotlinTypeMapper.java index 3f5935897b7..37c29cb85bb 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/state/KotlinTypeMapper.java +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/state/KotlinTypeMapper.java @@ -77,6 +77,7 @@ import org.jetbrains.org.objectweb.asm.commons.Method; import java.util.Collection; import java.util.Collections; import java.util.List; +import java.util.Map; import static org.jetbrains.kotlin.codegen.AsmUtil.isStaticMethod; import static org.jetbrains.kotlin.codegen.JvmCodegenUtil.*; @@ -1420,8 +1421,7 @@ public class KotlinTypeMapper { boolean hasOuter ) { ConstructorDescriptor superDescriptor = SamCodegenUtil.resolveSamAdapter(superCall.getResultingDescriptor()); - List valueArguments = superCall.getValueArgumentsByIndex(); - assert valueArguments != null : "Failed to arrange value arguments by index: " + superDescriptor; + Map valueArguments = superCall.getValueArguments(); List parameters = mapSignatureSkipGeneric(superDescriptor.getOriginal()).getValueParameters(); @@ -1444,10 +1444,11 @@ public class KotlinTypeMapper { if (isAnonymousObject(descriptor.getContainingDeclaration())) { // For anonymous objects, also add all real non-default value arguments passed to the super constructor - for (int i = 0; i < args; i++) { - ResolvedValueArgument valueArgument = valueArguments.get(i); - JvmMethodParameterSignature parameter = parameters.get(params - args + i); + for (Map.Entry argumentAndValue : valueArguments.entrySet()) { + + ResolvedValueArgument valueArgument = argumentAndValue.getValue(); if (!(valueArgument instanceof DefaultValueArgument)) { + JvmMethodParameterSignature parameter = parameters.get(params - args + argumentAndValue.getKey().getIndex()); writeParameter(sw, JvmMethodParameterKind.SUPER_CALL_PARAM, parameter.getAsmType()); } } diff --git a/compiler/testData/codegen/box/argumentOrder/argumentOrderInObjectSuperCall.kt b/compiler/testData/codegen/box/argumentOrder/argumentOrderInObjectSuperCall.kt new file mode 100644 index 00000000000..b194d4365b3 --- /dev/null +++ b/compiler/testData/codegen/box/argumentOrder/argumentOrderInObjectSuperCall.kt @@ -0,0 +1,10 @@ +var result = "fail" + +open class Base(val o: String, val k: String) + +fun box(): String { + val obj1 = object : Base(k = { result = "O"; "K"}() , o = {result += "K"; "O"}()) {} + + if (result != "OK") return "fail $result" + return obj1.o + obj1.k +} \ No newline at end of file diff --git a/compiler/testData/codegen/box/argumentOrder/argumentOrderInSuperCall.kt b/compiler/testData/codegen/box/argumentOrder/argumentOrderInSuperCall.kt new file mode 100644 index 00000000000..042a5dcbae6 --- /dev/null +++ b/compiler/testData/codegen/box/argumentOrder/argumentOrderInSuperCall.kt @@ -0,0 +1,11 @@ +var result = "fail" + +open class Base(val o: String, val k: String) +class Derived : Base(k = { result = "O"; "K"}() , o = {result += "K"; "O"}()) {} + +fun box(): String { + val derived = Derived() + + if (result != "OK") return "fail $result" + return derived.o + derived.k +} \ No newline at end of file diff --git a/compiler/testData/codegen/box/super/superConstructor/kt18356.kt b/compiler/testData/codegen/box/super/superConstructor/kt18356.kt new file mode 100644 index 00000000000..150d2f5dd85 --- /dev/null +++ b/compiler/testData/codegen/box/super/superConstructor/kt18356.kt @@ -0,0 +1,8 @@ +open class Base(val addr: Long, val name: String) + +fun box(): String { + val obj1 = object : Base(name = "OK", addr = 0x1234L) {} + if (obj1.addr != 0x1234L) return "fail ${obj1.addr}" + return obj1.name +} + diff --git a/compiler/testData/codegen/box/super/superConstructor/kt18356_2.kt b/compiler/testData/codegen/box/super/superConstructor/kt18356_2.kt new file mode 100644 index 00000000000..9773342f5fa --- /dev/null +++ b/compiler/testData/codegen/box/super/superConstructor/kt18356_2.kt @@ -0,0 +1,8 @@ +abstract class Base(val s: String, vararg ints: Int) + +fun foo(s: String, ints: IntArray) = object : Base(ints = *ints, s = s) {} + +fun box(): String { + return foo("OK", intArrayOf(1, 2)).s +} + diff --git a/compiler/testData/codegen/box/super/superConstructor/objectExtendsInner.kt b/compiler/testData/codegen/box/super/superConstructor/objectExtendsInner.kt new file mode 100644 index 00000000000..83d075cdeae --- /dev/null +++ b/compiler/testData/codegen/box/super/superConstructor/objectExtendsInner.kt @@ -0,0 +1,14 @@ +open class Foo(val value: String) { + + open inner class Inner(val d: Double = -1.0, val s: String, vararg val y: Int) { + open fun result() = "Fail" + } + + val obj = object : Inner(s = "O") { + override fun result() = s + value + } +} + +fun box(): String { + return Foo("K").obj.result() +} diff --git a/compiler/testData/codegen/box/super/superConstructor/objectExtendsLocalInner.kt b/compiler/testData/codegen/box/super/superConstructor/objectExtendsLocalInner.kt new file mode 100644 index 00000000000..ab3c695f278 --- /dev/null +++ b/compiler/testData/codegen/box/super/superConstructor/objectExtendsLocalInner.kt @@ -0,0 +1,17 @@ +fun box(): String { + val capture = "O" + + class Local { + val captured = capture + + open inner class Inner(val d: Double = -1.0, val s: String, vararg val y: Int) { + open fun result() = "Fail" + } + + val obj = object : Inner(s = "K") { + override fun result() = capture + s + } + } + + return Local().obj.result() +} diff --git a/compiler/tests-ir-jvm/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java b/compiler/tests-ir-jvm/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java index c955669c302..02679fe72e5 100644 --- a/compiler/tests-ir-jvm/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java +++ b/compiler/tests-ir-jvm/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java @@ -206,6 +206,18 @@ public class IrBlackBoxCodegenTestGenerated extends AbstractIrBlackBoxCodegenTes KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("compiler/testData/codegen/box/argumentOrder"), Pattern.compile("^(.+)\\.kt$"), TargetBackend.JVM, true); } + @TestMetadata("argumentOrderInObjectSuperCall.kt") + public void testArgumentOrderInObjectSuperCall() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/argumentOrder/argumentOrderInObjectSuperCall.kt"); + doTest(fileName); + } + + @TestMetadata("argumentOrderInSuperCall.kt") + public void testArgumentOrderInSuperCall() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/argumentOrder/argumentOrderInSuperCall.kt"); + doTest(fileName); + } + @TestMetadata("arguments.kt") public void testArguments() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/argumentOrder/arguments.kt"); @@ -18672,6 +18684,30 @@ public class IrBlackBoxCodegenTestGenerated extends AbstractIrBlackBoxCodegenTes String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/super/superConstructor/kt17464_linkedMapOf.kt"); doTest(fileName); } + + @TestMetadata("kt18356.kt") + public void testKt18356() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/super/superConstructor/kt18356.kt"); + doTest(fileName); + } + + @TestMetadata("kt18356_2.kt") + public void testKt18356_2() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/super/superConstructor/kt18356_2.kt"); + doTest(fileName); + } + + @TestMetadata("objectExtendsInner.kt") + public void testObjectExtendsInner() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/super/superConstructor/objectExtendsInner.kt"); + doTest(fileName); + } + + @TestMetadata("objectExtendsLocalInner.kt") + public void testObjectExtendsLocalInner() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/super/superConstructor/objectExtendsLocalInner.kt"); + doTest(fileName); + } } } diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java index c011aef776b..f95670eb0a8 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java @@ -206,6 +206,18 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest { KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("compiler/testData/codegen/box/argumentOrder"), Pattern.compile("^(.+)\\.kt$"), TargetBackend.JVM, true); } + @TestMetadata("argumentOrderInObjectSuperCall.kt") + public void testArgumentOrderInObjectSuperCall() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/argumentOrder/argumentOrderInObjectSuperCall.kt"); + doTest(fileName); + } + + @TestMetadata("argumentOrderInSuperCall.kt") + public void testArgumentOrderInSuperCall() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/argumentOrder/argumentOrderInSuperCall.kt"); + doTest(fileName); + } + @TestMetadata("arguments.kt") public void testArguments() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/argumentOrder/arguments.kt"); @@ -18672,6 +18684,30 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/super/superConstructor/kt17464_linkedMapOf.kt"); doTest(fileName); } + + @TestMetadata("kt18356.kt") + public void testKt18356() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/super/superConstructor/kt18356.kt"); + doTest(fileName); + } + + @TestMetadata("kt18356_2.kt") + public void testKt18356_2() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/super/superConstructor/kt18356_2.kt"); + doTest(fileName); + } + + @TestMetadata("objectExtendsInner.kt") + public void testObjectExtendsInner() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/super/superConstructor/objectExtendsInner.kt"); + doTest(fileName); + } + + @TestMetadata("objectExtendsLocalInner.kt") + public void testObjectExtendsLocalInner() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/super/superConstructor/objectExtendsLocalInner.kt"); + doTest(fileName); + } } } diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java index c4618f6d2b0..4cfc3e719fd 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java @@ -206,6 +206,18 @@ public class LightAnalysisModeTestGenerated extends AbstractLightAnalysisModeTes KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("compiler/testData/codegen/box/argumentOrder"), Pattern.compile("^(.+)\\.kt$"), TargetBackend.JVM, true); } + @TestMetadata("argumentOrderInObjectSuperCall.kt") + public void testArgumentOrderInObjectSuperCall() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/argumentOrder/argumentOrderInObjectSuperCall.kt"); + doTest(fileName); + } + + @TestMetadata("argumentOrderInSuperCall.kt") + public void testArgumentOrderInSuperCall() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/argumentOrder/argumentOrderInSuperCall.kt"); + doTest(fileName); + } + @TestMetadata("arguments.kt") public void testArguments() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/argumentOrder/arguments.kt"); @@ -18672,6 +18684,30 @@ public class LightAnalysisModeTestGenerated extends AbstractLightAnalysisModeTes String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/super/superConstructor/kt17464_linkedMapOf.kt"); doTest(fileName); } + + @TestMetadata("kt18356.kt") + public void testKt18356() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/super/superConstructor/kt18356.kt"); + doTest(fileName); + } + + @TestMetadata("kt18356_2.kt") + public void testKt18356_2() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/super/superConstructor/kt18356_2.kt"); + doTest(fileName); + } + + @TestMetadata("objectExtendsInner.kt") + public void testObjectExtendsInner() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/super/superConstructor/objectExtendsInner.kt"); + doTest(fileName); + } + + @TestMetadata("objectExtendsLocalInner.kt") + public void testObjectExtendsLocalInner() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/super/superConstructor/objectExtendsLocalInner.kt"); + doTest(fileName); + } } } 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 09235632fde..265d834b6f4 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 @@ -332,6 +332,18 @@ public class JsCodegenBoxTestGenerated extends AbstractJsCodegenBoxTest { KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("compiler/testData/codegen/box/argumentOrder"), Pattern.compile("^(.+)\\.kt$"), TargetBackend.JS, true); } + @TestMetadata("argumentOrderInObjectSuperCall.kt") + public void testArgumentOrderInObjectSuperCall() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/argumentOrder/argumentOrderInObjectSuperCall.kt"); + doTest(fileName); + } + + @TestMetadata("argumentOrderInSuperCall.kt") + public void testArgumentOrderInSuperCall() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/argumentOrder/argumentOrderInSuperCall.kt"); + doTest(fileName); + } + @TestMetadata("arguments.kt") public void testArguments() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/argumentOrder/arguments.kt"); @@ -22650,6 +22662,30 @@ public class JsCodegenBoxTestGenerated extends AbstractJsCodegenBoxTest { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/super/superConstructor/kt17464_linkedMapOf.kt"); doTest(fileName); } + + @TestMetadata("kt18356.kt") + public void testKt18356() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/super/superConstructor/kt18356.kt"); + doTest(fileName); + } + + @TestMetadata("kt18356_2.kt") + public void testKt18356_2() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/super/superConstructor/kt18356_2.kt"); + doTest(fileName); + } + + @TestMetadata("objectExtendsInner.kt") + public void testObjectExtendsInner() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/super/superConstructor/objectExtendsInner.kt"); + doTest(fileName); + } + + @TestMetadata("objectExtendsLocalInner.kt") + public void testObjectExtendsLocalInner() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/super/superConstructor/objectExtendsLocalInner.kt"); + doTest(fileName); + } } }