From 3f28b71509be692465598280815a27e70486bdda Mon Sep 17 00:00:00 2001 From: Mads Ager Date: Tue, 3 Sep 2019 13:39:27 +0200 Subject: [PATCH] [IR] Fix issue getting the length property from CharSequence bounded type parameter Use the class of the subtype of CharSequence when available. When it is not (for type parameters bounded by CharSequence) call the CharSequence getter and 'get' method. Using the most specific type posible fixes the forInStringSpecialized test that expects the use of INVOKEVIRTUAL and not INVOKEINTERFACE. Add tests for the type parameter use. --- .../common/lower/loops/ProgressionHandlers.kt | 33 ++++++++++++------- .../forLoop/forInCharSequenceTypeParameter.kt | 10 ++++++ .../forInCharSequenceTypeParameterIndices.kt | 27 +++++++++++++++ .../codegen/BytecodeTextTestGenerated.java | 10 ++++++ .../ir/IrBytecodeTextTestGenerated.java | 10 ++++++ 5 files changed, 79 insertions(+), 11 deletions(-) create mode 100644 compiler/testData/codegen/bytecodeText/forLoop/forInCharSequenceTypeParameter.kt create mode 100644 compiler/testData/codegen/bytecodeText/forLoop/forInIndices/forInCharSequenceTypeParameterIndices.kt diff --git a/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/lower/loops/ProgressionHandlers.kt b/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/lower/loops/ProgressionHandlers.kt index 9d6d0e8e6cd..940be685991 100644 --- a/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/lower/loops/ProgressionHandlers.kt +++ b/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/lower/loops/ProgressionHandlers.kt @@ -17,7 +17,6 @@ import org.jetbrains.kotlin.ir.declarations.IrSimpleFunction import org.jetbrains.kotlin.ir.declarations.IrVariable import org.jetbrains.kotlin.ir.expressions.IrCall import org.jetbrains.kotlin.ir.expressions.IrExpression -import org.jetbrains.kotlin.ir.symbols.IrSimpleFunctionSymbol import org.jetbrains.kotlin.ir.symbols.IrSymbol import org.jetbrains.kotlin.ir.types.* import org.jetbrains.kotlin.ir.util.* @@ -220,7 +219,7 @@ internal class UntilHandler(private val context: CommonBackendContext, private v } /** Builds a [HeaderInfo] for progressions built using the `indices` extension property. */ -internal abstract class IndicesHandler(private val context: CommonBackendContext) : ProgressionHandler { +internal abstract class IndicesHandler(protected val context: CommonBackendContext) : ProgressionHandler { // TODO: Handle Collection<*>.indices @@ -228,8 +227,7 @@ internal abstract class IndicesHandler(private val context: CommonBackendContext with(context.createIrBuilder(scopeOwner, expression.startOffset, expression.endOffset)) { // `last = array.size - 1` (last is inclusive) for the loop `for (i in array.indices)`. val receiver = expression.extensionReceiver!! - val sizeProperty = receiver.type.getClass()!!.properties.first { it.name.asString() == sizePropertyName() } - val last = irCall(sizeProperty.getter!!).apply { + val last = irCall(receiver.type.sizePropertyGetter).apply { dispatchReceiver = receiver }.decrement() @@ -243,7 +241,7 @@ internal abstract class IndicesHandler(private val context: CommonBackendContext ) } - internal abstract fun sizePropertyName() : String + abstract val IrType.sizePropertyGetter: IrSimpleFunction } internal class ArrayIndicesHandler(context: CommonBackendContext) : IndicesHandler(context) { @@ -253,7 +251,8 @@ internal class ArrayIndicesHandler(context: CommonBackendContext) : IndicesHandl parameterCount { it == 0 } } - override fun sizePropertyName(): String = "size" + override val IrType.sizePropertyGetter + get() = getClass()!!.properties.first { it.name.asString() == "size" }.getter!! } internal class CharSequenceIndicesHandler(context: CommonBackendContext) : IndicesHandler(context) { @@ -264,7 +263,13 @@ internal class CharSequenceIndicesHandler(context: CommonBackendContext) : Indic parameterCount { it == 0 } } - override fun sizePropertyName(): String = "length" + // The lowering operates on subtypes of CharSequence. Therefore, the IrType could be + // a type parameter bounded by CharSequence. When that is the case, we cannot get + // the class from the type and instead uses the CharSequence getter. + override val IrType.sizePropertyGetter + get() = getClass()?.properties?.first { it.name.asString() == "length" }?.let { + it.getter!! + } ?: context.ir.symbols.charSequence.getPropertyGetter("length")!!.owner } /** Builds a [HeaderInfo] for calls to reverse an iterable. */ @@ -372,7 +377,7 @@ internal class ArrayIterationHandler(context: CommonBackendContext) : IndexedGet override fun match(expression: IrExpression) = expression.type.run { isArray() || isPrimitiveArray() } override val IrType.sizePropertyGetter - get() = getClass()!!.properties.first { it.name == Name.identifier("size") }.getter!! + get() = getClass()!!.properties.first { it.name.asString() == "size" }.getter!! override val IrType.getFunction get() = getClass()!!.functions.first { it.name.asString() == "get" } @@ -382,9 +387,15 @@ internal class ArrayIterationHandler(context: CommonBackendContext) : IndexedGet internal class CharSequenceIterationHandler(context: CommonBackendContext) : IndexedGetIterationHandler(context) { override fun match(expression: IrExpression) = expression.type.isSubtypeOfClass(context.ir.symbols.charSequence) + // The lowering operates on subtypes of CharSequence. Therefore, the IrType could be + // a type parameter bounded by CharSequence. When that is the case, we cannot get + // the class from the type and instead uses the CharSequence getter and function. override val IrType.sizePropertyGetter - get() = context.ir.symbols.charSequence.getPropertyGetter("length")!!.owner + get() = getClass()?.properties?.first { it.name.asString() == "length" }?.let { + it.getter!! + } ?: context.ir.symbols.charSequence.getPropertyGetter("length")!!.owner override val IrType.getFunction - get() = context.ir.symbols.charSequence.getSimpleFunction("get")!!.owner -} \ No newline at end of file + get() = getClass()?.functions?.first { it.name.asString() == "get" } + ?: context.ir.symbols.charSequence.getSimpleFunction("get")!!.owner +} diff --git a/compiler/testData/codegen/bytecodeText/forLoop/forInCharSequenceTypeParameter.kt b/compiler/testData/codegen/bytecodeText/forLoop/forInCharSequenceTypeParameter.kt new file mode 100644 index 00000000000..d924104788a --- /dev/null +++ b/compiler/testData/codegen/bytecodeText/forLoop/forInCharSequenceTypeParameter.kt @@ -0,0 +1,10 @@ +fun test(sequence: T) { + var s = "" + for (c in sequence) { + s += c + } +} + +// 0 iterator +// 0 hasNext +// 0 nextChar diff --git a/compiler/testData/codegen/bytecodeText/forLoop/forInIndices/forInCharSequenceTypeParameterIndices.kt b/compiler/testData/codegen/bytecodeText/forLoop/forInIndices/forInCharSequenceTypeParameterIndices.kt new file mode 100644 index 00000000000..67fcc2d533a --- /dev/null +++ b/compiler/testData/codegen/bytecodeText/forLoop/forInIndices/forInCharSequenceTypeParameterIndices.kt @@ -0,0 +1,27 @@ +fun test(s: T): Int { + var result = 0 + for (i in s.indices) { + result = result * 10 + (i + 1) + } + return result +} + +// JVM non-IR uses while. +// JVM IR uses if + do-while. + +// 0 iterator +// 0 getStart +// 0 getEnd +// 0 getFirst +// 0 getLast + +// JVM_TEMPLATES +// 0 IF_ICMPGT +// 0 IF_ICMPEQ +// 1 IF_ICMPGE +// 1 IF + +// JVM_IR_TEMPLATES +// 1 IF_ICMPGT +// 1 IF_ICMPLE +// 2 IF diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/BytecodeTextTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/BytecodeTextTestGenerated.java index 0b3d15e8721..1b6a4fbf1bf 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/BytecodeTextTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/BytecodeTextTestGenerated.java @@ -1677,6 +1677,11 @@ public class BytecodeTextTestGenerated extends AbstractBytecodeTextTest { runTest("compiler/testData/codegen/bytecodeText/forLoop/forInCharSequence.kt"); } + @TestMetadata("forInCharSequenceTypeParameter.kt") + public void testForInCharSequenceTypeParameter() throws Exception { + runTest("compiler/testData/codegen/bytecodeText/forLoop/forInCharSequenceTypeParameter.kt"); + } + @TestMetadata("forInDownToCharMinValue.kt") public void testForInDownToCharMinValue() throws Exception { runTest("compiler/testData/codegen/bytecodeText/forLoop/forInDownToCharMinValue.kt"); @@ -1890,6 +1895,11 @@ public class BytecodeTextTestGenerated extends AbstractBytecodeTextTest { runTest("compiler/testData/codegen/bytecodeText/forLoop/forInIndices/forInCharSequenceIndices.kt"); } + @TestMetadata("forInCharSequenceTypeParameterIndices.kt") + public void testForInCharSequenceTypeParameterIndices() throws Exception { + runTest("compiler/testData/codegen/bytecodeText/forLoop/forInIndices/forInCharSequenceTypeParameterIndices.kt"); + } + @TestMetadata("forInCollectionImplicitReceiverIndices.kt") public void testForInCollectionImplicitReceiverIndices() throws Exception { runTest("compiler/testData/codegen/bytecodeText/forLoop/forInIndices/forInCollectionImplicitReceiverIndices.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBytecodeTextTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBytecodeTextTestGenerated.java index 72f07099c2f..1cf375adf4c 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBytecodeTextTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBytecodeTextTestGenerated.java @@ -1632,6 +1632,11 @@ public class IrBytecodeTextTestGenerated extends AbstractIrBytecodeTextTest { runTest("compiler/testData/codegen/bytecodeText/forLoop/forInCharSequence.kt"); } + @TestMetadata("forInCharSequenceTypeParameter.kt") + public void testForInCharSequenceTypeParameter() throws Exception { + runTest("compiler/testData/codegen/bytecodeText/forLoop/forInCharSequenceTypeParameter.kt"); + } + @TestMetadata("forInDownToCharMinValue.kt") public void testForInDownToCharMinValue() throws Exception { runTest("compiler/testData/codegen/bytecodeText/forLoop/forInDownToCharMinValue.kt"); @@ -1845,6 +1850,11 @@ public class IrBytecodeTextTestGenerated extends AbstractIrBytecodeTextTest { runTest("compiler/testData/codegen/bytecodeText/forLoop/forInIndices/forInCharSequenceIndices.kt"); } + @TestMetadata("forInCharSequenceTypeParameterIndices.kt") + public void testForInCharSequenceTypeParameterIndices() throws Exception { + runTest("compiler/testData/codegen/bytecodeText/forLoop/forInIndices/forInCharSequenceTypeParameterIndices.kt"); + } + @TestMetadata("forInCollectionImplicitReceiverIndices.kt") public void testForInCollectionImplicitReceiverIndices() throws Exception { runTest("compiler/testData/codegen/bytecodeText/forLoop/forInIndices/forInCollectionImplicitReceiverIndices.kt");