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

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();
+    }
 }

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<JsVar, JsExpression> 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;

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() {
+    private JsStatement translate() {
-        List<JsStatement> blockStatements = Lists.newArrayList();
+        context().addStatementToCurrentBlock(temporariesInitialization(loopRange, end).makeStmt());
-        blockStatements.add(temporariesInitialization(loopRange, end).makeStmt());
+        return new JsFor(getInitExpression(), getCondition(), getIncrementExpression(), getBody());
-        blockStatements.add(new JsFor(getInitExpression(), getCondition(), getIncrementExpression(), getBody()));
-        return new JsBlock(blockStatements);
     }
 
 

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<JsVars.JsVar, JsExpression> 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() {
+    private JsStatement translate() {
-        return new JsBlock(new JsVars(iterator.first), new JsWhile(hasNextMethodInvocation(), translateBody(nextMethodInvocation())));
+        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<FunctionDescriptor> resolvedCall = getHasNextCallable(bindingContext(), getLoopRange(expression));
-        return translateMethodInvocation(iterator.second, resolvedCall);
+        return translateMethodInvocation(iteratorVar.reference(), resolvedCall);
     }
 
     @NotNull

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() {
+    private JsStatement translate() {
-        List<JsStatement> blockStatements = Lists.newArrayList();
+        context().addStatementToCurrentBlock(temporariesInitialization(rangeExpression, start, end, increment).makeStmt());
-        blockStatements.add(temporariesInitialization(rangeExpression, start, end, increment).makeStmt());
+        return generateForExpression();
-        blockStatements.add(generateForExpression());
-        return new JsBlock(blockStatements);
     }
 
     @NotNull

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.lessThanEq;
-
-import static org.jetbrains.k2js.translate.utils.JsAstUtils.inequality;
 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);
+        JsBlock startBlock = new JsBlock();
-        rangeEnd = context.declareTemporary(getRangeEnd(loopRangeAsBinary));
+        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) {
+    private JsStatement translate() {
-        JsExpression rightExpression = translateRightExpression(context(), loopRangeAsBinary);
+        context().addStatementToCurrentBlock(temporariesInitialization(rangeEnd).makeStmt());
-        return new JsBinaryOperation(JsBinaryOperator.ADD, rightExpression, program().getNumberLiteral(1));
+        return new JsFor(initExpression(), getCondition(), getIncrExpression(), translateBody(null));
-    }
-
-    @NotNull
-    private JsBlock translate() {
-        List<JsStatement> blockStatements = Lists.newArrayList();
-        blockStatements.add(temporariesInitialization(rangeEnd).makeStmt());
-        blockStatements.add(new JsFor(initExpression(), getCondition(), getIncrExpression(), translateBody(null)));
-        return new JsBlock(blockStatements);
     }
 
     @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

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
 }

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"
+}

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"
+}

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"
+}

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++<i) {
+            n++;
+            if (j==2)
+                continue@forLabel0
+        }
+    }
+    assertEquals(19, n)
+
+    n = 0
+    @forLabel1 for(i in 0..10) {
+        var j = 0
+        @whileLabel1 while(try {j++} finally {} <i) {
+            n++;
+            if (j==2)
+                continue@forLabel1
+        }
+    }
+    assertEquals(19, n)
+
+    n = 0
+    @forLabel2 for(i in 0..10) {
+        var j = 0
+        @whileLabel2 while(j++<i) {
+            n++;
+            if (j==2)
+                break@forLabel2
+        }
+    }
+    assertEquals(3, n)
+
+    n = 0
+    @forLabel3 for(i in 0..10) {
+        var j = 0
+        @whileLabel3 while(j++<i) {
+            n++;
+            if (j==2)
+                break@whileLabel3
+        }
+    }
+    assertEquals(19, n)
+
+    return "OK"
+}

js/js.translator/testData/expression/for/cases/labeledForWithContinue.kt

@@ -0,0 +1,36 @@
+// http://youtrack.jetbrains.com/issue/KT-5257
+// JS: for with continue with label fails on runtime
+
+package foo
+
+var global: String = ""
+
+fun up(s: String, value: Int): Int {
+    global += s
+    return value
+}
+
+fun box(): String {
+
+    @list
+    for (i in 0..up("A", 5)) {
+        continue @list
+    }
+    assertEquals("A", global)
+
+    global = ""
+    @list1
+    for (i in up("A", 0)..try { global += "B"; 5} finally {}) {
+        continue @list1
+    }
+    assertEquals("AB", global)
+
+    global = ""
+    @list2
+    for (i in try { up("A", 0) } finally {}..try { global += "B"; 5} finally {}) {
+        continue @list2
+    }
+    assertEquals("AB", global)
+
+    return "OK"
+}