From 57ac23a60608ee40f33e521f14d5cbbebeb2b3e3 Mon Sep 17 00:00:00 2001
From: Alexey Andreev <Alexey.Andreev@jetbrains.com>
Date: Wed, 1 Jun 2016 12:57:53 +0300
Subject: [PATCH] JS/Inlining: in temporary variable elimination don't treat
 FQN without side effects as trivial, i.e. disable moving these vars freely
 across function body

---
 .../js/inline/clean/FunctionPostProcessor.kt  |  2 +-
 .../clean/TemporaryVariableElimination.kt     | 30 ++++++-------------
 .../cases/propertyAssignment.kt               |  2 +-
 3 files changed, 11 insertions(+), 23 deletions(-)

diff --git a/js/js.inliner/src/org/jetbrains/kotlin/js/inline/clean/FunctionPostProcessor.kt b/js/js.inliner/src/org/jetbrains/kotlin/js/inline/clean/FunctionPostProcessor.kt
index eb1cc864ef0..9a1938d6326 100644
--- a/js/js.inliner/src/org/jetbrains/kotlin/js/inline/clean/FunctionPostProcessor.kt
+++ b/js/js.inliner/src/org/jetbrains/kotlin/js/inline/clean/FunctionPostProcessor.kt
@@ -22,7 +22,7 @@ class FunctionPostProcessor(root: JsFunction) {
     val optimizations = listOf(
         { TemporaryAssignmentElimination(root.body).apply() },
         { RedundantLabelRemoval(root.body).apply() },
-        { TemporaryVariableElimination(root.body).apply() },
+        { TemporaryVariableElimination(root).apply() },
         { IfStatementReduction(root.body).apply() },
         { DeadCodeElimination(root.body).apply() },
         { RedundantVariableDeclarationElimination(root.body).apply() },
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 36c78a44abd..3eee00f4c5b 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
@@ -19,18 +19,19 @@ package org.jetbrains.kotlin.js.inline.clean
 import com.google.dart.compiler.backend.js.ast.*
 import com.google.dart.compiler.backend.js.ast.metadata.sideEffects
 import com.google.dart.compiler.backend.js.ast.metadata.synthetic
-import org.jetbrains.kotlin.js.inline.util.collectDefinedNames
 import org.jetbrains.kotlin.js.inline.util.collectFreeVariables
+import org.jetbrains.kotlin.js.inline.util.collectLocalVariables
 import org.jetbrains.kotlin.js.translate.utils.JsAstUtils
 
-internal class TemporaryVariableElimination(private val root: JsStatement) {
+internal class TemporaryVariableElimination(function: JsFunction) {
+    private val root = function.body
     private val definitions = mutableMapOf<JsName, Int>()
     private val usages = mutableMapOf<JsName, Int>()
     private val inconsistent = mutableSetOf<JsName>()
     private val definedValues = mutableMapOf<JsName, JsExpression>()
     private val temporary = mutableSetOf<JsName>()
     private var hasChanges = false
-    private val namesToProcess = mutableSetOf<JsName>()
+    private val namesToProcess = function.collectLocalVariables()
     private val statementsToRemove = mutableSetOf<JsNode>()
     private val namesToSubstitute = mutableSetOf<JsName>()
     private val variablesToRemove = mutableSetOf<JsName>()
@@ -43,8 +44,6 @@ internal class TemporaryVariableElimination(private val root: JsStatement) {
     }
 
     private fun analyze() {
-        namesToProcess += collectDefinedNames(root)
-
         object : RecursiveJsVisitor() {
             val currentScope = mutableSetOf<JsName>()
             var localVars = mutableSetOf<JsName>()
@@ -543,22 +542,11 @@ internal class TemporaryVariableElimination(private val root: JsStatement) {
 
     private fun isTrivial(expr: JsExpression): Boolean = when (expr) {
         is JsNameRef -> {
-            val qualifier = expr.qualifier
-            if (qualifier == null) {
-                if (!expr.sideEffects) {
-                    true
-                }
-                else {
-                    val name = expr.name
-                    name != null && when (definitions[name] ?: 0) {
-                        0 -> true
-                        1 -> name !in namesToSubstitute || definedValues[name]?.let { isTrivial(it) } ?: false
-                        else -> false
-                    }
-                }
-            }
-            else {
-                !expr.sideEffects && isTrivial(qualifier)
+            val name = expr.name
+            name in namesToProcess && when (definitions[name]) {
+                null, 0 -> true
+                1  -> name !in namesToSubstitute || definedValues[name]?.let { isTrivial(it) } ?: false
+                else -> false
             }
         }
         is JsLiteral.JsValueLiteral -> true
diff --git a/js/js.translator/testData/inlineSizeReduction/cases/propertyAssignment.kt b/js/js.translator/testData/inlineSizeReduction/cases/propertyAssignment.kt
index 3de81a4bd1b..6483c91d639 100644
--- a/js/js.translator/testData/inlineSizeReduction/cases/propertyAssignment.kt
+++ b/js/js.translator/testData/inlineSizeReduction/cases/propertyAssignment.kt
@@ -1,7 +1,7 @@
 package foo
 
 // CHECK_CONTAINS_NO_CALLS: test
-// CHECK_VARS_COUNT: function=test count=0
+// CHECK_VARS_COUNT: function=test count=1
 
 object SumHolder {
     var sum = 0