JS: fix translation of augmented assignment when RHS changes value of LHS

Alexey Andreev · Alexey Andreev · commit 40e00a62f593 · 2016-11-23T12:19:03.000+03:00
diff --git a/compiler/testData/codegen/box/coroutines/controlFlow/switchLikeWhen.kt b/compiler/testData/codegen/box/coroutines/controlFlow/switchLikeWhen.kt
@@ -1,9 +1,6 @@
 // WITH_RUNTIME
 // NO_INTERCEPT_RESUME_TESTS
 
-// TODO: fix bug in JVM backend and remove this directive
-// TARGET_BACKEND: JS
-
 class Controller {
     var result = ""
 
@@ -29,7 +26,7 @@ fun box(): String {
             }
         }
     }
-    if (value != "A;[B][C]!") return "fail: suspend as if condition: $value"
+    if (value != "A;B]C]!") return "fail: suspend as if condition: $value"
 
     return "OK"
 }
diff --git a/compiler/testData/codegen/box/increment/augmentedAssignmentWithComplexRhs.kt b/compiler/testData/codegen/box/increment/augmentedAssignmentWithComplexRhs.kt
@@ -0,0 +1,23 @@
+// WITH_RUNTIME
+import kotlin.test.*
+
+var log = ""
+var result = 20
+
+fun <T> id(value: T) = value
+
+fun box(): String {
+    result += if (id("true") == "true") {
+        result += 10
+        log += "true chosen"
+        3
+    }
+    else {
+        4
+    }
+
+    assertEquals(23, result)
+    assertEquals("true chosen", log)
+
+    return "OK"
+}
diff --git a/compiler/tests-ir-jvm/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java b/compiler/tests-ir-jvm/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java
@@ -4768,6 +4768,12 @@ public void testIfStatement() throws Exception {
                 doTest(fileName);
             }
 
