From d89ce80d97c067fe6a806448f88e40adec190f9a Mon Sep 17 00:00:00 2001
From: Alexey Andreev <Alexey.Andreev@jetbrains.com>
Date: Thu, 27 Apr 2017 19:51:04 +0300
Subject: [PATCH] Fix temporary variable elimination in JS BE

Don't eliminate temporary variable when between its usage and its
definition there's assignment to a non-local variable.

Fix KT-17540
---
 .../js/inline/clean/TemporaryVariableElimination.kt   |  2 +-
 .../optimizer/TemporaryVariableEliminationTest.kt     |  2 ++
 .../kotlin/js/test/semantics/BoxJsTestGenerated.java  |  6 ++++++
 .../box/inlineEvaluationOrder/alsoWithReassingment.kt | 10 ++++++++++
 .../assignmentToOuterVar.optimized.js                 | 11 +++++++++++
 .../assignmentToOuterVar.original.js                  | 11 +++++++++++
 6 files changed, 41 insertions(+), 1 deletion(-)
 create mode 100644 js/js.translator/testData/box/inlineEvaluationOrder/alsoWithReassingment.kt
 create mode 100644 js/js.translator/testData/js-optimizer/temporary-variable/assignmentToOuterVar.optimized.js
 create mode 100644 js/js.translator/testData/js-optimizer/temporary-variable/assignmentToOuterVar.original.js

diff --git a/js/js.inliner/src/org/jetbrains/kotlin/js/inline/clean/TemporaryVariableElimination.kt b/js/js.inliner/src/org/jetbrains/kotlin/js/inline/clean/TemporaryVariableElimination.kt
index 84bd33984c9..e7ee817b163 100644
--- a/js/js.inliner/src/org/jetbrains/kotlin/js/inline/clean/TemporaryVariableElimination.kt
+++ b/js/js.inliner/src/org/jetbrains/kotlin/js/inline/clean/TemporaryVariableElimination.kt
@@ -256,7 +256,7 @@ internal class TemporaryVariableElimination(private val function: JsFunction) {
             }
 
             private fun handleDefinition(name: JsName, value: JsExpression, node: JsNode) {
-                val sideEffects = handleExpression(value)
+                val sideEffects = handleExpression(value) || name !in localVariables
                 if (shouldConsiderTemporary(name)) {
                     if (isTrivial(value)) {
                         statementsToRemove += node
diff --git a/js/js.tests/test/org/jetbrains/kotlin/js/test/optimizer/TemporaryVariableEliminationTest.kt b/js/js.tests/test/org/jetbrains/kotlin/js/test/optimizer/TemporaryVariableEliminationTest.kt
index 8daae5e6b27..69514cbd940 100644
--- a/js/js.tests/test/org/jetbrains/kotlin/js/test/optimizer/TemporaryVariableEliminationTest.kt
+++ b/js/js.tests/test/org/jetbrains/kotlin/js/test/optimizer/TemporaryVariableEliminationTest.kt
@@ -44,4 +44,6 @@ class TemporaryVariableEliminationTest : BasicOptimizerTest("temporary-variable"
     @Test fun assignmentToNonLocal() = box()
 
     @Test fun removeUnusedAndSubstitute() = box()
+
+    @Test fun assignmentToOuterVar() = box()
 }
\ No newline at end of file
diff --git a/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/BoxJsTestGenerated.java b/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/BoxJsTestGenerated.java
index e0849b39142..4a84bc455b1 100644
--- a/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/BoxJsTestGenerated.java
+++ b/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/BoxJsTestGenerated.java
@@ -4112,6 +4112,12 @@ public class BoxJsTestGenerated extends AbstractBoxJsTest {
             KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("js/js.translator/testData/box/inlineEvaluationOrder"), Pattern.compile("^([^_](.+))\\.kt$"), TargetBackend.JS, true);
         }
 
+        @TestMetadata("alsoWithReassingment.kt")
+        public void testAlsoWithReassingment() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("js/js.translator/testData/box/inlineEvaluationOrder/alsoWithReassingment.kt");
+            doTest(fileName);
+        }
+
         @TestMetadata("argumentOfCall.kt")
         public void testArgumentOfCall() throws Exception {
             String fileName = KotlinTestUtils.navigationMetadata("js/js.translator/testData/box/inlineEvaluationOrder/argumentOfCall.kt");
diff --git a/js/js.translator/testData/box/inlineEvaluationOrder/alsoWithReassingment.kt b/js/js.translator/testData/box/inlineEvaluationOrder/alsoWithReassingment.kt
new file mode 100644
index 00000000000..3d7df2dd28f
--- /dev/null
+++ b/js/js.translator/testData/box/inlineEvaluationOrder/alsoWithReassingment.kt
@@ -0,0 +1,10 @@
+private var _value: String = "OK"
+
+private inline fun String.myAlso(f: (String) -> Unit): String {
+    f(this)
+    return this
+}
+
+fun overrideValueAndReturnOld(newValue: String) = _value.myAlso { _value = newValue }
+
+fun box() = overrideValueAndReturnOld("fail")
\ No newline at end of file
diff --git a/js/js.translator/testData/js-optimizer/temporary-variable/assignmentToOuterVar.optimized.js b/js/js.translator/testData/js-optimizer/temporary-variable/assignmentToOuterVar.optimized.js
new file mode 100644
index 00000000000..279720510e4
--- /dev/null
+++ b/js/js.translator/testData/js-optimizer/temporary-variable/assignmentToOuterVar.optimized.js
@@ -0,0 +1,11 @@
+var value = "OK";
+
+function foo(newValue) {
+    var $tmp = value;
+    value = newValue;
+    return $tmp;
+}
+
+function box() {
+    return foo("fail");
+}
\ No newline at end of file
diff --git a/js/js.translator/testData/js-optimizer/temporary-variable/assignmentToOuterVar.original.js b/js/js.translator/testData/js-optimizer/temporary-variable/assignmentToOuterVar.original.js
new file mode 100644
index 00000000000..279720510e4
--- /dev/null
+++ b/js/js.translator/testData/js-optimizer/temporary-variable/assignmentToOuterVar.original.js
@@ -0,0 +1,11 @@
+var value = "OK";
+
+function foo(newValue) {
+    var $tmp = value;
+    value = newValue;
+    return $tmp;
+}
+
+function box() {
+    return foo("fail");
+}
\ No newline at end of file