JVM_IR: refactor ToArrayLowering and make matching more precise

* If `toArray` is inherited from Java, it may take an argument or return a value of a flexible type, which looks nullable in IR; * The returned array may also have `out` variance of the type argument; * `Array<T>` is not the same as `Array<Any?>` if `T` is neither `Any` nor `in Something`, so presence of `toArray(): Array<T>` does not mean we don't need to generate a new `toArray`.
2020-02-28 10:46:34 +01:00
parent a2b6282d49
commit 20ea77d55d
9 changed files with 157 additions and 180 deletions
@@ -26683,6 +26683,11 @@ public class FirBlackBoxCodegenTestGenerated extends AbstractFirBlackBoxCodegenT
            runTest("compiler/testData/codegen/box/toArray/toArrayAlreadyPresent.kt");
        }

+        @TestMetadata("toArrayFromJava.kt")
+        public void testToArrayFromJava() throws Exception {
+            runTest("compiler/testData/codegen/box/toArray/toArrayFromJava.kt");
+        }
+
        @TestMetadata("toArrayShouldBePublic.kt")
        public void testToArrayShouldBePublic() throws Exception {
            runTest("compiler/testData/codegen/box/toArray/toArrayShouldBePublic.kt");
@@ -559,9 +559,8 @@ internal class SyntheticAccessorLowering(val context: JvmBackendContext) : IrEle
        if (!withSuper && !declaration.visibility.isPrivate && !declaration.visibility.isProtected) return true

        //`toArray` is always accessible cause mapped to public functions
-        if (symbolOwner is IrSimpleFunction && (symbolOwner.isNonGenericToArray() || symbolOwner.isGenericToArray(context))) {
-            val parent = symbolOwner.parent
-            if (parent is IrClass && parent.isCollectionSubClass()) {
+        if (symbolOwner is IrSimpleFunction && (symbolOwner.isNonGenericToArray(context) || symbolOwner.isGenericToArray(context))) {
+            if (symbolOwner.parentAsClass.isCollectionSubClass) {
                return true
            }
        }
@@ -11,33 +11,27 @@ import org.jetbrains.kotlin.backend.common.phaser.makeIrFilePhase
 import org.jetbrains.kotlin.backend.jvm.JvmBackendContext
 import org.jetbrains.kotlin.backend.jvm.JvmLoweredDeclarationOrigin
 import org.jetbrains.kotlin.backend.jvm.codegen.isJvmInterface
-import org.jetbrains.kotlin.builtins.KotlinBuiltIns
 import org.jetbrains.kotlin.descriptors.Modality
 import org.jetbrains.kotlin.descriptors.Visibilities
-import org.jetbrains.kotlin.ir.UNDEFINED_OFFSET
+import org.jetbrains.kotlin.ir.builders.declarations.addDispatchReceiver
+import org.jetbrains.kotlin.ir.builders.declarations.addFunction
+import org.jetbrains.kotlin.ir.builders.declarations.addTypeParameter
+import org.jetbrains.kotlin.ir.builders.declarations.addValueParameter
 import org.jetbrains.kotlin.ir.builders.irBlockBody
 import org.jetbrains.kotlin.ir.builders.irCall
+import org.jetbrains.kotlin.ir.builders.irGet
 import org.jetbrains.kotlin.ir.builders.irReturn
 import org.jetbrains.kotlin.ir.declarations.IrClass
 import org.jetbrains.kotlin.ir.declarations.IrDeclarationOrigin
 import org.jetbrains.kotlin.ir.declarations.IrSimpleFunction
 import org.jetbrains.kotlin.ir.declarations.impl.IrFunctionImpl
-import org.jetbrains.kotlin.ir.declarations.impl.IrTypeParameterImpl
-import org.jetbrains.kotlin.ir.declarations.impl.IrValueParameterImpl
-import org.jetbrains.kotlin.ir.descriptors.WrappedSimpleFunctionDescriptor
-import org.jetbrains.kotlin.ir.descriptors.WrappedTypeParameterDescriptor
-import org.jetbrains.kotlin.ir.descriptors.WrappedValueParameterDescriptor
-import org.jetbrains.kotlin.ir.expressions.impl.IrGetValueImpl
-import org.jetbrains.kotlin.ir.symbols.impl.IrSimpleFunctionSymbolImpl
-import org.jetbrains.kotlin.ir.symbols.impl.IrTypeParameterSymbolImpl
-import org.jetbrains.kotlin.ir.symbols.impl.IrValueParameterSymbolImpl
+import org.jetbrains.kotlin.ir.symbols.IrClassifierSymbol
 import org.jetbrains.kotlin.ir.types.*
 import org.jetbrains.kotlin.ir.util.defaultType
+import org.jetbrains.kotlin.ir.util.functions
 import org.jetbrains.kotlin.ir.util.isClass
 import org.jetbrains.kotlin.name.Name
-import org.jetbrains.kotlin.resolve.descriptorUtil.builtIns
 import org.jetbrains.kotlin.types.Variance
-import org.jetbrains.kotlin.types.checker.KotlinTypeChecker
 import org.jetbrains.kotlin.utils.DFS

 internal val toArrayPhase = makeIrFilePhase(
@@ -47,188 +41,99 @@ internal val toArrayPhase = makeIrFilePhase(
 )

 private class ToArrayLowering(private val context: JvmBackendContext) : ClassLoweringPass {
+    private val symbols = context.ir.symbols
+
    override fun lower(irClass: IrClass) {
-        if (irClass.isJvmInterface || !irClass.isCollectionSubClass()) return
+        if (irClass.isJvmInterface || !irClass.isCollectionSubClass) return

-        val irBuiltIns = context.irBuiltIns
-        val symbols = context.ir.symbols
+        val indirectCollectionSubClass = generateSequence(irClass.superClass, IrClass::superClass).firstOrNull {
+            it.origin != IrDeclarationOrigin.IR_EXTERNAL_JAVA_DECLARATION_STUB
+        }?.isCollectionSubClass == true

-        val toArrayName = Name.identifier("toArray")
-        val genericToArray = irClass.declarations.filterIsInstance<IrSimpleFunction>().find { it.isGenericToArray(context) }
-        val nonGenericToArray = irClass.declarations.filterIsInstance<IrSimpleFunction>().find { it.isNonGenericToArray() }
-        val isDirectCollectionSubClass = irClass.isDirectCollectionSubClass()
-        if (genericToArray == null) {
-            if (isDirectCollectionSubClass) {
-                val typeParameterDescriptor = WrappedTypeParameterDescriptor()
-                val typeParameter = IrTypeParameterImpl(
-                    UNDEFINED_OFFSET, UNDEFINED_OFFSET,
-                    JvmLoweredDeclarationOrigin.TO_ARRAY,
-                    IrTypeParameterSymbolImpl(typeParameterDescriptor),
-                    Name.identifier("T"),
-                    index = 0,
-                    variance = Variance.INVARIANT,
-                    isReified = false
-                ).apply {
-                    typeParameterDescriptor.bind(this)
-                    superTypes.add(irBuiltIns.anyNType)
+        irClass.findOrCreate(indirectCollectionSubClass, { it.isGenericToArray(context) }) {
+            irClass.addFunction {
+                name = Name.identifier("toArray")
+                origin = JvmLoweredDeclarationOrigin.TO_ARRAY
+                modality = Modality.OPEN
+            }.apply {
+                val elementType = addTypeParameter {
+                    name = Name.identifier("T")
+                    origin = JvmLoweredDeclarationOrigin.TO_ARRAY
+                    superTypes.add(context.irBuiltIns.anyNType)
                }
-
-                val substitutedArrayType = irBuiltIns.arrayClass.typeWith(typeParameter.defaultType)
-                val functionDescriptor = WrappedSimpleFunctionDescriptor()
-                val irFunction = IrFunctionImpl(
-                    UNDEFINED_OFFSET, UNDEFINED_OFFSET,
-                    JvmLoweredDeclarationOrigin.TO_ARRAY,
-                    IrSimpleFunctionSymbolImpl(functionDescriptor),
-                    toArrayName,
-                    Visibilities.PUBLIC,
-                    Modality.OPEN,
-                    returnType = substitutedArrayType,
-                    isInline = false,
-                    isExternal = false,
-                    isTailrec = false,
-                    isSuspend = false,
-                    isExpect = false,
-                    isFakeOverride = false,
-                isOperator = false
-            )
-            functionDescriptor.bind(irFunction)
-            irFunction.parent = irClass
-
-                typeParameter.parent = irFunction
-                irFunction.typeParameters += typeParameter
-
-                val dispatchReceiverParameterDescriptor = WrappedValueParameterDescriptor()
-                irFunction.dispatchReceiverParameter = IrValueParameterImpl(
-                    UNDEFINED_OFFSET, UNDEFINED_OFFSET,
-                    JvmLoweredDeclarationOrigin.TO_ARRAY,
-                    IrValueParameterSymbolImpl(dispatchReceiverParameterDescriptor),
-                    Name.special("<this>"),
-                    index = -1,
-                    type = irClass.defaultType,
-                    varargElementType = null,
-                    isCrossinline = false,
-                    isNoinline = false
-                ).apply {
-                    parent = irFunction
+                returnType = context.irBuiltIns.arrayClass.typeWith(elementType.defaultType)
+                val receiver = addDispatchReceiver {
+                    type = irClass.defaultType
+                    origin = JvmLoweredDeclarationOrigin.TO_ARRAY
                }
-                val valueParameterDescriptor = WrappedValueParameterDescriptor()
-                irFunction.valueParameters +=
-                    IrValueParameterImpl(
-                        UNDEFINED_OFFSET, UNDEFINED_OFFSET,
-                        JvmLoweredDeclarationOrigin.TO_ARRAY,
-                        IrValueParameterSymbolImpl(valueParameterDescriptor),
-                        Name.identifier("array"),
-                        index = 0,
-                        varargElementType = null,
-                        type = substitutedArrayType,
-                        isCrossinline = false,
-                        isNoinline = false
-                    ).apply {
-                        valueParameterDescriptor.bind(this)
-                        parent = irFunction
-                    }
-
-                irFunction.body = context.createIrBuilder(irFunction.symbol).irBlockBody {
-                    +irReturn(
-                        irCall(symbols.genericToArray, symbols.genericToArray.owner.returnType).apply {
-                            putValueArgument(
-                                0,
-                                IrGetValueImpl(UNDEFINED_OFFSET, UNDEFINED_OFFSET, irFunction.dispatchReceiverParameter!!.symbol)
-                            )
-                            putValueArgument(1, IrGetValueImpl(UNDEFINED_OFFSET, UNDEFINED_OFFSET, irFunction.valueParameters[0].symbol))
-                        })
+                val prototype = addValueParameter("array", returnType, JvmLoweredDeclarationOrigin.TO_ARRAY)
+                body = context.createIrBuilder(symbol).irBlockBody {
+                    +irReturn(irCall(symbols.genericToArray, symbols.genericToArray.owner.returnType).apply {
+                        putValueArgument(0, irGet(receiver))
+                        putValueArgument(1, irGet(prototype))
+                    })
                }
-
-                irClass.declarations.add(irFunction)
            }
-        } else {
-            (genericToArray as IrFunctionImpl).visibility = Visibilities.PUBLIC
        }

-        if (nonGenericToArray == null) {
-            if (isDirectCollectionSubClass) {
-                val functionDescriptor = WrappedSimpleFunctionDescriptor()
-                val irFunction = IrFunctionImpl(
-                    UNDEFINED_OFFSET, UNDEFINED_OFFSET,
-                    JvmLoweredDeclarationOrigin.TO_ARRAY,
-                    IrSimpleFunctionSymbolImpl(functionDescriptor),
-                    toArrayName,
-                    Visibilities.PUBLIC,
-                    Modality.OPEN,
-                    returnType = irBuiltIns.arrayClass.typeWith(irBuiltIns.anyNType),
-                    isInline = false,
-                    isExternal = false,
-                    isTailrec = false,
-                    isSuspend = false,
-                    isExpect = false,
-                    isFakeOverride = false,
-                isOperator = false
-            )
-            functionDescriptor.bind(irFunction)
-            irFunction.parent = irClass
-
-                val dispatchReceiverParameterDescriptor = WrappedValueParameterDescriptor()
-                irFunction.dispatchReceiverParameter = IrValueParameterImpl(
-                    UNDEFINED_OFFSET, UNDEFINED_OFFSET,
-                    JvmLoweredDeclarationOrigin.TO_ARRAY,
-                    IrValueParameterSymbolImpl(dispatchReceiverParameterDescriptor),
-                    Name.special("<this>"),
-                    index = -1,
-                    type = irClass.defaultType,
-                    varargElementType = null,
-                    isCrossinline = false,
-                    isNoinline = false
-                ).apply {
-                    parent = irFunction
+        irClass.findOrCreate(indirectCollectionSubClass, { it.isNonGenericToArray(context) }) {
+            irClass.addFunction {
+                name = Name.identifier("toArray")
+                origin = JvmLoweredDeclarationOrigin.TO_ARRAY
+                modality = Modality.OPEN
+                returnType = context.irBuiltIns.arrayClass.typeWith(context.irBuiltIns.anyNType)
+            }.apply {
+                val receiver = addDispatchReceiver {
+                    type = irClass.defaultType
+                    origin = JvmLoweredDeclarationOrigin.TO_ARRAY
                }
-
-                irFunction.body = context.createIrBuilder(irFunction.symbol).irBlockBody {
-                    +irReturn(
-                        irCall(symbols.nonGenericToArray, symbols.nonGenericToArray.owner.returnType).apply {
-                            putValueArgument(
-                                0,
-                                IrGetValueImpl(UNDEFINED_OFFSET, UNDEFINED_OFFSET, irFunction.dispatchReceiverParameter!!.symbol)
-                            )
-                        })
+                body = context.createIrBuilder(symbol).irBlockBody {
+                    +irReturn(irCall(symbols.nonGenericToArray, symbols.nonGenericToArray.owner.returnType).apply {
+                        putValueArgument(0, irGet(receiver))
+                    })
                }
-
-                irClass.declarations.add(irFunction)
            }
-        } else {
-            (nonGenericToArray as IrFunctionImpl).visibility = Visibilities.PUBLIC
        }
    }
+
+    private fun IrClass.findOrCreate(indirectSubclass: Boolean, matcher: (IrSimpleFunction) -> Boolean, fallback: () -> IrSimpleFunction) {
+        val existing = functions.find(matcher) as? IrFunctionImpl
+        if (existing != null) {
+            // This is an explicit override of a method defined in `kotlin.collections.AbstractCollection`
+            // or `java.util.Collection`. From here on, the frontend will check the existence of implementations;
+            // we just need to match visibility in the former case to the latter.
+            existing.visibility = Visibilities.PUBLIC
+            return
+        }
+        if (indirectSubclass) {
+            // There's a Kotlin class up the hierarchy that should already have `toArray`.
+            return
+        }
+        fallback()
+    }
 }

-private val IrClass.superClasses
-    get() = superTypes.mapNotNull { it.getClass() }
+private val IrClass.superClass: IrClass?
+    get() = superTypes.mapNotNull { it.getClass()?.takeIf { superClass -> superClass.isClass } }.singleOrNull()

-// Have to check by name, since irBuiltins is unreliable.
-internal fun IrClass.isCollectionSubClass() =
-    DFS.ifAny(listOf(this), IrClass::superClasses) { it.defaultType.isCollection() }
+internal val IrClass.isCollectionSubClass: Boolean
+    get() = DFS.ifAny(superTypes, { it.getClass()?.superTypes ?: listOf() }) { it.isCollection() }

-// If this class inherits from another Kotlin class that implements Collection, it already has toArray.
-private fun IrClass.isDirectCollectionSubClass() =
-    isCollectionSubClass() && !superClasses.any {
-        it.isClass && it.origin != IrDeclarationOrigin.IR_EXTERNAL_JAVA_DECLARATION_STUB && it.isCollectionSubClass()
+private fun IrType.isArrayOrNullableArrayOf(context: JvmBackendContext, element: IrClassifierSymbol): Boolean =
+    this is IrSimpleType && (isArray() || isNullableArray()) && arguments.size == 1 && element == when (val it = arguments[0]) {
+        is IrStarProjection -> context.irBuiltIns.anyClass
+        is IrTypeProjection -> if (it.variance == Variance.IN_VARIANCE) context.irBuiltIns.anyClass else it.type.classifierOrNull
+        else -> null
    }

+// Match `fun <T> toArray(prototype: Array<T>): Array<T>`
+internal fun IrSimpleFunction.isGenericToArray(context: JvmBackendContext): Boolean =
+    name.asString() == "toArray" && typeParameters.size == 1 && valueParameters.size == 1 &&
+            extensionReceiverParameter == null &&
+            returnType.isArrayOrNullableArrayOf(context, typeParameters[0].symbol) &&
+            valueParameters[0].type.isArrayOrNullableArrayOf(context, typeParameters[0].symbol)

-internal fun IrSimpleFunction.isGenericToArray(context: JvmBackendContext): Boolean {
-    if (name.asString() != "toArray") return false
-    if (typeParameters.size != 1 || valueParameters.size != 1 || extensionReceiverParameter != null) return false
-
-    val paramType = valueParameters[0].type
-
-    if (!returnType.isArray() || !paramType.isArray()) return false
-
-    val elementType = typeParameters[0].defaultType
-    val expectedType = context.ir.symbols.array.typeWith(elementType)
-    return expectedType == paramType && expectedType == returnType
-}
-
-internal fun IrSimpleFunction.isNonGenericToArray(): Boolean {
-    if (name.asString() != "toArray") return false
-    if (typeParameters.isNotEmpty() || valueParameters.isNotEmpty() || extensionReceiverParameter != null) return false
-    return returnType.isArray()
-}
+// Match `fun toArray(): Array<Any?>`
+internal fun IrSimpleFunction.isNonGenericToArray(context: JvmBackendContext): Boolean =
+    name.asString() == "toArray" && typeParameters.isEmpty() && valueParameters.isEmpty() &&
+            extensionReceiverParameter == null && returnType.isArrayOrNullableArrayOf(context, context.irBuiltIns.anyClass)
@@ -0,0 +1,51 @@
+// IGNORE_BACKEND_FIR: JVM_IR
+// TARGET_BACKEND: JVM
+// The old backend thinks `toArray(): Array<Int?>` is the same as `toArray(): Array<Any?>`
+// IGNORE_BACKEND: JVM
+// WITH_RUNTIME
+// FILE: MyListWithCustomToArray.java
+
+public abstract class MyListWithCustomToArray<E> extends java.util.AbstractList<E> {
+    public Object[] toArray() {
+        return new Object[]{null};
+    }
+
+    public <T> T[] toArray(T[] a) {
+        return a;
+    }
+}
+
+// FILE: a.kt
+class MyList<T>(val list: List<T>): java.util.AbstractList<T>() {
+    override fun get(index: Int): T = list[index]
+
+    override val size: Int
+        get() = list.size
+}
+
+class MyListSubclass<T>(val list: List<T>): MyListWithCustomToArray<T>() {
+    override fun get(index: Int): T = list[index]
+
+    override val size: Int
+        get() = list.size
+}
+
+class MyCollection<T>(val list: List<T>) : Collection<T> by list {
+    fun toArray(): Array<Int?> =
+        arrayOfNulls<Int>(0)
+}
+
+fun box(): String {
+    val list1 = MyList(listOf(2, 3, 9)) as java.util.Collection<*>
+    list1.toArray().contentToString().let { if (it != "[2, 3, 9]") return "fail 1: $it" }
+    list1.toArray(arrayOfNulls<Int>(0)).contentToString().let { if (it != "[2, 3, 9]") return "fail 2: $it" }
+
+    val list2 = MyListSubclass(listOf(2, 3, 9)) as java.util.Collection<*>
+    list2.toArray().contentToString().let { if (it != "[null]") return "fail 3: $it" }
+    list2.toArray(arrayOfNulls<Int>(1)).contentToString().let { if (it != "[null]") return "fail 4: $it" }
+
+    val list3 = MyCollection(listOf(2, 3, 9)) as java.util.Collection<*>
+    list3.toArray().contentToString().let { if (it != "[2, 3, 9]") return "fail 5: $it" }
+    list3.toArray(arrayOfNulls<Int>(0)).contentToString().let { if (it != "[2, 3, 9]") return "fail 6: $it" }
+    return "OK"
+}
@@ -1,4 +1,5 @@
 // TARGET_BACKEND: JVM
+// IGNORE_BACKEND_FIR: JVM_IR
 // WITH_RUNTIME

 // FILE: SingletonCollection.kt
@@ -1,4 +1,5 @@
 // TARGET_BACKEND: JVM
+// IGNORE_BACKEND_FIR: JVM_IR
 // WITH_RUNTIME
 // IGNORE_LIGHT_ANALYSIS

@@ -28199,6 +28199,11 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest {
            runTest("compiler/testData/codegen/box/toArray/toArrayAlreadyPresent.kt");
        }

+        @TestMetadata("toArrayFromJava.kt")
+        public void testToArrayFromJava() throws Exception {
+            runTest("compiler/testData/codegen/box/toArray/toArrayFromJava.kt");
+        }
+
        @TestMetadata("toArrayShouldBePublic.kt")
        public void testToArrayShouldBePublic() throws Exception {
            runTest("compiler/testData/codegen/box/toArray/toArrayShouldBePublic.kt");
@@ -26983,6 +26983,11 @@ public class LightAnalysisModeTestGenerated extends AbstractLightAnalysisModeTes
    @TestDataPath("$PROJECT_ROOT")
    @RunWith(JUnit3RunnerWithInners.class)
    public static class ToArray extends AbstractLightAnalysisModeTest {
+        @TestMetadata("toArrayFromJava.kt")
+        public void ignoreToArrayFromJava() throws Exception {
+            runTest("compiler/testData/codegen/box/toArray/toArrayFromJava.kt");
+        }
+
        private void runTest(String testDataFilePath) throws Exception {
            KotlinTestUtils.runTest(this::doTest, TargetBackend.JVM, testDataFilePath);
        }
@@ -26683,6 +26683,11 @@ public class IrBlackBoxCodegenTestGenerated extends AbstractIrBlackBoxCodegenTes
            runTest("compiler/testData/codegen/box/toArray/toArrayAlreadyPresent.kt");
        }

+        @TestMetadata("toArrayFromJava.kt")
+        public void testToArrayFromJava() throws Exception {
+            runTest("compiler/testData/codegen/box/toArray/toArrayFromJava.kt");
+        }
+
        @TestMetadata("toArrayShouldBePublic.kt")
        public void testToArrayShouldBePublic() throws Exception {
            runTest("compiler/testData/codegen/box/toArray/toArrayShouldBePublic.kt");