From ab0df532bcbd8b701fcf3d35bd5ef6cfdb86488b Mon Sep 17 00:00:00 2001 From: Michael Nedzelsky Date: Wed, 20 Aug 2014 12:25:14 +0400 Subject: [PATCH] JS backend: for with label, fix KT-5257 JS: for with continue with label fails on runtime, and KT-5262 JS: wrong code for `for` over range when start greater end #KT-5257 Fixed #KT-5262 Fixed --- .../k2js/test/semantics/ForeachTest.java | 20 ++++++++ .../translate/context/DynamicContext.java | 13 ----- .../foreach/ArrayForTranslator.java | 10 ++-- .../foreach/IteratorForTranslator.java | 15 +++--- .../foreach/RangeForTranslator.java | 11 ++--- .../foreach/RangeLiteralForTranslator.java | 37 +++++++------- .../for/cases/forIteratesOverArray.kt | 10 +++- .../for/cases/forIteratesOverLiteralRange.kt | 44 +++++++++++++++++ .../cases/forIteratesOverNonLiteralRange.kt | 16 ++++++ .../forIteratesOverSomethingWithIterator.kt | 21 ++++++++ .../expression/for/cases/forWithWhile.kt | 49 +++++++++++++++++++ .../for/cases/labeledForWithContinue.kt | 36 ++++++++++++++ 12 files changed, 228 insertions(+), 54 deletions(-) create mode 100644 js/js.translator/testData/expression/for/cases/forIteratesOverLiteralRange.kt create mode 100644 js/js.translator/testData/expression/for/cases/forIteratesOverNonLiteralRange.kt create mode 100644 js/js.translator/testData/expression/for/cases/forIteratesOverSomethingWithIterator.kt create mode 100644 js/js.translator/testData/expression/for/cases/forWithWhile.kt create mode 100644 js/js.translator/testData/expression/for/cases/labeledForWithContinue.kt diff --git a/js/js.tests/test/org/jetbrains/k2js/test/semantics/ForeachTest.java b/js/js.tests/test/org/jetbrains/k2js/test/semantics/ForeachTest.java index 55a2514f2b5..44a259aa938 100644 --- a/js/js.tests/test/org/jetbrains/k2js/test/semantics/ForeachTest.java +++ b/js/js.tests/test/org/jetbrains/k2js/test/semantics/ForeachTest.java @@ -34,7 +34,27 @@ public final class ForeachTest extends AbstractExpressionTest { fooBoxTest(); } + public void testLabeledForWithContinue() throws Exception { + checkFooBoxIsOk(); + } + public void testForWithComplexOneStatement() throws Exception { checkFooBoxIsOk(); } + + public void testForIteratesOverLiteralRange() throws Exception { + checkFooBoxIsOk(); + } + + public void testForIteratesOverNonLiteralRange() throws Exception { + checkFooBoxIsOk(); + } + + public void testForIteratesOverSomethingWithIterator() throws Exception { + checkFooBoxIsOk(); + } + + public void testForWithWhile() throws Exception { + checkFooBoxIsOk(); + } } \ No newline at end of file diff --git a/js/js.translator/src/org/jetbrains/k2js/translate/context/DynamicContext.java b/js/js.translator/src/org/jetbrains/k2js/translate/context/DynamicContext.java index 800bd869a7f..7d41a3d3b92 100644 --- a/js/js.translator/src/org/jetbrains/k2js/translate/context/DynamicContext.java +++ b/js/js.translator/src/org/jetbrains/k2js/translate/context/DynamicContext.java @@ -17,7 +17,6 @@ package org.jetbrains.k2js.translate.context; import com.google.dart.compiler.backend.js.ast.*; -import com.intellij.openapi.util.Pair; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -79,18 +78,6 @@ public final class DynamicContext { } } - //TODO: replace return type to make it more readable - @NotNull - public Pair createTemporary(@Nullable JsExpression initExpression) { - JsVar var = new JsVar(currentScope.declareTemporary(), initExpression); - return Pair.create(var, (JsExpression) new JsNameRef(var.getName())); - } - - @NotNull - public JsVar createTemporaryVar(@NotNull JsExpression initExpression) { - return new JsVar(currentScope.declareTemporary(), initExpression); - } - @NotNull public JsScope getScope() { return currentScope; diff --git a/js/js.translator/src/org/jetbrains/k2js/translate/expression/foreach/ArrayForTranslator.java b/js/js.translator/src/org/jetbrains/k2js/translate/expression/foreach/ArrayForTranslator.java index 825c86598c7..554021cbd0a 100644 --- a/js/js.translator/src/org/jetbrains/k2js/translate/expression/foreach/ArrayForTranslator.java +++ b/js/js.translator/src/org/jetbrains/k2js/translate/expression/foreach/ArrayForTranslator.java @@ -16,7 +16,6 @@ package org.jetbrains.k2js.translate.expression.foreach; -import com.google.common.collect.Lists; import com.google.dart.compiler.backend.js.ast.*; import org.jetbrains.annotations.NotNull; import org.jetbrains.jet.lang.psi.JetExpression; @@ -29,7 +28,6 @@ import org.jetbrains.k2js.translate.intrinsic.functions.factories.CompositeFIF; import org.jetbrains.k2js.translate.utils.BindingUtils; import java.util.Collections; -import java.util.List; import static org.jetbrains.jet.lang.resolve.DescriptorUtils.getClassDescriptorForType; import static org.jetbrains.k2js.translate.utils.JsAstUtils.*; @@ -75,11 +73,9 @@ public final class ArrayForTranslator extends ForTranslator { } @NotNull - private JsBlock translate() { - List blockStatements = Lists.newArrayList(); - blockStatements.add(temporariesInitialization(loopRange, end).makeStmt()); - blockStatements.add(new JsFor(getInitExpression(), getCondition(), getIncrementExpression(), getBody())); - return new JsBlock(blockStatements); + private JsStatement translate() { + context().addStatementToCurrentBlock(temporariesInitialization(loopRange, end).makeStmt()); + return new JsFor(getInitExpression(), getCondition(), getIncrementExpression(), getBody()); } diff --git a/js/js.translator/src/org/jetbrains/k2js/translate/expression/foreach/IteratorForTranslator.java b/js/js.translator/src/org/jetbrains/k2js/translate/expression/foreach/IteratorForTranslator.java index 6ad85a71283..fdd7ae42cdb 100644 --- a/js/js.translator/src/org/jetbrains/k2js/translate/expression/foreach/IteratorForTranslator.java +++ b/js/js.translator/src/org/jetbrains/k2js/translate/expression/foreach/IteratorForTranslator.java @@ -17,7 +17,6 @@ package org.jetbrains.k2js.translate.expression.foreach; import com.google.dart.compiler.backend.js.ast.*; -import com.intellij.openapi.util.Pair; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.jetbrains.jet.lang.descriptors.FunctionDescriptor; @@ -25,6 +24,7 @@ import org.jetbrains.jet.lang.psi.JetExpression; import org.jetbrains.jet.lang.psi.JetForExpression; import org.jetbrains.jet.lang.resolve.calls.model.ResolvedCall; import org.jetbrains.k2js.translate.callTranslator.CallTranslator; +import org.jetbrains.k2js.translate.context.TemporaryVariable; import org.jetbrains.k2js.translate.context.TranslationContext; import org.jetbrains.k2js.translate.general.Translation; @@ -33,7 +33,7 @@ import static org.jetbrains.k2js.translate.utils.PsiUtils.getLoopRange; public final class IteratorForTranslator extends ForTranslator { @NotNull - private final Pair iterator; + private final TemporaryVariable iteratorVar; @NotNull public static JsStatement doTranslate(@NotNull JetForExpression expression, @@ -43,23 +43,24 @@ public final class IteratorForTranslator extends ForTranslator { private IteratorForTranslator(@NotNull JetForExpression forExpression, @NotNull TranslationContext context) { super(forExpression, context); - iterator = context.dynamicContext().createTemporary(iteratorMethodInvocation()); + iteratorVar = context.declareTemporary(iteratorMethodInvocation()); } @NotNull - private JsBlock translate() { - return new JsBlock(new JsVars(iterator.first), new JsWhile(hasNextMethodInvocation(), translateBody(nextMethodInvocation()))); + private JsStatement translate() { + context().addStatementToCurrentBlock(iteratorVar.assignmentExpression().makeStmt()); + return new JsWhile(hasNextMethodInvocation(), translateBody(nextMethodInvocation())); } @NotNull private JsExpression nextMethodInvocation() { - return translateMethodInvocation(iterator.second, getNextFunction(bindingContext(), getLoopRange(expression))); + return translateMethodInvocation(iteratorVar.reference(), getNextFunction(bindingContext(), getLoopRange(expression))); } @NotNull private JsExpression hasNextMethodInvocation() { ResolvedCall resolvedCall = getHasNextCallable(bindingContext(), getLoopRange(expression)); - return translateMethodInvocation(iterator.second, resolvedCall); + return translateMethodInvocation(iteratorVar.reference(), resolvedCall); } @NotNull diff --git a/js/js.translator/src/org/jetbrains/k2js/translate/expression/foreach/RangeForTranslator.java b/js/js.translator/src/org/jetbrains/k2js/translate/expression/foreach/RangeForTranslator.java index 19f84fe055d..1a9aa78ffc6 100644 --- a/js/js.translator/src/org/jetbrains/k2js/translate/expression/foreach/RangeForTranslator.java +++ b/js/js.translator/src/org/jetbrains/k2js/translate/expression/foreach/RangeForTranslator.java @@ -16,7 +16,6 @@ package org.jetbrains.k2js.translate.expression.foreach; -import com.google.common.collect.Lists; import com.google.dart.compiler.backend.js.ast.*; import org.jetbrains.annotations.NotNull; import org.jetbrains.jet.lang.psi.JetExpression; @@ -27,8 +26,6 @@ import org.jetbrains.k2js.translate.context.TranslationContext; import org.jetbrains.k2js.translate.general.Translation; import org.jetbrains.k2js.translate.utils.BindingUtils; -import java.util.List; - import static org.jetbrains.jet.lang.resolve.DescriptorUtils.getClassDescriptorForType; import static org.jetbrains.k2js.translate.utils.JsAstUtils.*; import static org.jetbrains.k2js.translate.utils.PsiUtils.getLoopRange; @@ -69,11 +66,9 @@ public final class RangeForTranslator extends ForTranslator { } @NotNull - private JsBlock translate() { - List blockStatements = Lists.newArrayList(); - blockStatements.add(temporariesInitialization(rangeExpression, start, end, increment).makeStmt()); - blockStatements.add(generateForExpression()); - return new JsBlock(blockStatements); + private JsStatement translate() { + context().addStatementToCurrentBlock(temporariesInitialization(rangeExpression, start, end, increment).makeStmt()); + return generateForExpression(); } @NotNull diff --git a/js/js.translator/src/org/jetbrains/k2js/translate/expression/foreach/RangeLiteralForTranslator.java b/js/js.translator/src/org/jetbrains/k2js/translate/expression/foreach/RangeLiteralForTranslator.java index 2706d5f416f..333862466cd 100644 --- a/js/js.translator/src/org/jetbrains/k2js/translate/expression/foreach/RangeLiteralForTranslator.java +++ b/js/js.translator/src/org/jetbrains/k2js/translate/expression/foreach/RangeLiteralForTranslator.java @@ -16,7 +16,6 @@ package org.jetbrains.k2js.translate.expression.foreach; -import com.google.common.collect.Lists; import com.google.dart.compiler.backend.js.ast.*; import org.jetbrains.annotations.NotNull; import org.jetbrains.jet.lang.psi.JetBinaryExpression; @@ -25,10 +24,9 @@ import org.jetbrains.jet.lang.psi.JetForExpression; import org.jetbrains.jet.lexer.JetTokens; import org.jetbrains.k2js.translate.context.TemporaryVariable; import org.jetbrains.k2js.translate.context.TranslationContext; +import org.jetbrains.k2js.translate.utils.TranslationUtils; -import java.util.List; - -import static org.jetbrains.k2js.translate.utils.JsAstUtils.inequality; +import static org.jetbrains.k2js.translate.utils.JsAstUtils.lessThanEq; import static org.jetbrains.k2js.translate.utils.JsAstUtils.newVar; import static org.jetbrains.k2js.translate.utils.PsiUtils.getLoopRange; import static org.jetbrains.k2js.translate.utils.TemporariesUtils.temporariesInitialization; @@ -66,22 +64,25 @@ public final class RangeLiteralForTranslator extends ForTranslator { JetExpression loopRange = getLoopRange(expression); assert loopRange instanceof JetBinaryExpression; JetBinaryExpression loopRangeAsBinary = ((JetBinaryExpression) loopRange); - rangeStart = translateLeftExpression(context, loopRangeAsBinary); - rangeEnd = context.declareTemporary(getRangeEnd(loopRangeAsBinary)); + JsBlock startBlock = new JsBlock(); + JsExpression rangeStartExpression = translateLeftExpression(context, loopRangeAsBinary, startBlock); + JsBlock endBlock = new JsBlock(); + JsExpression rightExpression = translateRightExpression(context(), loopRangeAsBinary, endBlock); + if (TranslationUtils.isCacheNeeded(rangeStartExpression)) { + TemporaryVariable startVar = context.declareTemporary(rangeStartExpression); + rangeStartExpression = startVar.reference(); + context.addStatementToCurrentBlock(startVar.assignmentExpression().makeStmt()); + } + rangeStart = rangeStartExpression; + context.addStatementsToCurrentBlockFrom(startBlock); + context.addStatementsToCurrentBlockFrom(endBlock); + rangeEnd = context.declareTemporary(rightExpression); } @NotNull - private JsExpression getRangeEnd(@NotNull JetBinaryExpression loopRangeAsBinary) { - JsExpression rightExpression = translateRightExpression(context(), loopRangeAsBinary); - return new JsBinaryOperation(JsBinaryOperator.ADD, rightExpression, program().getNumberLiteral(1)); - } - - @NotNull - private JsBlock translate() { - List blockStatements = Lists.newArrayList(); - blockStatements.add(temporariesInitialization(rangeEnd).makeStmt()); - blockStatements.add(new JsFor(initExpression(), getCondition(), getIncrExpression(), translateBody(null))); - return new JsBlock(blockStatements); + private JsStatement translate() { + context().addStatementToCurrentBlock(temporariesInitialization(rangeEnd).makeStmt()); + return new JsFor(initExpression(), getCondition(), getIncrExpression(), translateBody(null)); } @NotNull @@ -91,7 +92,7 @@ public final class RangeLiteralForTranslator extends ForTranslator { @NotNull private JsExpression getCondition() { - return inequality(parameterName.makeRef(), rangeEnd.reference()); + return lessThanEq(parameterName.makeRef(), rangeEnd.reference()); } @NotNull diff --git a/js/js.translator/testData/expression/for/cases/forIteratesOverArray.kt b/js/js.translator/testData/expression/for/cases/forIteratesOverArray.kt index b1e389776ea..7d711edeb66 100644 --- a/js/js.translator/testData/expression/for/cases/forIteratesOverArray.kt +++ b/js/js.translator/testData/expression/for/cases/forIteratesOverArray.kt @@ -16,5 +16,13 @@ fun box(): Boolean { d += 1 } } - return (c == 2) && (d == 8) + assertEquals(2, c) + assertEquals(8, d) + + var s: String = "" + for(i in array(0,1,2)) + try { s += "A${i}:"} finally {} + assertEquals("A0:A1:A2:", s) + + return true } \ No newline at end of file diff --git a/js/js.translator/testData/expression/for/cases/forIteratesOverLiteralRange.kt b/js/js.translator/testData/expression/for/cases/forIteratesOverLiteralRange.kt new file mode 100644 index 00000000000..4d1d2e2969e --- /dev/null +++ b/js/js.translator/testData/expression/for/cases/forIteratesOverLiteralRange.kt @@ -0,0 +1,44 @@ +package foo + +var global: String = "" + +fun up(s: String, value: Int): Int { + global += s + return value +} + +fun box(): String { + + // http://youtrack.jetbrains.com/issue/KT-5262 + // JS: wrong code for `for` over range when start greater end + var n: Int = 0 + for(i in 3..0) + n++ + assertEquals(0, n) + + for(i in 0..3) + n++ + assertEquals(4, n) + + // Evaluation order + for (i in up("A", 0)..up("B", 5)) { + } + assertEquals("AB", global) + + global = "" + for (i in try { up("A", 0) } finally {} ..up("B", 5)) { + } + assertEquals("AB", global) + + global = "" + for (i in up("A", 0).. try { up("B", 5)} finally {}) { + } + assertEquals("AB", global) + + global = "" + for (i in try { up("A", 0) } finally {}.. try { up("B", 5)} finally {}) { + } + assertEquals("AB", global) + + return "OK" +} \ No newline at end of file diff --git a/js/js.translator/testData/expression/for/cases/forIteratesOverNonLiteralRange.kt b/js/js.translator/testData/expression/for/cases/forIteratesOverNonLiteralRange.kt new file mode 100644 index 00000000000..b17bc0ccbf1 --- /dev/null +++ b/js/js.translator/testData/expression/for/cases/forIteratesOverNonLiteralRange.kt @@ -0,0 +1,16 @@ +package foo + +fun box(): String { + var n = 0 + var r = 3..0 + for(i in r) + n++ + assertEquals(0, n) + + r = 0..3 + for(i in r) + n++ + assertEquals(4, n) + + return "OK" +} \ No newline at end of file diff --git a/js/js.translator/testData/expression/for/cases/forIteratesOverSomethingWithIterator.kt b/js/js.translator/testData/expression/for/cases/forIteratesOverSomethingWithIterator.kt new file mode 100644 index 00000000000..45f49493790 --- /dev/null +++ b/js/js.translator/testData/expression/for/cases/forIteratesOverSomethingWithIterator.kt @@ -0,0 +1,21 @@ +package foo + +class Iter(val upper: Int) { + var count: Int = 0 + fun hasNext(): Boolean = count < upper + fun next(): Int = count++ +} + +class A(val upper: Int) { + fun iterator(): Iter = Iter(upper) +} + +fun box(): String { + var n = 0 + for(i in A(10)) { + n++ + } + assertEquals(10, n) + + return "OK" +} \ No newline at end of file diff --git a/js/js.translator/testData/expression/for/cases/forWithWhile.kt b/js/js.translator/testData/expression/for/cases/forWithWhile.kt new file mode 100644 index 00000000000..38d53130a53 --- /dev/null +++ b/js/js.translator/testData/expression/for/cases/forWithWhile.kt @@ -0,0 +1,49 @@ +package foo + +fun box(): String { + var n = 0 + @forLabel0 for(i in 0..10) { + var j = 0 + @whileLabel0 while(j++