From 742c3a228a8a7a81d51f7749b77adb80e87237c9 Mon Sep 17 00:00:00 2001 From: Mikhael Bogdanov Date: Mon, 18 Nov 2019 16:49:12 +0100 Subject: [PATCH] JVM_IR. Proper lower `toArray` functions --- .../jvm/lower/SyntheticAccessorLowering.kt | 11 +- .../backend/jvm/lower/ToArrayLowering.kt | 283 +++++++++--------- .../box/toArray/toArrayShouldBePublic.kt | 1 - .../toArray/noAccessorForToArray.kt | 66 ++++ .../codegen/BytecodeTextTestGenerated.java | 18 ++ .../ir/IrBytecodeTextTestGenerated.java | 18 ++ 6 files changed, 259 insertions(+), 138 deletions(-) create mode 100644 compiler/testData/codegen/bytecodeText/toArray/noAccessorForToArray.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 d414c178ce3..b8b0896cc13 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 @@ -484,7 +484,8 @@ internal class SyntheticAccessorLowering(val context: JvmBackendContext) : IrEle /// We assume that IR code that reaches us has been checked for correctness at the frontend. /// This function needs to single out those cases where Java accessibility rules differ from Kotlin's. - val declarationRaw = owner as IrDeclarationWithVisibility + val symbolOwner = owner + val declarationRaw = symbolOwner as IrDeclarationWithVisibility val declaration = (declarationRaw as? IrSimpleFunction)?.resolveFakeOverride() ?: (declarationRaw as? IrField)?.resolveFakeOverride() ?: declarationRaw @@ -495,6 +496,14 @@ internal class SyntheticAccessorLowering(val context: JvmBackendContext) : IrEle // `internal` maps to public and requires no accessor. 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()) { + return true + } + } + // If local variables are accessible by Kotlin rules, they also are by Java rules. val symbolDeclarationContainer = (declaration.parent as? IrDeclarationContainer) as? IrElement ?: return true val contextDeclarationContainer = (currentScope!!.irElement as IrDeclaration).getAccessContext(withSuper) ?: return false diff --git a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/lower/ToArrayLowering.kt b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/lower/ToArrayLowering.kt index 04c9a1380a9..ce9d89bd322 100644 --- a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/lower/ToArrayLowering.kt +++ b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/lower/ToArrayLowering.kt @@ -11,6 +11,7 @@ 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 @@ -18,7 +19,6 @@ import org.jetbrains.kotlin.ir.builders.irBlockBody import org.jetbrains.kotlin.ir.builders.irCall import org.jetbrains.kotlin.ir.builders.irReturn import org.jetbrains.kotlin.ir.declarations.IrClass -import org.jetbrains.kotlin.ir.declarations.IrDeclaration import org.jetbrains.kotlin.ir.declarations.IrDeclarationOrigin import org.jetbrains.kotlin.ir.declarations.IrSimpleFunction import org.jetbrains.kotlin.ir.declarations.impl.IrFunctionImpl @@ -35,7 +35,9 @@ import org.jetbrains.kotlin.ir.types.* import org.jetbrains.kotlin.ir.util.defaultType 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( @@ -46,173 +48,162 @@ internal val toArrayPhase = makeIrFilePhase( private class ToArrayLowering(private val context: JvmBackendContext) : ClassLoweringPass { override fun lower(irClass: IrClass) { - if (irClass.isJvmInterface || !irClass.isDirectCollectionSubClass()) return + if (irClass.isJvmInterface || !irClass.isCollectionSubClass()) return val irBuiltIns = context.irBuiltIns val symbols = context.ir.symbols val toArrayName = Name.identifier("toArray") - val genericToArray = irClass.declarations.find { it.isGenericToArray() } - val nonGenericToArray = irClass.declarations.find { it.isNonGenericToArray() } - + val genericToArray = irClass.declarations.filterIsInstance().find { it.isGenericToArray(context) } + val nonGenericToArray = irClass.declarations.filterIsInstance().find { it.isNonGenericToArray() } + val isDirectCollectionSubClass = irClass.isDirectCollectionSubClass() if (genericToArray == null) { - 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) - } - - 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 - ) - functionDescriptor.bind(irFunction) - irFunction.parent = irClass - - typeParameter.parent = irFunction - irFunction.typeParameters.add(typeParameter) - - val dispatchReceiverParameterDescriptor = WrappedValueParameterDescriptor() - irFunction.dispatchReceiverParameter = IrValueParameterImpl( - UNDEFINED_OFFSET, UNDEFINED_OFFSET, - JvmLoweredDeclarationOrigin.TO_ARRAY, - IrValueParameterSymbolImpl(dispatchReceiverParameterDescriptor), - Name.special(""), - index = -1, - type = irClass.defaultType, - varargElementType = null, - isCrossinline = false, - isNoinline = false - ).apply { - parent = irFunction - } - val valueParameterDescriptor = WrappedValueParameterDescriptor() - irFunction.valueParameters.add( - IrValueParameterImpl( + if (isDirectCollectionSubClass) { + val typeParameterDescriptor = WrappedTypeParameterDescriptor() + val typeParameter = IrTypeParameterImpl( UNDEFINED_OFFSET, UNDEFINED_OFFSET, JvmLoweredDeclarationOrigin.TO_ARRAY, - IrValueParameterSymbolImpl(valueParameterDescriptor), - Name.identifier("array"), + IrTypeParameterSymbolImpl(typeParameterDescriptor), + Name.identifier("T"), index = 0, + variance = Variance.INVARIANT, + isReified = false + ).apply { + typeParameterDescriptor.bind(this) + superTypes.add(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 + ) + functionDescriptor.bind(irFunction) + irFunction.parent = irClass + + typeParameter.parent = irFunction + irFunction.typeParameters.add(typeParameter) + + val dispatchReceiverParameterDescriptor = WrappedValueParameterDescriptor() + irFunction.dispatchReceiverParameter = IrValueParameterImpl( + UNDEFINED_OFFSET, UNDEFINED_OFFSET, + JvmLoweredDeclarationOrigin.TO_ARRAY, + IrValueParameterSymbolImpl(dispatchReceiverParameterDescriptor), + Name.special(""), + index = -1, + type = irClass.defaultType, varargElementType = null, - type = substitutedArrayType, isCrossinline = false, isNoinline = false ).apply { - valueParameterDescriptor.bind(this) parent = irFunction } - ) + val valueParameterDescriptor = WrappedValueParameterDescriptor() + irFunction.valueParameters.add( + 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)) - }) + 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)) + }) + } + + irClass.declarations.add(irFunction) } - - irClass.declarations.add(irFunction) } else { - //TODO patch visibility + (genericToArray as IrFunctionImpl).visibility = Visibilities.PUBLIC } if (nonGenericToArray == null) { - 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 - ) - functionDescriptor.bind(irFunction) - irFunction.parent = irClass + 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 + ) + functionDescriptor.bind(irFunction) + irFunction.parent = irClass - val dispatchReceiverParameterDescriptor = WrappedValueParameterDescriptor() - irFunction.dispatchReceiverParameter = IrValueParameterImpl( - UNDEFINED_OFFSET, UNDEFINED_OFFSET, - JvmLoweredDeclarationOrigin.TO_ARRAY, - IrValueParameterSymbolImpl(dispatchReceiverParameterDescriptor), - Name.special(""), - index = -1, - type = irClass.defaultType, - varargElementType = null, - isCrossinline = false, - isNoinline = false - ).apply { - parent = irFunction + val dispatchReceiverParameterDescriptor = WrappedValueParameterDescriptor() + irFunction.dispatchReceiverParameter = IrValueParameterImpl( + UNDEFINED_OFFSET, UNDEFINED_OFFSET, + JvmLoweredDeclarationOrigin.TO_ARRAY, + IrValueParameterSymbolImpl(dispatchReceiverParameterDescriptor), + Name.special(""), + index = -1, + type = irClass.defaultType, + varargElementType = null, + isCrossinline = false, + isNoinline = false + ).apply { + parent = irFunction + } + + 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) + ) + }) + } + + irClass.declarations.add(irFunction) } - - 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) - ) - }) - } - - irClass.declarations.add(irFunction) } else { - //TODO patch visibility + (nonGenericToArray as IrFunctionImpl).visibility = Visibilities.PUBLIC } } - - private fun IrDeclaration.isGenericToArray(): Boolean { - if (this !is IrSimpleFunction) return false - val signature = context.methodSignatureMapper.mapAsmMethod(this) - return signature.toString() == "toArray([Ljava/lang/Object;)[Ljava/lang/Object;" - } - - private fun IrDeclaration.isNonGenericToArray(): Boolean { - if (this !is IrSimpleFunction) return false - if (this.name.asString() != "toArray") return false - if (typeParameters.isNotEmpty() || valueParameters.isNotEmpty()) return false - if (!returnType.isArray()) return false - - return true - } } private val IrClass.superClasses get() = superTypes.mapNotNull { it.getClass() } // Have to check by name, since irBuiltins is unreliable. -private fun IrClass.isCollectionSubClass() = +internal fun IrClass.isCollectionSubClass() = DFS.ifAny(listOf(this), IrClass::superClasses) { it.defaultType.isCollection() } // If this class inherits from another Kotlin class that implements Collection, it already has toArray. @@ -220,3 +211,23 @@ private fun IrClass.isDirectCollectionSubClass() = isCollectionSubClass() && !superClasses.any { it.isClass && it.origin != IrDeclarationOrigin.IR_EXTERNAL_JAVA_DECLARATION_STUB && it.isCollectionSubClass() } + + +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() +} \ No newline at end of file diff --git a/compiler/testData/codegen/box/toArray/toArrayShouldBePublic.kt b/compiler/testData/codegen/box/toArray/toArrayShouldBePublic.kt index 9612c8a8cbf..ebe10a9631a 100644 --- a/compiler/testData/codegen/box/toArray/toArrayShouldBePublic.kt +++ b/compiler/testData/codegen/box/toArray/toArrayShouldBePublic.kt @@ -1,5 +1,4 @@ // IGNORE_BACKEND_FIR: JVM_IR -// IGNORE_BACKEND: JVM_IR // TARGET_BACKEND: JVM // WITH_RUNTIME diff --git a/compiler/testData/codegen/bytecodeText/toArray/noAccessorForToArray.kt b/compiler/testData/codegen/bytecodeText/toArray/noAccessorForToArray.kt new file mode 100644 index 00000000000..aec869ff2e9 --- /dev/null +++ b/compiler/testData/codegen/bytecodeText/toArray/noAccessorForToArray.kt @@ -0,0 +1,66 @@ +// TARGET_BACKEND: JVM +// WITH_RUNTIME + +// FILE: SingletonCollection.kt +package test + +open class SingletonCollection(val value: T) : AbstractCollection() { + override val size = 1 + override fun iterator(): Iterator = listOf(value).iterator() + + protected override final fun toArray(): Array = + arrayOf(value) + + protected override final fun toArray(a: Array): Array { + a[0] = value as E + return a + } +} + +open class SingletonCollection2(val value: T) : AbstractCollection() { + override val size = 1 + override fun iterator(): Iterator = listOf(value).iterator() +} + +// FILE: DerivedSingletonCollection.kt +package test2 + +import test.* + +class DerivedSingletonCollection(value: T) : SingletonCollection(value) { + fun test() = object { + fun test() = toArray() + }.test() + + fun test(a: Array) = object { + fun test() = toArray(a) + }.test() +} + +class DerivedSingletonCollection2(value: T) : SingletonCollection2(value) { + fun test() = object { + fun test() = toArray() + }.test() + + fun test(a: Array) = object { + fun test() = toArray(a) + }.test() + +} + +// @test/SingletonCollection.class: +// 0 access\$ +// 2 public final toArray +// 0 \.toArray + +// @test/SingletonCollection2.class: +// 0 access\$ +// 0 toArray + +// @test2/DerivedSingletonCollection.class: +// 0 access\$ +// 0 toArray + +// @test2/DerivedSingletonCollection2.class: +// 0 access\$ +// 0 toArray diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/BytecodeTextTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/BytecodeTextTestGenerated.java index a588fad5491..b5c2d930766 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/BytecodeTextTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/BytecodeTextTestGenerated.java @@ -4061,6 +4061,24 @@ public class BytecodeTextTestGenerated extends AbstractBytecodeTextTest { } } + @TestMetadata("compiler/testData/codegen/bytecodeText/toArray") + @TestDataPath("$PROJECT_ROOT") + @RunWith(JUnit3RunnerWithInners.class) + public static class ToArray extends AbstractBytecodeTextTest { + private void runTest(String testDataFilePath) throws Exception { + KotlinTestUtils.runTest(this::doTest, TargetBackend.JVM, testDataFilePath); + } + + public void testAllFilesPresentInToArray() throws Exception { + KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("compiler/testData/codegen/bytecodeText/toArray"), Pattern.compile("^(.+)\\.kt$"), TargetBackend.JVM, true); + } + + @TestMetadata("noAccessorForToArray.kt") + public void testNoAccessorForToArray() throws Exception { + runTest("compiler/testData/codegen/bytecodeText/toArray/noAccessorForToArray.kt"); + } + } + @TestMetadata("compiler/testData/codegen/bytecodeText/unsignedTypes") @TestDataPath("$PROJECT_ROOT") @RunWith(JUnit3RunnerWithInners.class) diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBytecodeTextTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBytecodeTextTestGenerated.java index ce5296eec99..46ca9370a50 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBytecodeTextTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBytecodeTextTestGenerated.java @@ -3979,6 +3979,24 @@ public class IrBytecodeTextTestGenerated extends AbstractIrBytecodeTextTest { } } + @TestMetadata("compiler/testData/codegen/bytecodeText/toArray") + @TestDataPath("$PROJECT_ROOT") + @RunWith(JUnit3RunnerWithInners.class) + public static class ToArray extends AbstractIrBytecodeTextTest { + private void runTest(String testDataFilePath) throws Exception { + KotlinTestUtils.runTest(this::doTest, TargetBackend.JVM_IR, testDataFilePath); + } + + public void testAllFilesPresentInToArray() throws Exception { + KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("compiler/testData/codegen/bytecodeText/toArray"), Pattern.compile("^(.+)\\.kt$"), TargetBackend.JVM_IR, true); + } + + @TestMetadata("noAccessorForToArray.kt") + public void testNoAccessorForToArray() throws Exception { + runTest("compiler/testData/codegen/bytecodeText/toArray/noAccessorForToArray.kt"); + } + } + @TestMetadata("compiler/testData/codegen/bytecodeText/unsignedTypes") @TestDataPath("$PROJECT_ROOT") @RunWith(JUnit3RunnerWithInners.class)