+            @TestMetadata("switchLikeWhen.kt")
+            public void testSwitchLikeWhen() throws Exception {
+                String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/coroutines/controlFlow/switchLikeWhen.kt");
+                doTest(fileName);
+            }
+
             @TestMetadata("throwFromCatch.kt")
             public void testThrowFromCatch() throws Exception {
                 String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/coroutines/controlFlow/throwFromCatch.kt");
@@ -7679,6 +7685,12 @@ public void testAssignPlusOnSmartCast() throws Exception {
             doTest(fileName);
         }
 
+        @TestMetadata("augmentedAssignmentWithComplexRhs.kt")
+        public void testAugmentedAssignmentWithComplexRhs() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/increment/augmentedAssignmentWithComplexRhs.kt");
+            doTest(fileName);
+        }
+
         @TestMetadata("classNaryGetSet.kt")
         public void testClassNaryGetSet() throws Exception {
             String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/increment/classNaryGetSet.kt");
diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java
@@ -4768,6 +4768,12 @@ public void testIfStatement() throws Exception {
                 doTest(fileName);
             }
 
+            @TestMetadata("switchLikeWhen.kt")
+            public void testSwitchLikeWhen() throws Exception {
+                String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/coroutines/controlFlow/switchLikeWhen.kt");
+                doTest(fileName);
+            }
+
             @TestMetadata("throwFromCatch.kt")
             public void testThrowFromCatch() throws Exception {
                 String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/coroutines/controlFlow/throwFromCatch.kt");
@@ -7679,6 +7685,12 @@ public void testAssignPlusOnSmartCast() throws Exception {
             doTest(fileName);
         }
 
+        @TestMetadata("augmentedAssignmentWithComplexRhs.kt")
+        public void testAugmentedAssignmentWithComplexRhs() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/increment/augmentedAssignmentWithComplexRhs.kt");
+            doTest(fileName);
+        }
+
         @TestMetadata("classNaryGetSet.kt")
         public void testClassNaryGetSet() throws Exception {
             String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/increment/classNaryGetSet.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
@@ -8838,6 +8838,12 @@ public void testAssignPlusOnSmartCast() throws Exception {
             doTest(fileName);
         }
 
+        @TestMetadata("augmentedAssignmentWithComplexRhs.kt")
+        public void testAugmentedAssignmentWithComplexRhs() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/increment/augmentedAssignmentWithComplexRhs.kt");
+            doTest(fileName);
+        }
+
         @TestMetadata("classNaryGetSet.kt")
         public void testClassNaryGetSet() throws Exception {
             String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/increment/classNaryGetSet.kt");
diff --git a/js/js.translator/src/org/jetbrains/kotlin/js/translate/callTranslator/VariableCallCases.kt b/js/js.translator/src/org/jetbrains/kotlin/js/translate/callTranslator/VariableCallCases.kt
@@ -91,7 +91,11 @@ object DefaultVariableAccessCase : VariableAccessCase() {
             getCapturedVarAccessor(functionRef)
         }
         else {
-            functionRef
+            functionRef.apply {
+                if (isGetAccess()) {
+                    sideEffects = SideEffectKind.DEPENDS_ON_STATE
+                }
+            }
         }
 
         val localVariableDescriptor = resolvedCall.resultingDescriptor as? LocalVariableDescriptor
diff --git a/js/js.translator/src/org/jetbrains/kotlin/js/translate/operation/IntrinsicAssignmentTranslator.java b/js/js.translator/src/org/jetbrains/kotlin/js/translate/operation/IntrinsicAssignmentTranslator.java
@@ -23,6 +23,7 @@
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.kotlin.js.translate.context.TranslationContext;
 import org.jetbrains.kotlin.js.translate.reference.AccessTranslator;
+import org.jetbrains.kotlin.lexer.KtSingleValueToken;
 import org.jetbrains.kotlin.lexer.KtToken;
 import org.jetbrains.kotlin.psi.KtBinaryExpression;
 import org.jetbrains.kotlin.types.expressions.OperatorConventions;
@@ -33,8 +34,10 @@
 import static org.jetbrains.kotlin.js.translate.utils.TranslationUtils.translateRightExpression;
 
 public final class IntrinsicAssignmentTranslator extends AssignmentTranslator {
-    private JsExpression right;
-    private AccessTranslator accessTranslator;
+    private final JsExpression right;
+    private final AccessTranslator accessTranslator;
+    private final boolean rightExpressionTrivial;
+    private final JsBlock rightBlock = new JsBlock();
 
     @NotNull
     public static JsExpression doTranslate(@NotNull KtBinaryExpression expression,
@@ -46,10 +49,9 @@ private IntrinsicAssignmentTranslator(@NotNull KtBinaryExpression expression,
                                           @NotNull TranslationContext context) {
         super(expression, context);
 
-        JsBlock rightBlock = new JsBlock();
         right = translateRightExpression(context, expression, rightBlock);
-        accessTranslator = createAccessTranslator(expression.getLeft(), !rightBlock.isEmpty());
-        context.addStatementsToCurrentBlockFrom(rightBlock);
+        rightExpressionTrivial = rightBlock.isEmpty();
+        accessTranslator = createAccessTranslator(expression.getLeft(), !rightExpressionTrivial);
     }
 
     @NotNull
@@ -62,7 +64,7 @@ private JsExpression translate() {
 
     @NotNull
     private JsExpression translateAsAssignmentOperation() {
-        if (isSimpleNameExpressionNotDelegatedLocalVar(expression.getLeft(), context())) {
+        if (isSimpleNameExpressionNotDelegatedLocalVar(expression.getLeft(), context()) && rightExpressionTrivial) {
             return translateAsPlainAssignmentOperation();
         }
         return translateAsAssignToCounterpart();
@@ -71,14 +73,19 @@ private JsExpression translateAsAssignmentOperation() {
     @NotNull
     private JsExpression translateAsAssignToCounterpart() {
         JsBinaryOperator operator = getCounterpartOperator();
-        JsBinaryOperation counterpartOperation =
-                new JsBinaryOperation(operator, accessTranslator.translateAsGet(), right);
+        JsExpression oldValue = accessTranslator.translateAsGet();
+        if (!rightExpressionTrivial) {
+            oldValue = context().defineTemporary(oldValue);
+        }
+        JsBinaryOperation counterpartOperation = new JsBinaryOperation(operator, oldValue, right);
+        context().addStatementsToCurrentBlockFrom(rightBlock);
         return accessTranslator.translateAsSet(counterpartOperation);
     }
 
     @NotNull
     private JsBinaryOperator getCounterpartOperator() {
         KtToken assignmentOperationToken = getOperationToken(expression);
+        assert assignmentOperationToken instanceof KtSingleValueToken;
         assert OperatorConventions.ASSIGNMENT_OPERATIONS.containsKey(assignmentOperationToken);
         KtToken counterpartToken = OperatorConventions.ASSIGNMENT_OPERATION_COUNTERPARTS.get(assignmentOperationToken);
         assert OperatorTable.hasCorrespondingBinaryOperator(counterpartToken) :
@@ -88,13 +95,15 @@ private JsBinaryOperator getCounterpartOperator() {
 
     @NotNull
     private JsExpression translateAsPlainAssignmentOperation() {
+        context().addStatementsToCurrentBlockFrom(rightBlock);
         JsBinaryOperator operator = getAssignmentOperator();
         return new JsBinaryOperation(operator, accessTranslator.translateAsGet(), right);
     }
 
     @NotNull
     private JsBinaryOperator getAssignmentOperator() {
         KtToken token = getOperationToken(expression);
+        assert token instanceof KtSingleValueToken;
         assert OperatorConventions.ASSIGNMENT_OPERATIONS.containsKey(token);
         assert OperatorTable.hasCorrespondingBinaryOperator(token) :
                 "Unsupported token encountered: " + token.toString();
@@ -103,6 +112,7 @@ private JsBinaryOperator getAssignmentOperator() {
 
     @NotNull
     private JsExpression translateAsPlainAssignment() {
+        context().addStatementsToCurrentBlockFrom(rightBlock);
         return accessTranslator.translateAsSet(right);
     }
 }

Original file line number	Diff line number	Diff line change
`@@ -1,9 +1,6 @@`
`1`	`1`	`// WITH_RUNTIME`
`2`	`2`	`// NO_INTERCEPT_RESUME_TESTS`
`3`	`3`
`4`		`-// TODO: fix bug in JVM backend and remove this directive`
`5`		`-// TARGET_BACKEND: JS`
`6`		`-`
`7`	`4`	`class Controller {`
`8`	`5`	`var result = ""`
`9`	`6`
`@@ -29,7 +26,7 @@ fun box(): String {`
`29`	`26`	`}`
`30`	`27`	`}`
`31`	`28`	`}`
`32`		`- if (value != "A;[B][C]!") return "fail: suspend as if condition: $value"`
	`29`	`+ if (value != "A;B]C]!") return "fail: suspend as if condition: $value"`
`33`	`30`
`34`	`31`	`return "OK"`
`35`	`32`	`}`
Original file line number	Diff line number	Diff line change
`@@ -91,7 +91,11 @@ object DefaultVariableAccessCase : VariableAccessCase() {`
`91`	`91`	`getCapturedVarAccessor(functionRef)`
`92`	`92`	`}`
`93`	`93`	`else {`
`94`		`- functionRef`
	`94`	`+ functionRef.apply {`
	`95`	`+ if (isGetAccess()) {`
	`96`	`+ sideEffects = SideEffectKind.DEPENDS_ON_STATE`
	`97`	`+ }`
	`98`	`+ }`
`95`	`99`	`}`
`96`	`100`
`97`	`101`	`val localVariableDescriptor = resolvedCall.resultingDescriptor as? LocalVariableDescriptor`