Fixed a problem which sometimes caused expressions to be evaluated in the wrong order.

2012-03-27 17:16:06 +04:00
parent 25323d560d
commit 1e180f550f
17 changed files with 611 additions and 5 deletions
@@ -0,0 +1,53 @@
+/*
+ * Copyright 2010-2012 JetBrains s.r.o.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.jetbrains.k2js.test.semantics;
+
+import org.jetbrains.k2js.test.SingleFileTranslationTest;
+
+/**
+ * @author Pavel Talanov
+ */
+public final class DangerousTest extends SingleFileTranslationTest {
+
+    public DangerousTest() {
+        super("dangerous/");
+    }
+
+    public void testIfAsFunArgument() throws Exception {
+        checkFooBoxIsTrue("ifAsFunArgument.kt");
+    }
+
+    public void testIfAsPlusArgument() throws Exception {
+        checkFooBoxIsTrue("ifAsPlusArgument.kt");
+    }
+
+    public void testWhenAsMinusArgument() throws Exception {
+        checkFooBoxIsTrue("whenAsMinusArgument.kt");
+    }
+
+    public void testEvaluationOrder() throws Exception {
+        checkFooBoxIsTrue("evaluationOrder.kt");
+    }
+
+    public void testDangerousInsideDangerous() throws Exception {
+        checkFooBoxIsTrue("dangerousInsideDangerous.kt");
+    }
+
+    public void test2dangerousInExpression() throws Exception {
+        checkFooBoxIsTrue("2dangerousInExpression.kt");
+    }
+}
@@ -52,7 +52,8 @@ public final class MiscTest extends AbstractExpressionTest {
        try {
            checkFooBoxIsTrue("ifAsExpressionWithThrow.kt");
            fail();
-        } catch (JavaScriptException e) {
+        }
+        catch (JavaScriptException e) {
        }
    }

@@ -21,6 +21,7 @@ import com.google.dart.compiler.backend.js.ast.JsName;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 import org.jetbrains.jet.lang.descriptors.DeclarationDescriptor;
+import org.jetbrains.jet.lang.psi.JetExpression;

 import java.util.Map;

