From 7d62f0b5aa062ca03e49b3f26d638c706928efaa Mon Sep 17 00:00:00 2001 From: Dmitry Petrov Date: Tue, 13 Apr 2021 14:35:18 +0300 Subject: [PATCH] JVM_IR KT-45187 use Arrays.copyOf to copy an array in spread operator Creating a new array (and copying data into it with System.arraycopy) doesn't work in generic case, because the actual array class depends on call site. --- .../FirBlackBoxCodegenTestGenerated.java | 6 ++ .../jetbrains/kotlin/backend/common/ir/Ir.kt | 9 +++ .../kotlin/backend/jvm/JvmSymbols.kt | 48 ++++++++++++---- .../kotlin/backend/jvm/ir/IrArrayBuilder.kt | 17 ++---- .../testData/codegen/box/vararg/kt45187.kt | 13 +++++ .../codegen/box/vararg/spreadCopiesArray.kt | 50 ++++++++++++++++- .../ir/irText/classes/kt45853.fir.txt | 55 +++++++++++++++++++ .../codegen/BlackBoxCodegenTestGenerated.java | 6 ++ .../IrBlackBoxCodegenTestGenerated.java | 6 ++ .../LightAnalysisModeTestGenerated.java | 5 ++ 10 files changed, 192 insertions(+), 23 deletions(-) create mode 100644 compiler/testData/codegen/box/vararg/kt45187.kt create mode 100644 compiler/testData/ir/irText/classes/kt45853.fir.txt diff --git a/compiler/fir/fir2ir/tests-gen/org/jetbrains/kotlin/test/runners/codegen/FirBlackBoxCodegenTestGenerated.java b/compiler/fir/fir2ir/tests-gen/org/jetbrains/kotlin/test/runners/codegen/FirBlackBoxCodegenTestGenerated.java index 624a860639b..c35c93e1dbf 100644 --- a/compiler/fir/fir2ir/tests-gen/org/jetbrains/kotlin/test/runners/codegen/FirBlackBoxCodegenTestGenerated.java +++ b/compiler/fir/fir2ir/tests-gen/org/jetbrains/kotlin/test/runners/codegen/FirBlackBoxCodegenTestGenerated.java @@ -40940,6 +40940,12 @@ public class FirBlackBoxCodegenTestGenerated extends AbstractFirBlackBoxCodegenT runTest("compiler/testData/codegen/box/vararg/kt37779.kt"); } + @Test + @TestMetadata("kt45187.kt") + public void testKt45187() throws Exception { + runTest("compiler/testData/codegen/box/vararg/kt45187.kt"); + } + @Test @TestMetadata("kt581.kt") public void testKt581() throws Exception { diff --git a/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/ir/Ir.kt b/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/ir/Ir.kt index 52be21aee2d..a8e969ad58c 100644 --- a/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/ir/Ir.kt +++ b/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/ir/Ir.kt @@ -204,6 +204,15 @@ open class BuiltinSymbolsBase(protected val irBuiltIns: IrBuiltIns, protected va val doubleArray = primitiveArrayClass(PrimitiveType.DOUBLE) val booleanArray = primitiveArrayClass(PrimitiveType.BOOLEAN) + val byteArrayType get() = byteArray.owner.defaultType + val charArrayType get() = charArray.owner.defaultType + val shortArrayType get() = shortArray.owner.defaultType + val intArrayType get() = intArray.owner.defaultType + val longArrayType get() = longArray.owner.defaultType + val floatArrayType get() = floatArray.owner.defaultType + val doubleArrayType get() = doubleArray.owner.defaultType + val booleanArrayType get() = booleanArray.owner.defaultType + val unsignedArrays = UnsignedType.values().mapNotNull { unsignedType -> unsignedArrayClass(unsignedType)?.let { unsignedType to it } }.toMap() diff --git a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/JvmSymbols.kt b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/JvmSymbols.kt index 3a75c47c837..fca7e32e40b 100644 --- a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/JvmSymbols.kt +++ b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/JvmSymbols.kt @@ -23,12 +23,12 @@ import org.jetbrains.kotlin.ir.builders.declarations.* import org.jetbrains.kotlin.ir.declarations.IrClass import org.jetbrains.kotlin.ir.declarations.IrDeclarationOrigin import org.jetbrains.kotlin.ir.declarations.IrPackageFragment +import org.jetbrains.kotlin.ir.declarations.IrSimpleFunction import org.jetbrains.kotlin.ir.declarations.impl.IrExternalPackageFragmentImpl import org.jetbrains.kotlin.ir.descriptors.IrBuiltIns import org.jetbrains.kotlin.ir.symbols.* import org.jetbrains.kotlin.ir.types.* import org.jetbrains.kotlin.ir.util.* -import org.jetbrains.kotlin.load.java.JvmAnnotationNames import org.jetbrains.kotlin.name.FqName import org.jetbrains.kotlin.name.Name import org.jetbrains.kotlin.resolve.JVM_INLINE_ANNOTATION_FQ_NAME @@ -51,6 +51,7 @@ class JvmSymbols( private val kotlinJvmFunctionsPackage: IrPackageFragment = createPackage(FqName("kotlin.jvm.functions")) private val kotlinReflectPackage: IrPackageFragment = createPackage(FqName("kotlin.reflect")) private val javaLangPackage: IrPackageFragment = createPackage(FqName("java.lang")) + private val javaUtilPackage: IrPackageFragment = createPackage(FqName("java.util")) // Special package for functions representing dynamic symbols referenced by 'INVOKEDYNAMIC' instruction - e.g., // 'get(Ljava/lang/String;)Ljava/util/function/Supplier;' @@ -90,6 +91,7 @@ class JvmSymbols( "kotlin.jvm" -> kotlinJvmPackage "kotlin.reflect" -> kotlinReflectPackage "java.lang" -> javaLangPackage + "java.util" -> javaUtilPackage else -> error("Other packages are not supported yet: $fqName") } createImplicitParameterDeclarationWithWrappedDescriptor() @@ -548,6 +550,7 @@ class JvmSymbols( }.symbol val arrayOfAnyType = irBuiltIns.arrayClass.typeWith(irBuiltIns.anyType) + val arrayOfAnyNType = irBuiltIns.arrayClass.typeWith(irBuiltIns.anyNType) // Intrinsic to represent closure creation using INVOKEDYNAMIC with LambdaMetafactory.{metafactory, altMetafactory} // as a bootstrap method. @@ -742,13 +745,40 @@ class JvmSymbols( } } - private val systemClass: IrClassSymbol = createClass(FqName("java.lang.System")) { klass -> - klass.addFunction("arraycopy", irBuiltIns.unitType, isStatic = true).apply { - addValueParameter("src", irBuiltIns.anyNType) - addValueParameter("srcPos", irBuiltIns.intType) - addValueParameter("dest", irBuiltIns.anyNType) - addValueParameter("destPos", irBuiltIns.intType) - addValueParameter("length", irBuiltIns.intType) + private val arraysCopyOfFunctions = HashMap() + + private fun IrClass.addArraysCopyOfFunction(arrayType: IrSimpleType) { + addFunction("copyOf", arrayType, isStatic = true).apply { + addValueParameter("original", arrayType) + addValueParameter("newLength", irBuiltIns.intType) + arraysCopyOfFunctions[arrayType] = this + } + } + + val arraysClass: IrClassSymbol = + createClass(FqName("java.util.Arrays")) { irClass -> + irClass.addArraysCopyOfFunction(booleanArrayType) + irClass.addArraysCopyOfFunction(byteArrayType) + irClass.addArraysCopyOfFunction(charArrayType) + irClass.addArraysCopyOfFunction(shortArrayType) + irClass.addArraysCopyOfFunction(intArrayType) + irClass.addArraysCopyOfFunction(longArrayType) + irClass.addArraysCopyOfFunction(floatArrayType) + irClass.addArraysCopyOfFunction(doubleArrayType) + + // public static T[] copyOf(T[] original, int newLength) + irClass.addArraysCopyOfFunction(arrayOfAnyNType) + } + + fun getArraysCopyOfFunction(arrayType: IrSimpleType): IrSimpleFunctionSymbol { + val copyOf = arraysCopyOfFunctions[arrayType] + return when { + copyOf != null -> + copyOf.symbol + arrayType.classifierOrFail == array -> + arraysCopyOfFunctions[arrayOfAnyNType]!!.symbol + else -> + throw AssertionError("Array type expected: ${arrayType.render()}") } } @@ -798,8 +828,6 @@ class JvmSymbols( val remainderUnsignedLong: IrSimpleFunctionSymbol = javaLangLong.functionByName("remainderUnsigned") val toUnsignedStringLong: IrSimpleFunctionSymbol = javaLangLong.functionByName("toUnsignedString") - val systemArraycopy: IrSimpleFunctionSymbol = systemClass.functionByName("arraycopy") - val signatureStringIntrinsic: IrSimpleFunctionSymbol = irFactory.buildFun { name = Name.special("") diff --git a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/ir/IrArrayBuilder.kt b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/ir/IrArrayBuilder.kt index eb2fe5fbb0a..19211b548ce 100644 --- a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/ir/IrArrayBuilder.kt +++ b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/ir/IrArrayBuilder.kt @@ -11,10 +11,7 @@ import org.jetbrains.kotlin.ir.expressions.IrConstructorCall import org.jetbrains.kotlin.ir.expressions.IrExpression import org.jetbrains.kotlin.ir.expressions.IrFunctionAccessExpression import org.jetbrains.kotlin.ir.expressions.IrGetValue -import org.jetbrains.kotlin.ir.types.IrType -import org.jetbrains.kotlin.ir.types.classOrNull -import org.jetbrains.kotlin.ir.types.getArrayElementType -import org.jetbrains.kotlin.ir.types.isBoxedArray +import org.jetbrains.kotlin.ir.types.* import org.jetbrains.kotlin.ir.util.constructors import org.jetbrains.kotlin.ir.util.functions import org.jetbrains.kotlin.ir.util.getPropertyGetter @@ -105,16 +102,12 @@ class IrArrayBuilder(val builder: JvmIrBuilder, val arrayType: IrType) { return builder.irBlock { val spreadVar = if (spread is IrGetValue) spread.symbol.owner else irTemporary(spread) val size = unwrappedArrayType.classOrNull!!.getPropertyGetter("size")!! - fun getSize() = irCall(size).apply { dispatchReceiver = irGet(spreadVar) } - val result = irTemporary(newArray(getSize())) - +irCall(builder.irSymbols.systemArraycopy).apply { + val arrayCopyOf = builder.irSymbols.getArraysCopyOfFunction(unwrappedArrayType as IrSimpleType) + // TODO consider using System.arraycopy if the requested array type is non-generic. + +irCall(arrayCopyOf).apply { putValueArgument(0, coerce(irGet(spreadVar), unwrappedArrayType)) - putValueArgument(1, irInt(0)) - putValueArgument(2, irGet(result)) - putValueArgument(3, irInt(0)) - putValueArgument(4, getSize()) + putValueArgument(1, irCall(size).apply { dispatchReceiver = irGet(spreadVar) }) } - +irGet(result) } } diff --git a/compiler/testData/codegen/box/vararg/kt45187.kt b/compiler/testData/codegen/box/vararg/kt45187.kt new file mode 100644 index 00000000000..9908ee0cab6 --- /dev/null +++ b/compiler/testData/codegen/box/vararg/kt45187.kt @@ -0,0 +1,13 @@ +// TARGET_BACKEND: JVM + +fun box(): String = + object : A() { + override fun f(vararg params: Void): Void? = null + }.execute() + +abstract class A { + protected abstract fun f(vararg params: P): R? + + fun execute(vararg params: P): String = + if (f(*params) == null) "OK" else "Fail" +} diff --git a/compiler/testData/codegen/box/vararg/spreadCopiesArray.kt b/compiler/testData/codegen/box/vararg/spreadCopiesArray.kt index 5582075eda9..d3d1697a61d 100644 --- a/compiler/testData/codegen/box/vararg/spreadCopiesArray.kt +++ b/compiler/testData/codegen/box/vararg/spreadCopiesArray.kt @@ -8,7 +8,15 @@ fun copyArray(vararg data: T): Array = data inline fun reifiedCopyArray(vararg data: T): Array = data +fun copyBooleanArray(vararg data: Boolean): BooleanArray = data +fun copyByteArray(vararg data: Byte): ByteArray = data +fun copyCharArray(vararg data: Char): CharArray = data +fun copyShortArray(vararg data: Short): ShortArray = data fun copyIntArray(vararg data: Int): IntArray = data +fun copyLongArray(vararg data: Long): LongArray = data +fun copyFloatArray(vararg data: Float): FloatArray = data +fun copyDoubleArray(vararg data: Double): DoubleArray = data +fun copyStringArray(vararg data: String): Array = data fun box(): String { val sarr = arrayOf("OK") @@ -21,11 +29,51 @@ fun box(): String { rsarr[0] = "Array was not copied" assertEquals(rsarr2[0], "OK", "Failed: Array, reified copy") + val boolArray = booleanArrayOf(true) + val boolArray2 = copyBooleanArray(*boolArray) + boolArray[0] = false + assertEquals(boolArray2[0], true, "Failed: BooleanArray") + + val byteArray = byteArrayOf(1) + val byteArray2 = copyByteArray(*byteArray) + byteArray[0] = 42 + assertEquals(1, byteArray2[0], "Failed: ByteArray") + + val charArray = charArrayOf('a') + val charArray2 = copyCharArray(*charArray) + charArray[0] = 'b' + assertEquals(charArray2[0], 'a', "Failed: CharArray") + + val shortArray = shortArrayOf(1) + val shortArray2 = copyShortArray(*shortArray) + shortArray[0] = 42 + assertEquals(1, shortArray2[0], "Failed: ShortArray") + val iarr = IntArray(1) iarr[0] = 1 val iarr2 = copyIntArray(*iarr) iarr[0] = 42 - assertEquals(iarr2[0], 1, "Failed: IntArray") + assertEquals(1, iarr2[0], "Failed: IntArray") + + val longArray = longArrayOf(1L) + val longArray2 = copyLongArray(*longArray) + longArray[0] = 42L + assertEquals(1L, longArray2[0], "Failed: LongArray") + + val floatArray = floatArrayOf(1.0f) + val floatArray2 = copyFloatArray(*floatArray) + floatArray[0] = 42.0f + assertEquals(1.0f, floatArray2[0], "Failed: FloatArray") + + val doubleArray = doubleArrayOf(1.0) + val doubleArray2 = copyDoubleArray(*doubleArray) + doubleArray[0] = 42.0 + assertEquals(1.0, doubleArray2[0], "Failed: DoubleArray") + + val stringArray = arrayOf("abc") + val stringArray2 = copyStringArray(*stringArray) + stringArray[0] = "def" + assertEquals("abc", stringArray2[0], "Failed: Array") return "OK" } diff --git a/compiler/testData/ir/irText/classes/kt45853.fir.txt b/compiler/testData/ir/irText/classes/kt45853.fir.txt new file mode 100644 index 00000000000..17d78db5dad --- /dev/null +++ b/compiler/testData/ir/irText/classes/kt45853.fir.txt @@ -0,0 +1,55 @@ +FILE fqName: fileName:/kt45853.kt + CLASS CLASS name:A modality:ABSTRACT visibility:public superTypes:[kotlin.Any] + $this: VALUE_PARAMETER INSTANCE_RECEIVER name: type:.A + CONSTRUCTOR visibility:public <> () returnType:.A [primary] + BLOCK_BODY + DELEGATING_CONSTRUCTOR_CALL 'public constructor () [primary] declared in kotlin.Any' + INSTANCE_INITIALIZER_CALL classDescriptor='CLASS CLASS name:A modality:ABSTRACT visibility:public superTypes:[kotlin.Any]' + PROPERTY name:a visibility:public modality:ABSTRACT [val] + FUN DEFAULT_PROPERTY_ACCESSOR name: visibility:public modality:ABSTRACT <> ($this:.A) returnType:.A? + correspondingProperty: PROPERTY name:a visibility:public modality:ABSTRACT [val] + $this: VALUE_PARAMETER name: type:.A + FUN FAKE_OVERRIDE name:equals visibility:public modality:OPEN <> ($this:kotlin.Any, other:kotlin.Any?) returnType:kotlin.Boolean [fake_override,operator] + overridden: + public open fun equals (other: kotlin.Any?): kotlin.Boolean [operator] declared in kotlin.Any + $this: VALUE_PARAMETER name: type:kotlin.Any + VALUE_PARAMETER name:other index:0 type:kotlin.Any? + FUN FAKE_OVERRIDE name:hashCode visibility:public modality:OPEN <> ($this:kotlin.Any) returnType:kotlin.Int [fake_override] + overridden: + public open fun hashCode (): kotlin.Int declared in kotlin.Any + $this: VALUE_PARAMETER name: type:kotlin.Any + FUN FAKE_OVERRIDE name:toString visibility:public modality:OPEN <> ($this:kotlin.Any) returnType:kotlin.String [fake_override] + overridden: + public open fun toString (): kotlin.String declared in kotlin.Any + $this: VALUE_PARAMETER name: type:kotlin.Any + CLASS CLASS name:B modality:FINAL visibility:public superTypes:[.AX] + $this: VALUE_PARAMETER INSTANCE_RECEIVER name: type:.B + CONSTRUCTOR visibility:public <> () returnType:.B [primary] + BLOCK_BODY + DELEGATING_CONSTRUCTOR_CALL 'public constructor () [primary] declared in .AX' + INSTANCE_INITIALIZER_CALL classDescriptor='CLASS CLASS name:B modality:FINAL visibility:public superTypes:[.AX]' + FUN name:getA visibility:public modality:FINAL <> ($this:.B) returnType:.X? + $this: VALUE_PARAMETER name: type:.B + BLOCK_BODY + RETURN type=kotlin.Nothing from='public final fun getA (): .X? declared in .B' + CALL 'public open fun (): @[FlexibleNullability] .AX? declared in .AX' superQualifier='CLASS IR_EXTERNAL_JAVA_DECLARATION_STUB CLASS name:AX modality:ABSTRACT visibility:public superTypes:[.A; .X]' type=@[FlexibleNullability] .AX? origin=GET_PROPERTY + $this: GET_VAR ': .B declared in .B.getA' type=.B origin=null + PROPERTY FAKE_OVERRIDE name:a visibility:public modality:OPEN [fake_override,val] + FUN FAKE_OVERRIDE name: visibility:public modality:OPEN <> ($this:.AX) returnType:@[FlexibleNullability] .AX? [fake_override] + correspondingProperty: PROPERTY FAKE_OVERRIDE name:a visibility:public modality:OPEN [fake_override,val] + overridden: + public open fun (): @[FlexibleNullability] .AX? declared in .AX + $this: VALUE_PARAMETER name: type:.AX + FUN FAKE_OVERRIDE name:equals visibility:public modality:OPEN <> ($this:kotlin.Any, other:kotlin.Any?) returnType:kotlin.Boolean [fake_override,operator] + overridden: + public open fun equals (other: kotlin.Any?): kotlin.Boolean [fake_override,operator] declared in .AX + $this: VALUE_PARAMETER name: type:kotlin.Any + VALUE_PARAMETER name:other index:0 type:kotlin.Any? + FUN FAKE_OVERRIDE name:hashCode visibility:public modality:OPEN <> ($this:kotlin.Any) returnType:kotlin.Int [fake_override] + overridden: + public open fun hashCode (): kotlin.Int [fake_override] declared in .AX + $this: VALUE_PARAMETER name: type:kotlin.Any + FUN FAKE_OVERRIDE name:toString visibility:public modality:OPEN <> ($this:kotlin.Any) returnType:kotlin.String [fake_override] + overridden: + public open fun toString (): kotlin.String [fake_override] declared in .AX + $this: VALUE_PARAMETER name: type:kotlin.Any diff --git a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/BlackBoxCodegenTestGenerated.java b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/BlackBoxCodegenTestGenerated.java index a0b5a787709..1cc09af74ba 100644 --- a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/BlackBoxCodegenTestGenerated.java +++ b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/BlackBoxCodegenTestGenerated.java @@ -40922,6 +40922,12 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest { runTest("compiler/testData/codegen/box/vararg/kt37779.kt"); } + @Test + @TestMetadata("kt45187.kt") + public void testKt45187() throws Exception { + runTest("compiler/testData/codegen/box/vararg/kt45187.kt"); + } + @Test @TestMetadata("kt581.kt") public void testKt581() throws Exception { diff --git a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/IrBlackBoxCodegenTestGenerated.java b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/IrBlackBoxCodegenTestGenerated.java index 2d1349f7217..25f80d530f4 100644 --- a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/IrBlackBoxCodegenTestGenerated.java +++ b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/IrBlackBoxCodegenTestGenerated.java @@ -40940,6 +40940,12 @@ public class IrBlackBoxCodegenTestGenerated extends AbstractIrBlackBoxCodegenTes runTest("compiler/testData/codegen/box/vararg/kt37779.kt"); } + @Test + @TestMetadata("kt45187.kt") + public void testKt45187() throws Exception { + runTest("compiler/testData/codegen/box/vararg/kt45187.kt"); + } + @Test @TestMetadata("kt581.kt") public void testKt581() throws Exception { diff --git a/compiler/tests-gen/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java b/compiler/tests-gen/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java index 05e2710ecc7..d102066ab77 100644 --- a/compiler/tests-gen/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java +++ b/compiler/tests-gen/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java @@ -32907,6 +32907,11 @@ public class LightAnalysisModeTestGenerated extends AbstractLightAnalysisModeTes runTest("compiler/testData/codegen/box/vararg/kt37779.kt"); } + @TestMetadata("kt45187.kt") + public void testKt45187() throws Exception { + runTest("compiler/testData/codegen/box/vararg/kt45187.kt"); + } + @TestMetadata("kt581.kt") public void testKt581() throws Exception { runTest("compiler/testData/codegen/box/vararg/kt581.kt");