From 6809f4439c18f9363846bff29d0a515de7d702e9 Mon Sep 17 00:00:00 2001 From: Dmitry Petrov Date: Mon, 16 Mar 2020 11:40:35 +0300 Subject: [PATCH] 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 --- .../optimization/fixStack/FixStackAnalyzer.kt | 28 ++++++-- .../fixStack/FixStackMethodTransformer.kt | 2 +- .../ir/FirBlackBoxCodegenTestGenerated.java | 10 +++ .../common/lower/loops/HeaderProcessor.kt | 10 ++- .../testData/codegen/box/ranges/kt37370.kt | 65 +++++++++++++++++++ .../testData/codegen/box/ranges/kt37370a.kt | 28 ++++++++ .../codegen/box/ranges/unsigned/kt36953.kt | 1 - .../box/ranges/unsigned/kt36953_continue.kt | 3 - .../codegen/BlackBoxCodegenTestGenerated.java | 10 +++ .../LightAnalysisModeTestGenerated.java | 10 +++ .../ir/IrBlackBoxCodegenTestGenerated.java | 10 +++ .../kotlin/ir/IrTextTestCaseGenerated.java | 10 +-- .../IrJsCodegenBoxTestGenerated.java | 10 +++ .../semantics/JsCodegenBoxTestGenerated.java | 10 +++ 14 files changed, 188 insertions(+), 19 deletions(-) create mode 100644 compiler/testData/codegen/box/ranges/kt37370.kt create mode 100644 compiler/testData/codegen/box/ranges/kt37370a.kt diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/fixStack/FixStackAnalyzer.kt b/compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/fixStack/FixStackAnalyzer.kt index bfbc421a54c..a554bf94c67 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/fixStack/FixStackAnalyzer.kt +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/fixStack/FixStackAnalyzer.kt @@ -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() + private val loopEntryPointMarkers = hashMapOf>() 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) } } } diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/fixStack/FixStackMethodTransformer.kt b/compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/fixStack/FixStackMethodTransformer.kt index 3d3bb861758..d7404717a47 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/fixStack/FixStackMethodTransformer.kt +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/fixStack/FixStackMethodTransformer.kt @@ -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") diff --git a/compiler/fir/fir2ir/tests/org/jetbrains/kotlin/codegen/ir/FirBlackBoxCodegenTestGenerated.java b/compiler/fir/fir2ir/tests/org/jetbrains/kotlin/codegen/ir/FirBlackBoxCodegenTestGenerated.java index 73905f8e27b..2cb1b4a4efe 100644 --- a/compiler/fir/fir2ir/tests/org/jetbrains/kotlin/codegen/ir/FirBlackBoxCodegenTestGenerated.java +++ b/compiler/fir/fir2ir/tests/org/jetbrains/kotlin/codegen/ir/FirBlackBoxCodegenTestGenerated.java @@ -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"); diff --git a/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/lower/loops/HeaderProcessor.kt b/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/lower/loops/HeaderProcessor.kt index 4a6a359f18c..af329e0e342 100644 --- a/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/lower/loops/HeaderProcessor.kt +++ b/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/lower/loops/HeaderProcessor.kt @@ -73,8 +73,12 @@ internal abstract class NumericForLoopHeader( // 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( // 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( // 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( /** 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 diff --git a/compiler/testData/codegen/box/ranges/kt37370.kt b/compiler/testData/codegen/box/ranges/kt37370.kt new file mode 100644 index 00000000000..b795fca4f5d --- /dev/null +++ b/compiler/testData/codegen/box/ranges/kt37370.kt @@ -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" +} \ No newline at end of file diff --git a/compiler/testData/codegen/box/ranges/kt37370a.kt b/compiler/testData/codegen/box/ranges/kt37370a.kt new file mode 100644 index 00000000000..cba79358b15 --- /dev/null +++ b/compiler/testData/codegen/box/ranges/kt37370a.kt @@ -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" +} \ No newline at end of file diff --git a/compiler/testData/codegen/box/ranges/unsigned/kt36953.kt b/compiler/testData/codegen/box/ranges/unsigned/kt36953.kt index 0442b3e0a4c..61b204783d0 100644 --- a/compiler/testData/codegen/box/ranges/unsigned/kt36953.kt +++ b/compiler/testData/codegen/box/ranges/unsigned/kt36953.kt @@ -1,4 +1,3 @@ -// IGNORE_BACKEND_FIR: JVM_IR // WITH_RUNTIME // KJS_WITH_FULL_RUNTIME diff --git a/compiler/testData/codegen/box/ranges/unsigned/kt36953_continue.kt b/compiler/testData/codegen/box/ranges/unsigned/kt36953_continue.kt index 98b3a7e636a..946822fe2d1 100644 --- a/compiler/testData/codegen/box/ranges/unsigned/kt36953_continue.kt +++ b/compiler/testData/codegen/box/ranges/unsigned/kt36953_continue.kt @@ -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) { diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java index f61d9e06ef4..37a9b22d996 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java @@ -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"); diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java index b9e2e237f08..b0836e3415f 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java @@ -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"); diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java index 113854f105e..32a66555cff 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java @@ -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"); diff --git a/compiler/tests/org/jetbrains/kotlin/ir/IrTextTestCaseGenerated.java b/compiler/tests/org/jetbrains/kotlin/ir/IrTextTestCaseGenerated.java index 3b8e9b7b079..190d48480c2 100644 --- a/compiler/tests/org/jetbrains/kotlin/ir/IrTextTestCaseGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/ir/IrTextTestCaseGenerated.java @@ -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"); diff --git a/js/js.tests/test/org/jetbrains/kotlin/js/test/ir/semantics/IrJsCodegenBoxTestGenerated.java b/js/js.tests/test/org/jetbrains/kotlin/js/test/ir/semantics/IrJsCodegenBoxTestGenerated.java index f1b70ea9233..c8aa19f7702 100644 --- a/js/js.tests/test/org/jetbrains/kotlin/js/test/ir/semantics/IrJsCodegenBoxTestGenerated.java +++ b/js/js.tests/test/org/jetbrains/kotlin/js/test/ir/semantics/IrJsCodegenBoxTestGenerated.java @@ -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"); diff --git a/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/JsCodegenBoxTestGenerated.java b/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/JsCodegenBoxTestGenerated.java index 813af8da3a6..dc8f06a32ae 100644 --- a/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/JsCodegenBoxTestGenerated.java +++ b/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/JsCodegenBoxTestGenerated.java @@ -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");