JS/Inlining: refactor TemporaryAssignmentElimination, add more tests

js/js.inliner/src/org/jetbrains/kotlin/js/inline/clean/TemporaryAssignmentElimination.kt

@@ -24,10 +24,17 @@ import org.jetbrains.kotlin.js.inline.util.collectFreeVariables
 import org.jetbrains.kotlin.js.translate.utils.JsAstUtils
 
 internal class TemporaryAssignmentElimination(private val root: JsBlock) {
-    private val usageCount = mutableMapOf<JsName, Int>()
+    // We say "use" about any references to a temporary variable
+    private val useCount = mutableMapOf<JsName, Int>()
+
+    // We say "usage" about special kind of a reference to a temporary variable in the following cases:
+    //   someVar = $tmp;
+    //   var someVar = $tmp;
+    //   someObj.prop = $tmp;
+    //   return $tmp;
     private val usages = mutableMapOf<JsName, Usage>()
     private val statementsToRemove = mutableSetOf<JsStatement>()
-    private val mappedUsages = mutableMapOf<JsName, UsageHolder?>()
+    private val usageSequences = mutableMapOf<JsName, UsageSequence?>()
     private val syntheticNames = mutableSetOf<JsName>()
     private var hasChanges = false
     private val namesToProcess = mutableSetOf<JsName>()
@@ -120,32 +127,16 @@ internal class TemporaryAssignmentElimination(private val root: JsBlock) {
         usages.keys.retainAll(syntheticNames)
     }
 
-    private fun getUsage(name: JsName): UsageHolder? {
-        return mappedUsages.getOrPut(name) {
-            if (usageCount[name] != 1) return null
+    private fun getUsageSequence(name: JsName): UsageSequence? {
+        return usageSequences.getOrPut(name) {
+            if (useCount[name] != 1) return null
 
             val usage = usages[name]
-            val mappedUsage: UsageHolder? = when (usage) {
-                is Usage.VariableAssignment -> {
-                    val result = getUsage(usage.target)
-                    if (result != null) {
-                        UsageHolder(result.value, usage.statement, result)
-                    }
-                    else {
-                        UsageHolder(usage, usage.statement, null)
-                    }
-                }
-                is Usage.VariableDeclaration -> {
-                    val result = getUsage(usage.target)
-                    if (result != null) {
-                        UsageHolder(result.value, usage.statement, result)
-                    }
-                    else {
-                        UsageHolder(usage, usage.statement, null)
-                    }
-                }
+            val mappedUsage: UsageSequence? = when (usage) {
+                is Usage.VariableAssignment -> UsageSequence(usage, getUsageSequence(usage.target))
+                is Usage.VariableDeclaration -> UsageSequence(usage, getUsageSequence(usage.target))
                 null -> null
-                else -> UsageHolder(usage, usage.statement, null)
+                else -> UsageSequence(usage, null)
             }
 
             mappedUsage
@@ -153,13 +144,13 @@ internal class TemporaryAssignmentElimination(private val root: JsBlock) {
     }
 
     private fun calculateDeclarations() {
-        usages.keys.forEach { getUsage(it) }
+        usages.keys.forEach { getUsageSequence(it) }
 
         object : RecursiveJsVisitor() {
             override fun visitExpressionStatement(x: JsExpressionStatement) {
                 val assignment = JsAstUtils.decomposeAssignmentToVariable(x.expression)
                 if (assignment != null) {
-                    val usage = getUsage(assignment.first)?.value
+                    val usage = getUsageSequence(assignment.first)?.lastUsage()
                     if (usage is Usage.VariableDeclaration) {
                         usage.count++
                     }
@@ -181,9 +172,9 @@ internal class TemporaryAssignmentElimination(private val root: JsBlock) {
                 val assignment = JsAstUtils.decomposeAssignmentToVariable(x.expression)
                 if (assignment != null) {
                     val (name, value) = assignment
-                    val usageHolder = getUsage(name)
-                    if (usageHolder != null) {
-                        val usage = usageHolder.value
+                    val usageSequence = getUsageSequence(name)
+                    if (usageSequence != null) {
+                        val usage = usageSequence.lastUsage()
                         val replacement = when (usage) {
                             is Usage.Return -> JsReturn(value).apply { source(x.expression.source) }
                             is Usage.VariableAssignment -> {
@@ -213,7 +204,7 @@ internal class TemporaryAssignmentElimination(private val root: JsBlock) {
                         }
                         hasChanges = true
                         ctx.replaceMe(replacement)
-                        statementsToRemove += usageHolder.collectStatements()
+                        statementsToRemove += usageSequence.collectStatements()
                         return false
                     }
                 }
@@ -264,7 +255,7 @@ internal class TemporaryAssignmentElimination(private val root: JsBlock) {
     }
 
     private fun use(name: JsName) {
-        usageCount[name] = 1 + (usageCount[name] ?: 0)
+        useCount[name] = 1 + (useCount[name] ?: 0)
     }
 
     private sealed class Usage(val statement: JsStatement) {
@@ -279,7 +270,9 @@ internal class TemporaryAssignmentElimination(private val root: JsBlock) {
         class PropertyMutation(statement: JsStatement, val target: JsExpression) : Usage(statement)
     }
 
-    private class UsageHolder(val value: Usage, val statement: JsStatement, val next: UsageHolder?) {
-        fun collectStatements() = generateSequence (this) { it.next }.map { it.statement }
+    private class UsageSequence(val value: Usage, val next: UsageSequence?) {
+        fun collectStatements() = generateSequence(this) { it.next }.map { it.value.statement }
+
+        fun lastUsage() = generateSequence(this) { it.next }.last().value
     }
 }

js/js.tests/test/org/jetbrains/kotlin/js/test/optimizer/TemporaryAssignmentEliminationTest.kt

@@ -28,4 +28,10 @@ class TemporaryAssignmentEliminationTest : BasicOptimizerTest("temporary-assignm
     @Test fun skipsGlobalDeclarations() = box()
 
     @Test fun transitiveAssignment() = box()
+
+    @Test fun transitiveChain() = box()
+
+    @Test fun tryCatch() = box()
+
+    @Test fun ifWithoutElse() = box()
 }

js/js.translator/testData/js-optimizer/temporary-assignment/ifWithoutElse.optimized.js

@@ -0,0 +1,19 @@
+function f(x) {
+    return x;
+}
+
+function box() {
+    var result1, result2;
+
+    if (f(true)) {
+        result1 = "1";
+    }
+    if (result1 !== "1") return "fail1: " + result1;
+
+    if (f(false)) {
+        result2 = "1";
+    }
+    if (result2 !== void 0) return "fail2: " + result2;
+
+    return "OK";
+}

js/js.translator/testData/js-optimizer/temporary-assignment/ifWithoutElse.original.js

@@ -0,0 +1,21 @@
+function f(x) {
+    return x;
+}
+
+function box() {
+    var result1, result2, $a, $b;
+    
+    if (f(true)) {
+        $a = "1";
+    }
+    result1 = $a;
+    if (result1 !== "1") return "fail1: " + result1;
+    
+    if (f(false)) {
+        $b = "1";
+    }
+    result2 = $b;
+    if (result2 !== void 0) return "fail2: " + result2;
+    
+    return "OK";
+}

js/js.translator/testData/js-optimizer/temporary-assignment/transitiveChain.optimized.js

@@ -0,0 +1,71 @@
+function f(x) {
+    return x;
+}
+
+function testRegular() {
+    var result;
+    result = f("testRegular");
+    return result;
+}
+
+function testIrregular() {
+    var result;
+    result = f("testIrregular");
+    return result;
+}
+
+function testDoubleUse1() {
+    var result;
+    var $b;
+    $b = f("testDoubleUse1");
+    result = $b;
+    f($b);
+    return result;
+}
+
+function testDoubleUse2() {
+    var $d;
+    $d = f("testDoubleUse2");
+    var result = $d;
+    f($d);
+    return result;
+}
+
+function testDoubleUse3() {
+    var result;
+    var $a;
+    $a = f("testDoubleUse3");
+    result = $a;
+    f($a);
+    return result;
+}
+
+function testCircular() {
+    var $b;
+    $b = f("testCircular");
+    $b = $b;
+    var result = $b;
+    return result;
+}
+
+function box() {
+    var result = testRegular();
+    if (result != "testRegular") return "failRegular: " + result;
+
+    result = testIrregular();
+    if (result != "testIrregular") return "failIrregular: " + result;
+
+    result = testDoubleUse1();
+    if (result != "testDoubleUse1") return "failDoubleUse1: " + result;
+
+    result = testDoubleUse2();
+    if (result != "testDoubleUse2") return "failDoubleUse2: " + result;
+
+    result = testDoubleUse3();
+    if (result != "testDoubleUse3") return "failDoubleUse3: " + result;
+
+    result = testCircular();
+    if (result != "testCircular") return "failCircular: " + result;
+
+    return "OK"
+}

js/js.translator/testData/js-optimizer/temporary-assignment/transitiveChain.original.js

@@ -0,0 +1,89 @@
+function f(x) {
+    return x;
+}
+
+function testRegular() {
+    var $a, $b, $c, $d;
+    $a = f("testRegular");
+    $b = $a;
+    $c = $b;
+    $d = $c;
+    var result = $d;
+    return result;
+}
+
+function testIrregular() {
+    var $a, $b, $d;
+    $a = f("testIrregular");
+    $b = $a;
+    var $c = $b;
+    $d = $c;
+    var result = $d;
+    return result;
+}
+
+function testDoubleUse1() {
+    var $a, $b, $c, $d;
+    $a = f("testDoubleUse1");
+    $b = $a;
+    $c = $b;
+    $d = $c;
+    var result = $d;
+    f($b);
+    return result;
+}
+
+function testDoubleUse2() {
+    var $a, $b, $c, $d;
+    $a = f("testDoubleUse2");
+    $b = $a;
+    $c = $b;
+    $d = $c;
+    var result = $d;
+    f($d);
+    return result;
+}
+
+function testDoubleUse3() {
+    var $a, $b, $c, $d;
+    $a = f("testDoubleUse3");
+    $b = $a;
+    $c = $b;
+    $d = $c;
+    var result = $d;
+    f($a);
+    return result;
+}
+
+function testCircular() {
+    var $a, $b, $c, $d;
+    $a = f("testCircular");
+    $b = $a;
+    $c = $b;
+    $d = $c;
+    $b = $d;
+    var result = $b;
+    return result;
+}
+
+function box() {
+    var result = testRegular();
+    if (result != "testRegular") return "failRegular: " + result;
+    
+    result = testIrregular();
+    if (result != "testIrregular") return "failIrregular: " + result;
+
+    result = testDoubleUse1();
+    if (result != "testDoubleUse1") return "failDoubleUse1: " + result;
+
+    result = testDoubleUse2();
+    if (result != "testDoubleUse2") return "failDoubleUse2: " + result;
+
+    result = testDoubleUse3();
+    if (result != "testDoubleUse3") return "failDoubleUse3: " + result;
+
+    result = testCircular();
+    if (result != "testCircular") return "failCircular: " + result;
+    
+    return "OK"
+}

js/js.translator/testData/js-optimizer/temporary-assignment/tryCatch.optimized.js

@@ -0,0 +1,57 @@
+function f(x) {
+    return x;
+}
+
+function testCatch1() {
+    var result;
+    try {
+        result = f("testCatch1");
+        throw new Error();
+    } catch (e) {
+    }
+    return result;
+}
+
+function testCatch2() {
+    var result;
+    try {
+        throw new Error();
+    } catch (e) {
+    }
+    return result;
+}
+
+function testFinally() {
+    var result;
+    try {
+        result = f("testFinally");
+    } finally {
+    }
+    return result;
+}
+
+function testOuter() {
+    var result;
+    try {
+        result = f("testOuter");
+    } finally {
+        f("23")
+    }
+    return result;
+}
+
+function box() {
+    var result = testCatch1();
+    if (result !== "testCatch1") return "failCatch1: " + result;
+
+    result = testCatch2();
+    if (result !== void 0) return "failCatch2: " + result;
+
+    result = testFinally();
+    if (result !== "testFinally") return "failFinally: " + result;
+
+    result = testOuter();
+    if (result !== "testOuter") return "testOuter: " + result;
+
+    return "OK";
+}

js/js.translator/testData/js-optimizer/temporary-assignment/tryCatch.original.js

@@ -0,0 +1,66 @@
+function f(x) {
+    return x;
+}
+
+function testCatch1() {
+    var $tmp;
+    var result;
+    try {
+        $tmp = f("testCatch1");
+        throw new Error();
+    } catch (e) {
+        result = $tmp;
+    }
+    return result;
+}
+
+function testCatch2() {
+    var $tmp;
+    var result;
+    try {
+        throw new Error();
+        $tmp = f("testCatch2");
+    } catch (e) {
+        result = $tmp;
+    }
+    return result;
+}
+
+function testFinally() {
+    var $tmp;
+    var result;
+    try {
+        $tmp = f("testFinally");
+    } finally {
+        result = $tmp;
+    }
+    return result;
+}
+
+function testOuter() {
+    var $tmp;
+    var result;
+    try {
+        $tmp = f("testOuter");
+    } finally {
+        f("23")
+    }
+    result = $tmp;
+    return result;
+}
+
+function box() {
+    var result = testCatch1();
+    if (result !== "testCatch1") return "failCatch1: " + result;
+
+    result = testCatch2();
+    if (result !== void 0) return "failCatch2: " + result;
+
+    result = testFinally();
+    if (result !== "testFinally") return "failFinally: " + result;
+
+    result = testOuter();
+    if (result !== "testOuter") return "testOuter: " + result;
+
+    return "OK";
+}