@@ -39,6 +40,11 @@ public class AliasingContext {
        public JsName getAliasForDescriptor(@NotNull DeclarationDescriptor descriptor) {
            return null;
        }
+
+        @Override
+        public JsName getAliasForExpression(@NotNull JetExpression element) {
+            return null;
+        }
    };

    public static AliasingContext getCleanContext() {
@@ -49,6 +55,8 @@ public class AliasingContext {
    private final Map<DeclarationDescriptor, JsName> aliasesForDescriptors = Maps.newHashMap();
    @NotNull
    private final Map<DeclarationDescriptor, JsName> aliasesForThis = Maps.newHashMap();
+    @NotNull
+    private final Map<JetExpression, JsName> aliasesForExpressions = Maps.newHashMap();

    @Nullable
    private final AliasingContext parent;
@@ -60,18 +68,23 @@ public class AliasingContext {
    @NotNull
    public AliasingContext withThisAliased(@NotNull DeclarationDescriptor correspondingDescriptor, @NotNull JsName alias) {
        AliasingContext newContext = new AliasingContext(this);
-        assert !newContext.aliasesForThis.containsKey(correspondingDescriptor);
        newContext.aliasesForThis.put(correspondingDescriptor, alias);
        return newContext;
    }

+    @NotNull
+    public AliasingContext withAliasesForExpressions(@NotNull Map<JetExpression, JsName> aliasesForExpressions) {
+        AliasingContext newContext = new AliasingContext(this);
+        newContext.aliasesForExpressions.putAll(aliasesForExpressions);
+        return newContext;
+    }
+
    @NotNull
    private AliasingContext getParent() {
        assert parent != null;
        return parent;
    }

-
    @Nullable
    public JsName getAliasForThis(@NotNull DeclarationDescriptor descriptor) {
        JsName alias = aliasesForThis.get(descriptor.getOriginal());
@@ -89,4 +102,13 @@ public class AliasingContext {
        }
        return getParent().getAliasForDescriptor(descriptor);
    }
+
+    @Nullable
+    public JsName getAliasForExpression(@NotNull JetExpression element) {
+        JsName alias = aliasesForExpressions.get(element);
+        if (alias != null) {
+            return alias;
+        }
+        return getParent().getAliasForExpression(element);
+    }
 }
@@ -22,7 +22,8 @@ import org.jetbrains.annotations.NotNull;
 import static org.jetbrains.k2js.translate.utils.JsAstUtils.addVarDeclaration;
 import static org.jetbrains.k2js.translate.utils.JsAstUtils.newVar;

-public class DynamicContext {
+//TODO: consider renaming to scoping context
+public final class DynamicContext {

    @NotNull
    public static DynamicContext rootContext(@NotNull NamingScope rootScope, @NotNull JsBlock globalBlock) {
@@ -22,9 +22,12 @@ import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 import org.jetbrains.jet.lang.descriptors.CallableDescriptor;
 import org.jetbrains.jet.lang.descriptors.DeclarationDescriptor;
+import org.jetbrains.jet.lang.psi.JetExpression;
 import org.jetbrains.jet.lang.resolve.BindingContext;
 import org.jetbrains.k2js.translate.intrinsic.Intrinsics;

+import java.util.Map;
+
 import static org.jetbrains.k2js.translate.utils.BindingUtils.getDescriptorForElement;

 /**
@@ -85,6 +88,11 @@ public final class TranslationContext {
        return new TranslationContext(staticContext, dynamicContext, aliasingContext.withThisAliased(correspondingDescriptor, alias));
    }

+    @NotNull
+    public TranslationContext innerContextWithAliasesForExpressions(@NotNull Map<JetExpression, JsName> aliases) {
+        return new TranslationContext(staticContext, dynamicContext, aliasingContext.withAliasesForExpressions(aliases));
+    }
+
    @NotNull
    public JsBlock getBlockForDescriptor(@NotNull DeclarationDescriptor descriptor) {
        if (descriptor instanceof CallableDescriptor) {
@@ -34,12 +34,15 @@ import org.jetbrains.k2js.translate.expression.PatternTranslator;
 import org.jetbrains.k2js.translate.expression.WhenTranslator;
 import org.jetbrains.k2js.translate.initializer.ClassInitializerTranslator;
 import org.jetbrains.k2js.translate.initializer.NamespaceInitializerTranslator;
+import org.jetbrains.k2js.translate.utils.dangerous.DangerousData;
+import org.jetbrains.k2js.translate.utils.dangerous.DangerousTranslator;

 import java.util.List;
 import java.util.Map;

 import static org.jetbrains.k2js.translate.utils.JsAstUtils.convertToExpression;
 import static org.jetbrains.k2js.translate.utils.JsAstUtils.convertToStatement;
+import static org.jetbrains.k2js.translate.utils.dangerous.DangerousData.collect;

 /**
 * @author Pavel Talanov
@@ -77,6 +80,20 @@ public final class Translation {

    @NotNull
    public static JsNode translateExpression(@NotNull JetExpression expression, @NotNull TranslationContext context) {
+        JsName aliasForExpression = context.aliasingContext().getAliasForExpression(expression);
+        if (aliasForExpression != null) {
+            return aliasForExpression.makeRef();
+        }
+        DangerousData data = collect(expression, context);
+        if (data.shouldBeTranslated()) {
+            return DangerousTranslator.translate(data, context);
+        }
+        return doTranslateExpression(expression, context);
+    }
+
+    //NOTE: use with care
+    @NotNull
+    public static JsNode doTranslateExpression(JetExpression expression, TranslationContext context) {
        return expression.accept(new ExpressionVisitor(), context);
    }

@@ -139,7 +139,6 @@ public final class BindingUtils {
        Boolean isStatement = context.get(BindingContext.STATEMENT, expression);
        assert isStatement != null : "Invalid behaviour of get(BindingContext.STATEMENT)";
        return isStatement;
-        // return IsStatement.isStatement(expression);
    }

    @NotNull
@@ -0,0 +1,137 @@
+/*
+ * Copyright 2010-2012 JetBrains s.r.o.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.jetbrains.k2js.translate.utils.dangerous;
+
+import com.google.common.collect.Lists;
+import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
+import org.jetbrains.jet.lang.psi.JetBlockExpression;
+import org.jetbrains.jet.lang.psi.JetElement;
+import org.jetbrains.jet.lang.psi.JetExpression;
+import org.jetbrains.k2js.translate.context.TranslationContext;
+
+import java.util.List;
+
+/**
+ * @author Pavel Talanov
+ *         <p/>
+ *         This module uses a methaphor for naming.
+ *         <p/>
+ *         Dangerous are the nodes that can be expressions in Kotlin but can't be expressions in JavaScript.
+ *         These are: when, if, inlined functions.
+ *         The issue with them is that we have to translate them to a list of statements. And also all the expressions which must be computed before
+ *         the dangerous expressions.
+ *         RootNode is a node which contains such an expression. For example, it may be a statement expression belongs to.
+ */
+public class DangerousData {
+
+    @NotNull
+    public static DangerousData collect(@NotNull JetExpression expression, @NotNull TranslationContext context) {
+        if (cantContainDangerousElements(expression, context)) {
+            return emptyData();
+        }
+        return doCollectData(expression, context);
+    }
+
+    private static boolean cantContainDangerousElements(@NotNull JetElement element, @NotNull TranslationContext context) {
+        if (element instanceof JetBlockExpression) {
+            return true;
+        }
+        return false;
+    }
+
+    @NotNull
+    private static DangerousData doCollectData(@NotNull JetExpression expression,
+                                               @NotNull TranslationContext context) {
+        DangerousData data = new DangerousData();
+        FindDangerousVisitor visitor = new FindDangerousVisitor(context.bindingContext());
+        expression.accept(visitor, data);
+        if (!data.exists()) {
+            return emptyData();
+        }
+        data.setRootNode(expression);
+        FindPreviousVisitor findPreviousVisitor = new FindPreviousVisitor();
+        expression.accept(findPreviousVisitor, data);
+        return data;
+    }
+
+    private static final DangerousData EMPTY = new DangerousData() {
+        @Override
+        public boolean exists() {
+            return false;
+        }
+
+        @Override
+        public boolean shouldBeTranslated() {
+            return false;
+        }
+    };
+
+    @NotNull
+    public static DangerousData emptyData() {
+        return EMPTY;
+    }
+
+    @NotNull
+    private List<JetExpression> nodesToBeGeneratedBefore = Lists.newArrayList();
+
+    @Nullable
+    private JetExpression dangerousNode = null;
+
+    @Nullable
+    private JetExpression rootNode = null;
+
+    public void setDangerousNode(@NotNull JetExpression dangerousNode) {
+        assert this.dangerousNode == null : "Should be assigned only once";
+        this.dangerousNode = dangerousNode;
+    }
+
+    @NotNull
+    public List<JetExpression> getNodesToBeGeneratedBefore() {
+        return nodesToBeGeneratedBefore;
+    }
+
+    @SuppressWarnings("NullableProblems")
+    public void setNodesToBeGeneratedBefore(@NotNull List<JetExpression> nodesToBeGeneratedBefore) {
+        this.nodesToBeGeneratedBefore = nodesToBeGeneratedBefore;
+    }
+
+    public boolean exists() {
+        return dangerousNode != null;
+    }
+
+    public boolean shouldBeTranslated() {
+        return exists() && !nodesToBeGeneratedBefore.isEmpty();
+    }
+
+    @NotNull
+    public JetExpression getDangerousNode() {
+        assert dangerousNode != null;
+        return dangerousNode;
+    }
+
+    @NotNull
+    public JetExpression getRootNode() {
+        assert rootNode != null;
+        return rootNode;
+    }
+
+    @SuppressWarnings("NullableProblems")
+    public void setRootNode(@NotNull JetExpression rootNode) {
+        this.rootNode = rootNode;
+    }
+}
@@ -0,0 +1,73 @@
+/*
+ * Copyright 2010-2012 JetBrains s.r.o.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.jetbrains.k2js.translate.utils.dangerous;
+
+import com.google.common.collect.Maps;
+import com.google.dart.compiler.backend.js.ast.JsExpression;
+import com.google.dart.compiler.backend.js.ast.JsName;
+import com.google.dart.compiler.backend.js.ast.JsNode;
+import org.jetbrains.annotations.NotNull;
+import org.jetbrains.jet.lang.psi.JetExpression;
+import org.jetbrains.k2js.translate.context.TemporaryVariable;
+import org.jetbrains.k2js.translate.context.TranslationContext;
+import org.jetbrains.k2js.translate.general.AbstractTranslator;
+import org.jetbrains.k2js.translate.general.Translation;
+
+import java.util.List;
+import java.util.Map;
+
+/**
+ * @author Pavel Talanov
+ */
+public final class DangerousTranslator extends AbstractTranslator {
+
+
+    @NotNull
+    public static JsNode translate(@NotNull DangerousData data, @NotNull TranslationContext context) {
+        assert data.exists();
+        DangerousTranslator translator = new DangerousTranslator(data, context);
+        return translator.translate();
+    }
+
+    @NotNull
+    private final DangerousData data;
+
+    private DangerousTranslator(@NotNull DangerousData data, @NotNull TranslationContext context) {
+        super(context);
+        this.data = data;
+    }
+
+    @NotNull
+    private JsNode translate() {
+        Map<JetExpression, JsName> aliasesForExpressions =
+            translateAllExpressionsAndCreateAliasesForThem(data.getNodesToBeGeneratedBefore());
+        TranslationContext contextWithAliases = context().innerContextWithAliasesForExpressions(aliasesForExpressions);
+        return Translation.doTranslateExpression(data.getRootNode(), contextWithAliases);
+    }
+
+    @NotNull
+    private Map<JetExpression, JsName> translateAllExpressionsAndCreateAliasesForThem(@NotNull List<JetExpression> expressions) {
+        Map<JetExpression, JsName> aliasesForExpressions = Maps.newHashMap();
+        for (JetExpression expression : expressions) {
+            JsExpression translatedExpression = Translation.translateAsExpression(expression, context());
+            TemporaryVariable aliasForExpression = context().declareTemporary(translatedExpression);
+            context().addStatementToCurrentBlock(aliasForExpression.assignmentExpression().makeStmt());
+            aliasesForExpressions.put(expression, aliasForExpression.name());
+        }
+        return aliasesForExpressions;
+    }
+}
@@ -0,0 +1,86 @@
+/*
+ * Copyright 2010-2012 JetBrains s.r.o.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.jetbrains.k2js.translate.utils.dangerous;
+
+import org.jetbrains.annotations.NotNull;
+import org.jetbrains.jet.lang.psi.*;
+import org.jetbrains.jet.lang.resolve.BindingContext;
+
+import static org.jetbrains.k2js.translate.utils.BindingUtils.isStatement;
+
+/**
+ * @author Pavel Talanov
+ */
+public final class FindDangerousVisitor extends JetTreeVisitor<DangerousData> {
+
+    @NotNull
+    private final BindingContext bindingContext;
+
+    public FindDangerousVisitor(@NotNull BindingContext context) {
+        bindingContext = context;
+    }
+
+    @Override
+    public Void visitDeclaration(JetDeclaration dcl, DangerousData data) {
+        return null;
+    }
+
+    @Override
+    public Void visitJetElement(JetElement element, DangerousData data) {
+        if (data.exists()) {
+            return null;
+        }
+        return super.visitJetElement(element, data);
+    }
+
+    @Override
+    public Void visitWhenExpression(JetWhenExpression expression, DangerousData data) {
+        if (expressionFound(expression, data)) {
+            return null;
+        }
+        return super.visitWhenExpression(expression, data);
+    }
+
+    @Override
+    public Void visitIfExpression(JetIfExpression expression, DangerousData data) {
+        if (expressionFound(expression, data)) {
+            return null;
+        }
+        return super.visitIfExpression(expression, data);
+    }
+
+    @Override
+    public Void visitBlockExpression(JetBlockExpression expression, DangerousData data) {
+        if (isStatement(bindingContext, expression)) {
+            return null;
+        }
+        else {
+            return super.visitBlockExpression(expression, data);
+        }
+    }
+
+    private boolean expressionFound(@NotNull JetExpression expression, @NotNull DangerousData data) {
+        if (data.exists()) {
+            return true;
+        }
+        if (!isStatement(bindingContext, expression)) {
+            data.setDangerousNode(expression);
+            return true;
+        }
+        return false;
+    }
+}
@@ -0,0 +1,108 @@
+/*
+ * Copyright 2010-2012 JetBrains s.r.o.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.jetbrains.k2js.translate.utils.dangerous;
+
+import com.intellij.psi.PsiElement;
+import org.jetbrains.annotations.NotNull;
+import org.jetbrains.jet.lang.psi.*;
+
+/**
+ * @author Pavel Talanov
+ */
+public final class FindPreviousVisitor extends JetTreeVisitor<DangerousData> {
+
+    @Override
+    public Void visitJetElement(JetElement element, DangerousData data) {
+        if (data.getDangerousNode() == element) {
+            return null;
+        }
+        if (!hasDangerous(element, data)) {
+            addElement(element, data);
+        }
+        else {
+            acceptChildrenThatAreBeforeTheDangerousNode(element, data);
+        }
+        return null;
+    }
+
+    private static boolean addElement(@NotNull JetElement element, @NotNull DangerousData data) {
+        if (element instanceof JetExpression) {
+            data.getNodesToBeGeneratedBefore().add((JetExpression)element);
+            return true;
+        }
+        return false;
+    }
+
+    private void acceptChildrenThatAreBeforeTheDangerousNode(@NotNull JetElement element, @NotNull DangerousData data) {
+        PsiElement current = element.getFirstChild();
+        while (current != null) {
+            if (current instanceof JetElement) {
+                ((JetElement)current).accept(this, data);
+                if (hasDangerous(element, data)) {
+                    break;
+                }
+            }
+            current = current.getNextSibling();
+        }
+    }
+
+    @Override
+    public Void visitCallExpression(@NotNull JetCallExpression expression, @NotNull DangerousData data) {
+        if (data.getDangerousNode() == expression) {
+            return null;
+        }
+        if (!hasDangerous(expression, data)) {
+            data.getNodesToBeGeneratedBefore().add(expression);
+        }
+        else {
+            acceptArgumentsThatAreBeforeDangerousNode(expression, data);
+        }
+        return null;
+    }
+
+    private void acceptArgumentsThatAreBeforeDangerousNode(@NotNull JetCallExpression expression, @NotNull DangerousData data) {
+        for (ValueArgument argument : expression.getValueArguments()) {
+            JetExpression argumentExpression = argument.getArgumentExpression();
+            assert argumentExpression != null;
+            argumentExpression.accept(this, data);
+            if (hasDangerous(argumentExpression, data)) {
+                break;
+            }
+        }
+    }
+
+    private static boolean hasDangerous(@NotNull JetElement element, @NotNull DangerousData data) {
+        HasDangerousVisitor visitor = new HasDangerousVisitor();
+        element.accept(visitor, data);
+        return visitor.hasDangerous;
+    }
+
+    private static final class HasDangerousVisitor extends JetTreeVisitor<DangerousData> {
+
+        private boolean hasDangerous = false;
+
+        @Override
+        public Void visitJetElement(JetElement element, DangerousData data) {
+            if (element == data.getDangerousNode()) {
+                hasDangerous = true;
+                return null;
+            }
+            element.acceptChildren(this, data);
+            return null;
+        }
+    }
+}
@@ -0,0 +1,26 @@
+package foo
+
+fun box() : Boolean {
+  if (f(0) != -3) {
+      return false
+  }
+  if (f(102) != 201) {
+      return false;
+  }
+  if (f(103) != 100) {
+      return false
+  }
+  if (f(-100) != -100) {
+      return false
+  }
+  if (f(-99) != -201) {
+      return false
+  }
+
+  return true
+}
+
+fun f(i : Int) : Int {
+  var j = i
+  return --j + (if (j < -100) return -100 else --j) + (if (j > 100) return 100 else 0)
+}
@@ -0,0 +1,19 @@
+package foo
+
+fun box() : Boolean {
+  if (f(0) != 201) {
+      return false
+  }
+  if (f(1) != 104) {
+        return false
+  }
+  if (f(-2) != -100) {
+      return false
+  }
+  return true
+}
+
+fun f(i : Int) : Int {
+  var j = i
+  return ++j + if (j != 1) {(if (j > 0) 100 else return -100) + 2} else 200
+}
@@ -0,0 +1,17 @@
+package foo
+
+var d = 0
+
+fun f() : Int {
+  d = if (d < 0) -100 else 100
+  return d
+}
+
+fun box() : Boolean {
+   d = d-- + f() + when(d) {
+   -100 -> return true
+   1 -> 1
+   else -> return false
+  }
+  return false
+}
@@ -0,0 +1,14 @@
+package foo
+
+fun box() : Boolean {
+  var i = 0
+  val c = sum(++i, if (i == 0) return false else i + 2)
+  if (c != 4) {
+    return false
+  }
+  return true
+}
+
+
+
+fun sum(a1 : Int, a2 : Int) = a1 + a2
@@ -0,0 +1,10 @@
+package foo
+
+fun box() : Boolean {
+  var  i = 0
+  var t = ++i + if (i == 0) 0 else 2
+  if (t != 3) {
+    return false
+  }
+  return true
+}
@@ -0,0 +1,15 @@
+package foo
+
+fun box() : Boolean {
+  var  i = 0
+  var t = ++i + when(i) {
+    3 -> 4
+    1 -> 2
+    0 -> 1
+    else -> 100
+  }
+  if (t != 3) {
+    return false
+  }
+  return true
+}