From 3f995935d37528a132c3a93f240362e8ddc94690 Mon Sep 17 00:00:00 2001 From: Michael Bogdanov Date: Wed, 11 Nov 2015 13:17:18 +0300 Subject: [PATCH] Fix for KT-9022: Wrong result when use break in if condition #KT-9022 Fixed --- .../kotlin/codegen/ExpressionCodegen.java | 81 +++++++++++-------- .../breakContinueInExpressions/kt9022And.kt | 9 +++ .../breakContinueInExpressions/kt9022Or.kt | 9 +++ .../box/controlStructures/kt9022Return.kt | 26 ++++++ .../box/controlStructures/kt9022Throw.kt | 9 +++ .../BlackBoxCodegenTestGenerated.java | 24 ++++++ 6 files changed, 125 insertions(+), 33 deletions(-) create mode 100644 compiler/testData/codegen/box/controlStructures/breakContinueInExpressions/kt9022And.kt create mode 100644 compiler/testData/codegen/box/controlStructures/breakContinueInExpressions/kt9022Or.kt create mode 100644 compiler/testData/codegen/box/controlStructures/kt9022Return.kt create mode 100644 compiler/testData/codegen/box/controlStructures/kt9022Throw.kt diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/ExpressionCodegen.java b/compiler/backend/src/org/jetbrains/kotlin/codegen/ExpressionCodegen.java index 92c65fee23d..63f66027198 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/ExpressionCodegen.java +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/ExpressionCodegen.java @@ -1229,7 +1229,7 @@ public class ExpressionCodegen extends KtVisitor impleme private StackValue generateBreakOrContinueExpression( @NotNull KtExpressionWithLabel expression, boolean isBreak, - @NotNull Label afterBreakContinueLabel + final @NotNull Label afterBreakContinueLabel ) { assert expression instanceof KtContinueExpression || expression instanceof KtBreakExpression; @@ -1248,10 +1248,16 @@ public class ExpressionCodegen extends KtVisitor impleme if (labelElement == null || loopBlockStackElement.targetLabel != null && labelElement.getReferencedName().equals(loopBlockStackElement.targetLabel.getReferencedName())) { - Label label = isBreak ? loopBlockStackElement.breakLabel : loopBlockStackElement.continueLabel; - PseudoInsnsKt.fixStackAndJump(v, label); - v.mark(afterBreakContinueLabel); - return StackValue.none(); + final Label label = isBreak ? loopBlockStackElement.breakLabel : loopBlockStackElement.continueLabel; + return StackValue.operation(Type.VOID_TYPE, new Function1() { + @Override + public Unit invoke(InstructionAdapter adapter) { + PseudoInsnsKt.fixStackAndJump(v, label); + v.mark(afterBreakContinueLabel); + return Unit.INSTANCE; + } + } + ); } } else { @@ -1859,33 +1865,37 @@ public class ExpressionCodegen extends KtVisitor impleme } @Override - public StackValue visitReturnExpression(@NotNull KtReturnExpression expression, StackValue receiver) { - KtExpression returnedExpression = expression.getReturnedExpression(); - CallableMemberDescriptor descriptor = getContext().getContextDescriptor(); - NonLocalReturnInfo nonLocalReturn = getNonLocalReturnInfo(descriptor, expression); - boolean isNonLocalReturn = nonLocalReturn != null; - if (isNonLocalReturn && !state.isInlineEnabled()) { - state.getDiagnostics().report(Errors.NON_LOCAL_RETURN_IN_DISABLED_INLINE.on(expression)); - genThrow(v, "java/lang/UnsupportedOperationException", - "Non-local returns are not allowed with inlining disabled"); - return StackValue.none(); - } + public StackValue visitReturnExpression(@NotNull final KtReturnExpression expression, StackValue receiver) { + return StackValue.operation(Type.VOID_TYPE, new Function1() { + @Override + public Unit invoke(InstructionAdapter adapter) { + KtExpression returnedExpression = expression.getReturnedExpression(); + CallableMemberDescriptor descriptor = getContext().getContextDescriptor(); + NonLocalReturnInfo nonLocalReturn = getNonLocalReturnInfo(descriptor, expression); + boolean isNonLocalReturn = nonLocalReturn != null; + if (isNonLocalReturn && !state.isInlineEnabled()) { + state.getDiagnostics().report(Errors.NON_LOCAL_RETURN_IN_DISABLED_INLINE.on(expression)); + genThrow(v, "java/lang/UnsupportedOperationException", + "Non-local returns are not allowed with inlining disabled"); + return Unit.INSTANCE; + } - Type returnType = isNonLocalReturn ? nonLocalReturn.returnType : this.returnType; - if (returnedExpression != null) { - gen(returnedExpression, returnType); - } + Type returnType = isNonLocalReturn ? nonLocalReturn.returnType : ExpressionCodegen.this.returnType; + if (returnedExpression != null) { + gen(returnedExpression, returnType); + } - Label afterReturnLabel = new Label(); - generateFinallyBlocksIfNeeded(returnType, afterReturnLabel); + Label afterReturnLabel = new Label(); + generateFinallyBlocksIfNeeded(returnType, afterReturnLabel); - if (isNonLocalReturn) { - InlineCodegenUtil.generateGlobalReturnFlag(v, nonLocalReturn.labelName); - } - v.visitInsn(returnType.getOpcode(Opcodes.IRETURN)); - v.mark(afterReturnLabel); - - return StackValue.none(); + if (isNonLocalReturn) { + InlineCodegenUtil.generateGlobalReturnFlag(v, nonLocalReturn.labelName); + } + v.visitInsn(returnType.getOpcode(Opcodes.IRETURN)); + v.mark(afterReturnLabel); + return Unit.INSTANCE; + } + }); } public void generateFinallyBlocksIfNeeded(Type returnType, @NotNull Label afterReturnLabel) { @@ -3571,10 +3581,15 @@ public class ExpressionCodegen extends KtVisitor impleme } @Override - public StackValue visitThrowExpression(@NotNull KtThrowExpression expression, StackValue receiver) { - gen(expression.getThrownExpression(), JAVA_THROWABLE_TYPE); - v.athrow(); - return StackValue.none(); + public StackValue visitThrowExpression(@NotNull final KtThrowExpression expression, StackValue receiver) { + return StackValue.operation(Type.VOID_TYPE, new Function1() { + @Override + public Unit invoke(InstructionAdapter adapter) { + gen(expression.getThrownExpression(), JAVA_THROWABLE_TYPE); + v.athrow(); + return Unit.INSTANCE; + } + }); } @Override diff --git a/compiler/testData/codegen/box/controlStructures/breakContinueInExpressions/kt9022And.kt b/compiler/testData/codegen/box/controlStructures/breakContinueInExpressions/kt9022And.kt new file mode 100644 index 00000000000..757b5c940be --- /dev/null +++ b/compiler/testData/codegen/box/controlStructures/breakContinueInExpressions/kt9022And.kt @@ -0,0 +1,9 @@ +fun box(): String { + var cycle = true; + while (true) { + if (true && break) { + return "fail" + } + } + return "OK" +} \ No newline at end of file diff --git a/compiler/testData/codegen/box/controlStructures/breakContinueInExpressions/kt9022Or.kt b/compiler/testData/codegen/box/controlStructures/breakContinueInExpressions/kt9022Or.kt new file mode 100644 index 00000000000..3b8a943d781 --- /dev/null +++ b/compiler/testData/codegen/box/controlStructures/breakContinueInExpressions/kt9022Or.kt @@ -0,0 +1,9 @@ +fun box(): String { + var cycle = true; + while (true) { + if (true || break) { + return "OK" + } + } + return "fail" +} \ No newline at end of file diff --git a/compiler/testData/codegen/box/controlStructures/kt9022Return.kt b/compiler/testData/codegen/box/controlStructures/kt9022Return.kt new file mode 100644 index 00000000000..f8ec8a28450 --- /dev/null +++ b/compiler/testData/codegen/box/controlStructures/kt9022Return.kt @@ -0,0 +1,26 @@ +fun testOr(b: Boolean): Boolean { + return b || return !b; +} + +fun testOr(): Boolean { + return true || return false; +} + + +fun testAnd(b: Boolean): Boolean { + return b && return !b; +} + +fun testAnd(): Boolean { + return true && return false; +} + +fun box(): String { + if (testOr(false) != true) return "fail 1" + if (testOr(true) != true) return "fail 2" + if (testAnd(false) != false) return "fail 3" + if (testAnd(true) != false) return "fail 4" + if (testOr() != true) return "fail 5" + if (testAnd() != false) return "fail 6" + return "OK" +} \ No newline at end of file diff --git a/compiler/testData/codegen/box/controlStructures/kt9022Throw.kt b/compiler/testData/codegen/box/controlStructures/kt9022Throw.kt new file mode 100644 index 00000000000..30c0c7ea47a --- /dev/null +++ b/compiler/testData/codegen/box/controlStructures/kt9022Throw.kt @@ -0,0 +1,9 @@ +fun box(): String { + var cycle = true; + while (true) { + if (true || throw java.lang.RuntimeException()) { + return "OK" + } + } + return "fail" +} \ No newline at end of file diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/generated/BlackBoxCodegenTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/generated/BlackBoxCodegenTestGenerated.java index 85aee211f69..cdaac91c5ce 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/generated/BlackBoxCodegenTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/generated/BlackBoxCodegenTestGenerated.java @@ -2368,6 +2368,18 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest { doTest(fileName); } + @TestMetadata("kt9022Return.kt") + public void testKt9022Return() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/controlStructures/kt9022Return.kt"); + doTest(fileName); + } + + @TestMetadata("kt9022Throw.kt") + public void testKt9022Throw() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/controlStructures/kt9022Throw.kt"); + doTest(fileName); + } + @TestMetadata("kt910.kt") public void testKt910() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/controlStructures/kt910.kt"); @@ -2436,6 +2448,18 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest { doTest(fileName); } + @TestMetadata("kt9022And.kt") + public void testKt9022And() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/controlStructures/breakContinueInExpressions/kt9022And.kt"); + doTest(fileName); + } + + @TestMetadata("kt9022Or.kt") + public void testKt9022Or() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/controlStructures/breakContinueInExpressions/kt9022Or.kt"); + doTest(fileName); + } + @TestMetadata("popSizes.kt") public void testPopSizes() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/controlStructures/breakContinueInExpressions/popSizes.kt");