Fix range-based 'for' loop with 'continue' in range bounds

1. Search for increment function in range element type, not in inferred
induction variable type
(which can be inappropriate, e.g., 'Nothing' in case of 'continue').

2. Handle nested loops with shared exit labels
(generated by JVM_IR for KT-37370 case).

KT-37370 KT-37373
This commit is contained in:
Dmitry Petrov
2020-03-16 11:40:35 +03:00
parent d5b65abc5d
commit 6809f4439c
14 changed files with 188 additions and 19 deletions
@@ -23,6 +23,7 @@ import org.jetbrains.kotlin.codegen.inline.isMarkedReturn
import org.jetbrains.kotlin.codegen.optimization.common.MethodAnalyzer
import org.jetbrains.kotlin.codegen.optimization.common.OptimizationBasicInterpreter
import org.jetbrains.kotlin.codegen.pseudoInsns.PseudoInsn
import org.jetbrains.kotlin.utils.SmartList
import org.jetbrains.org.objectweb.asm.Opcodes
import org.jetbrains.org.objectweb.asm.tree.AbstractInsnNode
import org.jetbrains.org.objectweb.asm.tree.JumpInsnNode
@@ -44,28 +45,43 @@ internal class FixStackAnalyzer(
const val DEAD_CODE_STACK_SIZE = -1
}
private val expectedStackNode = hashMapOf<LabelNode, AbstractInsnNode>()
private val loopEntryPointMarkers = hashMapOf<LabelNode, SmartList<AbstractInsnNode>>()
val maxExtraStackSize: Int get() = analyzer.maxExtraStackSize
fun getStackToSpill(location: AbstractInsnNode) = analyzer.spilledStacks[location]
fun getActualStack(location: AbstractInsnNode) = getFrame(location)?.getStackContent()
fun getActualStackSize(location: AbstractInsnNode) = getFrame(location)?.stackSizeWithExtra ?: DEAD_CODE_STACK_SIZE
fun getExpectedStackSize(location: AbstractInsnNode) = getExpectedStackFrame(location)?.stackSizeWithExtra ?: DEAD_CODE_STACK_SIZE
private fun getExpectedStackFrame(location: AbstractInsnNode) = getFrame(expectedStackNode[location] ?: location)
fun getExpectedStackSize(location: AbstractInsnNode): Int {
// We should look for expected stack size at loop entry point markers if available,
// otherwise at location itself.
val expectedStackSizeNodes = loopEntryPointMarkers[location] ?: listOf(location)
// Find 1st live node among expected stack size nodes and return corresponding stack size
for (node in expectedStackSizeNodes) {
val frame = getFrame(node) ?: continue
return frame.stackSizeWithExtra
}
// No live nodes found
// => loop entry point is unreachable or node itself is unreachable
return DEAD_CODE_STACK_SIZE
}
private fun getFrame(location: AbstractInsnNode) = analyzer.getFrame(location) as? InternalAnalyzer.FixStackFrame
fun analyze() {
preprocess()
recordLoopEntryPointMarkers()
analyzer.analyze()
}
private fun preprocess() {
private fun recordLoopEntryPointMarkers() {
// NB JVM_IR can generate nested loops with same exit labels (see kt37370.kt)
for (marker in context.fakeAlwaysFalseIfeqMarkers) {
val next = marker.next
if (next is JumpInsnNode) {
expectedStackNode[next.label] = marker
loopEntryPointMarkers.getOrPut(next.label) { SmartList() }.add(marker)
}
}
}
@@ -106,7 +106,7 @@ class FixStackMethodTransformer : MethodTransformer() {
"Label at $labelIndex, jump at $gotoIndex: stack underflow: $expectedStackSize > $actualStackSize"
}
val actualStackContent = analyzer.getActualStack(gotoNode)
?: throw AssertionError("Jump at $gotoIndex should be alive")
?: throw AssertionError("Jump at $gotoIndex should be alive")
actions.add { replaceMarkerWithPops(methodNode, gotoNode.previous, expectedStackSize, actualStackContent) }
} else if (actualStackSize >= 0 && expectedStackSize < 0) {
throw AssertionError("Live jump $gotoIndex to dead label $labelIndex")
@@ -18951,6 +18951,16 @@ public class FirBlackBoxCodegenTestGenerated extends AbstractFirBlackBoxCodegenT
runTest("compiler/testData/codegen/box/ranges/forNullableIntInRangeWithImplicitReceiver.kt");
}
@TestMetadata("kt37370.kt")
public void testKt37370() throws Exception {
runTest("compiler/testData/codegen/box/ranges/kt37370.kt");
}
@TestMetadata("kt37370a.kt")
public void testKt37370a() throws Exception {
runTest("compiler/testData/codegen/box/ranges/kt37370a.kt");
}
@TestMetadata("multiAssignmentIterationOverIntRange.kt")
public void testMultiAssignmentIterationOverIntRange() throws Exception {
runTest("compiler/testData/codegen/box/ranges/multiAssignmentIterationOverIntRange.kt");
@@ -73,8 +73,12 @@ internal abstract class NumericForLoopHeader<T : NumericHeaderInfo>(
// Always copy `lastExpression` is it may be used in multiple conditions.
get() = field.deepCopyWithSymbols()
private val elementType: IrType
init {
with(builder) {
elementType = headerInfo.progressionType.elementType(context.irBuiltIns)
// For this loop:
//
// for (i in first()..last() step step())
@@ -86,7 +90,7 @@ internal abstract class NumericForLoopHeader<T : NumericHeaderInfo>(
// LongProgression so last() should be cast to a Long.
inductionVariable = scope.createTemporaryVariable(
headerInfo.first.castIfNecessary(
headerInfo.progressionType.elementType(context.irBuiltIns),
elementType,
headerInfo.progressionType.elementCastFunctionName
),
nameHint = "inductionVariable",
@@ -99,7 +103,7 @@ internal abstract class NumericForLoopHeader<T : NumericHeaderInfo>(
// TODO: Confirm if casting to non-nullable is still necessary
val last = ensureNotNullable(
headerInfo.last.castIfNecessary(
headerInfo.progressionType.elementType(context.irBuiltIns),
elementType,
headerInfo.progressionType.elementCastFunctionName
)
)
@@ -138,7 +142,7 @@ internal abstract class NumericForLoopHeader<T : NumericHeaderInfo>(
/** Statement used to increment the induction variable. */
protected fun incrementInductionVariable(builder: DeclarationIrBuilder): IrStatement = with(builder) {
// inductionVariable = inductionVariable + step
val plusFun = inductionVariable.type.getClass()!!.functions.single {
val plusFun = elementType.getClass()!!.functions.single {
it.name == OperatorNameConventions.PLUS &&
it.valueParameters.size == 1 &&
it.valueParameters[0].type == stepVariable.type
+65
View File
@@ -0,0 +1,65 @@
// WITH_RUNTIME
fun testContinue1() {
for (i in 0..1) {
for (j in continue downTo 1) {}
}
}
fun testContinue2() {
for (i in 0..1) {
for (j in 1 downTo continue) {}
}
}
fun falseCond() = false
fun testContinue3() {
for (i in 0..1) {
for (j in (if (falseCond()) 10 else continue) downTo 1) {}
}
}
fun testContinue4() {
for (i in 0..1) {
for (j in 10 downTo (if (falseCond()) 1 else continue)) {}
}
}
fun testContinue5() {
for (i in 0..1) {
for (j in (
if (try {
if (falseCond()) true else return
} finally {
if (!falseCond()) continue
}
)
10
else
continue
)
downTo 1) {
//...
}
}
}
fun start(i1: Int, i2: Int, i3: Int) = 10
fun testContinue6() {
for (i in 0..1) {
for (j in start(1, 2, continue) downTo 0) {
}
}
}
fun box(): String {
testContinue1()
testContinue2()
testContinue3()
testContinue4()
testContinue5()
testContinue6()
return "OK"
}
+28
View File
@@ -0,0 +1,28 @@
// IGNORE_BACKEND_FIR: JVM_IR
// WITH_RUNTIME
import kotlin.test.*
fun testContinue7() {
var x = 0
fun inc() = ++x
for (i in 0..1) {
for (j in inc() downTo continue) {}
}
assertEquals(2, x)
}
fun testContinue8() {
var x = 0
fun inc() = ++x
for (i in 0..1) {
for (j in continue downTo inc()) {}
}
assertEquals(0, x)
}
fun box(): String {
testContinue7()
testContinue8()
return "OK"
}
@@ -1,4 +1,3 @@
// IGNORE_BACKEND_FIR: JVM_IR
// WITH_RUNTIME
// KJS_WITH_FULL_RUNTIME
@@ -1,8 +1,5 @@
// IGNORE_BACKEND_FIR: JVM_IR
// WITH_RUNTIME
// KJS_WITH_FULL_RUNTIME
// IGNORE_BACKEND: JVM_IR
// ^ TODO KT-37373
fun testContinue() {
for (i in 0..1) {
@@ -20467,6 +20467,16 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest {
runTest("compiler/testData/codegen/box/ranges/forNullableIntInRangeWithImplicitReceiver.kt");
}
@TestMetadata("kt37370.kt")
public void testKt37370() throws Exception {
runTest("compiler/testData/codegen/box/ranges/kt37370.kt");
}
@TestMetadata("kt37370a.kt")
public void testKt37370a() throws Exception {
runTest("compiler/testData/codegen/box/ranges/kt37370a.kt");
}
@TestMetadata("multiAssignmentIterationOverIntRange.kt")
public void testMultiAssignmentIterationOverIntRange() throws Exception {
runTest("compiler/testData/codegen/box/ranges/multiAssignmentIterationOverIntRange.kt");
@@ -20467,6 +20467,16 @@ public class LightAnalysisModeTestGenerated extends AbstractLightAnalysisModeTes
runTest("compiler/testData/codegen/box/ranges/forNullableIntInRangeWithImplicitReceiver.kt");
}
@TestMetadata("kt37370.kt")
public void testKt37370() throws Exception {
runTest("compiler/testData/codegen/box/ranges/kt37370.kt");
}
@TestMetadata("kt37370a.kt")
public void testKt37370a() throws Exception {
runTest("compiler/testData/codegen/box/ranges/kt37370a.kt");
}
@TestMetadata("multiAssignmentIterationOverIntRange.kt")
public void testMultiAssignmentIterationOverIntRange() throws Exception {
runTest("compiler/testData/codegen/box/ranges/multiAssignmentIterationOverIntRange.kt");
@@ -18951,6 +18951,16 @@ public class IrBlackBoxCodegenTestGenerated extends AbstractIrBlackBoxCodegenTes
runTest("compiler/testData/codegen/box/ranges/forNullableIntInRangeWithImplicitReceiver.kt");
}
@TestMetadata("kt37370.kt")
public void testKt37370() throws Exception {
runTest("compiler/testData/codegen/box/ranges/kt37370.kt");
}
@TestMetadata("kt37370a.kt")
public void testKt37370a() throws Exception {
runTest("compiler/testData/codegen/box/ranges/kt37370a.kt");
}
@TestMetadata("multiAssignmentIterationOverIntRange.kt")
public void testMultiAssignmentIterationOverIntRange() throws Exception {
runTest("compiler/testData/codegen/box/ranges/multiAssignmentIterationOverIntRange.kt");
@@ -1871,16 +1871,16 @@ public class IrTextTestCaseGenerated extends AbstractIrTextTestCase {
runTest("compiler/testData/ir/irText/types/coercionToUnitInLambdaReturnValue.kt");
}
@TestMetadata("genericFunWithStar.kt")
public void testGenericFunWithStar() throws Exception {
runTest("compiler/testData/ir/irText/types/genericFunWithStar.kt");
}
@TestMetadata("genericDelegatedDeepProperty.kt")
public void testGenericDelegatedDeepProperty() throws Exception {
runTest("compiler/testData/ir/irText/types/genericDelegatedDeepProperty.kt");
}
@TestMetadata("genericFunWithStar.kt")
public void testGenericFunWithStar() throws Exception {
runTest("compiler/testData/ir/irText/types/genericFunWithStar.kt");
}
@TestMetadata("genericPropertyReferenceType.kt")
public void testGenericPropertyReferenceType() throws Exception {
runTest("compiler/testData/ir/irText/types/genericPropertyReferenceType.kt");
@@ -15872,6 +15872,16 @@ public class IrJsCodegenBoxTestGenerated extends AbstractIrJsCodegenBoxTest {
runTest("compiler/testData/codegen/box/ranges/forNullableIntInRangeWithImplicitReceiver.kt");
}
@TestMetadata("kt37370.kt")
public void testKt37370() throws Exception {
runTest("compiler/testData/codegen/box/ranges/kt37370.kt");
}
@TestMetadata("kt37370a.kt")
public void testKt37370a() throws Exception {
runTest("compiler/testData/codegen/box/ranges/kt37370a.kt");
}
@TestMetadata("multiAssignmentIterationOverIntRange.kt")
public void testMultiAssignmentIterationOverIntRange() throws Exception {
runTest("compiler/testData/codegen/box/ranges/multiAssignmentIterationOverIntRange.kt");
@@ -15977,6 +15977,16 @@ public class JsCodegenBoxTestGenerated extends AbstractJsCodegenBoxTest {
runTest("compiler/testData/codegen/box/ranges/forNullableIntInRangeWithImplicitReceiver.kt");
}
@TestMetadata("kt37370.kt")
public void testKt37370() throws Exception {
runTest("compiler/testData/codegen/box/ranges/kt37370.kt");
}
@TestMetadata("kt37370a.kt")
public void testKt37370a() throws Exception {
runTest("compiler/testData/codegen/box/ranges/kt37370a.kt");
}
@TestMetadata("multiAssignmentIterationOverIntRange.kt")
public void testMultiAssignmentIterationOverIntRange() throws Exception {
runTest("compiler/testData/codegen/box/ranges/multiAssignmentIterationOverIntRange.kt");