Make JS optimizations respect nested functions

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

@@ -67,6 +67,8 @@ internal class RedundantLabelRemoval(private val root: JsStatement) {
 
                 super.endVisit(x, ctx)
             }
+
+            override fun visit(x: JsFunction, ctx: JsContext<*>) = false
         }.accept(root)
     }
 

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

@@ -18,6 +18,7 @@ package org.jetbrains.kotlin.js.inline.clean
 
 import com.google.dart.compiler.backend.js.ast.*
 import com.google.dart.compiler.backend.js.ast.metadata.synthetic
+import org.jetbrains.kotlin.js.inline.util.collectFreeVariables
 
 internal class RedundantVariableDeclarationElimination(private val root: JsStatement) {
     private val usages = mutableSetOf<JsName>()
@@ -40,6 +41,11 @@ internal class RedundantVariableDeclarationElimination(private val root: JsState
             override fun visit(x: JsBreak, ctx: JsContext<*>) = false
 
             override fun visit(x: JsContinue, ctx: JsContext<*>) = false
+
+            override fun visit(x: JsFunction, ctx: JsContext<*>): Boolean {
+                usages += x.collectFreeVariables()
+                return false
+            }
         }.accept(root)
     }
 
@@ -54,6 +60,8 @@ internal class RedundantVariableDeclarationElimination(private val root: JsState
                 }
                 super.endVisit(x, ctx)
             }
+
+            override fun visit(x: JsFunction, ctx: JsContext<*>) = false
         }.accept(root)
     }
 }

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

@@ -19,6 +19,7 @@ package org.jetbrains.kotlin.js.inline.clean
 import com.google.dart.compiler.backend.js.ast.*
 import com.google.dart.compiler.backend.js.ast.metadata.synthetic
 import org.jetbrains.kotlin.js.inline.util.canHaveSideEffect
+import org.jetbrains.kotlin.js.inline.util.collectFreeVariables
 import org.jetbrains.kotlin.js.translate.utils.JsAstUtils
 
 internal class TemporaryAssignmentElimination(private val root: JsBlock) {
@@ -97,6 +98,15 @@ internal class TemporaryAssignmentElimination(private val root: JsBlock) {
                 }
                 return super.visit(x, ctx)
             }
+
+            override fun visit(x: JsFunction, ctx: JsContext<*>): Boolean {
+                x.collectFreeVariables().forEach { use(it); use(it) }
+                return false
+            }
+
+            override fun visit(x: JsBreak, ctx: JsContext<*>) = false
+
+            override fun visit(x: JsContinue, ctx: JsContext<*>) = false
         }.accept(root)
 
         usages.keys.retainAll(syntheticNames)
@@ -198,6 +208,8 @@ internal class TemporaryAssignmentElimination(private val root: JsBlock) {
                 }
                 return super.visit(x, ctx)
             }
+
+            override fun visit(x: JsFunction, ctx: JsContext<*>) = false
         }.accept(root)
     }
 

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

@@ -19,6 +19,7 @@ package org.jetbrains.kotlin.js.inline.clean
 import com.google.dart.compiler.backend.js.ast.*
 import com.google.dart.compiler.backend.js.ast.metadata.synthetic
 import org.jetbrains.kotlin.js.inline.util.canHaveSideEffect
+import org.jetbrains.kotlin.js.inline.util.collectFreeVariables
 import org.jetbrains.kotlin.js.translate.utils.JsAstUtils
 
 internal class TemporaryVariableElimination(private val root: JsStatement) {
@@ -121,6 +122,11 @@ internal class TemporaryVariableElimination(private val root: JsStatement) {
                 }
                 return false
             }
+
+            override fun visit(x: JsFunction, ctx: JsContext<*>): Boolean {
+                x.collectFreeVariables().forEach { useVariable(it); useVariable(it) }
+                return super.visit(x, ctx)
+            }
         }.accept(root)
     }
 

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

@@ -19,7 +19,7 @@ package org.jetbrains.kotlin.js.inline.clean
 import com.google.dart.compiler.backend.js.ast.*
 import com.google.dart.compiler.backend.js.ast.metadata.staticRef
 
-import org.jetbrains.kotlin.js.inline.util.collectReferencesInside
+import org.jetbrains.kotlin.js.inline.util.collectUsedNames
 
 /**
  * Removes unused local function declarations like:
@@ -53,7 +53,7 @@ private class UnusedInstanceCollector : JsVisitorWithContextImpl() {
         val currentStatement = statementContext.currentNode
         tracker.addCandidateForRemoval(name, currentStatement!!)
 
-        val references = collectReferencesInside(x)
+        val references = collectUsedNames(x)
         references.filterNotNull()
                   .forEach { tracker.addRemovableReference(name, it) }
 

js/js.inliner/src/org/jetbrains/kotlin/js/inline/util/collectUtils.kt

@@ -19,32 +19,105 @@ package org.jetbrains.kotlin.js.inline.util
 import com.google.dart.compiler.backend.js.ast.*
 import com.google.dart.compiler.backend.js.ast.metadata.staticRef
 
-import java.util.IdentityHashMap
-import org.jetbrains.kotlin.js.inline.util.collectors.ReferenceNameCollector
-import org.jetbrains.kotlin.js.inline.util.collectors.NameCollector
 import org.jetbrains.kotlin.js.inline.util.collectors.InstanceCollector
 import org.jetbrains.kotlin.js.inline.util.collectors.PropertyCollector
 import org.jetbrains.kotlin.js.translate.expression.*
+import java.util.*
 
 fun collectFunctionReferencesInside(scope: JsNode): List<JsName> =
-     collectReferencesInside(scope).filter { it.staticRef is JsFunction }
+        collectReferencedNames(scope).filter { it.staticRef is JsFunction }
 
-fun collectReferencesInside(scope: JsNode): List<JsName> {
+fun collectReferencedNames(scope: JsNode): Set<JsName> {
-    return with(ReferenceNameCollector()) {
+    val references = IdentitySet<JsName>()
-        accept(scope)
+
-        references
+    object : JsVisitorWithContextImpl() {
-    }
+        override fun visit(x: JsBreak, ctx: JsContext<*>) = false
+
+        override fun visit(x: JsContinue, ctx: JsContext<*>) = false
+
+        override fun visit(x: JsVars.JsVar, ctx: JsContext<*>): Boolean {
+            val initializer = x.initExpression
+            if (initializer != null) {
+                accept(initializer)
+            }
+            return false
+        }
+
+        override fun endVisit(x: JsNameRef, ctx: JsContext<*>) {
+            val name = x.name
+            if (name != null) {
+                references.add(name)
+            }
+        }
+    }.accept(scope)
+
+    return references
 }
 
-fun collectLocalNames(function: JsFunction): List<JsName> {
+fun collectUsedNames(scope: JsNode): Set<JsName> {
-    val functionScope = function.scope
+    val references = IdentitySet<JsName>()
 
-    return with(NameCollector(functionScope)) {
+    object : JsVisitorWithContextImpl() {
-        accept(function.body)
+        override fun visit(x: JsBreak, ctx: JsContext<*>) = false
-        names.values.toList()
+
-    }
+        override fun visit(x: JsContinue, ctx: JsContext<*>) = false
+
+        override fun visit(x: JsVars.JsVar, ctx: JsContext<*>): Boolean {
+            val initializer = x.initExpression
+            if (initializer != null) {
+                accept(initializer)
+            }
+            return false
+        }
+
+        override fun endVisit(x: JsNameRef, ctx: JsContext<*>) {
+            val name = x.name
+            if (name != null && x.qualifier == null) {
+                references.add(name)
+            }
+        }
+
+        override fun visit(x: JsFunction, ctx: JsContext<*>): Boolean {
+            references += x.collectFreeVariables()
+            return false
+        }
+    }.accept(scope)
+
+    return references
 }
 
+fun collectDefinedNames(scope: JsNode): Set<JsName> {
+    val names: MutableMap<String, JsName> = HashMap()
+
+    object : RecursiveJsVisitor() {
+        override fun visit(x: JsVars.JsVar) {
+            val initializer = x.initExpression
+            if (initializer != null) {
+                accept(initializer)
+            }
+            addNameIfNeeded(x.name)
+        }
+
+        override fun visitFunction(x: JsFunction) {
+            val name = x.name
+            if (name != null) {
+                addNameIfNeeded(x.name)
+            }
+        }
+
+        private fun addNameIfNeeded(name: JsName) {
+            val ident = name.ident
+            val nameCollected = names[ident]
+            assert(nameCollected == null || nameCollected === name) { "ambiguous identifier $name" }
+            names[ident] = name
+        }
+    }.accept(scope)
+
+    return names.values.toSet()
+}
+
+fun JsFunction.collectFreeVariables() = collectUsedNames(body) - collectDefinedNames(body) - parameters.map { it.name }
+
 fun collectJsProperties(scope: JsNode): IdentityHashMap<JsName, JsExpression> {
     val collector = PropertyCollector()
     collector.accept(scope)

js/js.inliner/src/org/jetbrains/kotlin/js/inline/util/collectors/NameCollector.kt

@@ -1,50 +0,0 @@
-/*
- * Copyright 2010-2015 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.kotlin.js.inline.util.collectors
-
-import com.google.dart.compiler.backend.js.ast.JsScope
-import com.google.dart.compiler.backend.js.ast.RecursiveJsVisitor
-import com.google.dart.compiler.backend.js.ast.JsFunction
-import com.google.dart.compiler.backend.js.ast.HasName
-import com.google.dart.compiler.backend.js.ast.JsName
-import com.google.dart.compiler.backend.js.ast.JsVars
-
-import java.util.HashMap
-
-class NameCollector(private val scope: JsScope) : RecursiveJsVisitor() {
-    val names: MutableMap<String, JsName> = HashMap()
-
-    override fun visit(x: JsVars.JsVar) {
-        super.visit(x)
-        addNameIfNeeded(x)
-    }
-
-    override fun visitFunction(x: JsFunction) { }
-
-    private fun addNameIfNeeded(hasName: HasName?) {
-        val name = hasName?.name
-        val ident = name?.ident
-
-        if (name == null || ident == null) return
-
-        val nameCollected = names.get(ident)
-        assert(nameCollected == null || nameCollected === name) { "ambiguous identifier for $hasName" }
-        assert(scope.hasOwnName(ident)) { "non-local name was added $hasName" }
-
-        names.put(ident, name)
-    }
-}

js/js.inliner/src/org/jetbrains/kotlin/js/inline/util/collectors/ReferenceNameCollector.kt

@@ -1,37 +0,0 @@
-/*
- * Copyright 2010-2015 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.kotlin.js.inline.util.collectors
-
-import com.google.dart.compiler.backend.js.ast.JsVisitorWithContextImpl
-import com.google.dart.compiler.backend.js.ast.JsName
-import com.google.dart.compiler.backend.js.ast.JsNameRef
-import com.google.dart.compiler.backend.js.ast.JsContext
-import org.jetbrains.kotlin.js.inline.util.IdentitySet
-
-class ReferenceNameCollector : JsVisitorWithContextImpl() {
-    private val referenceSet = IdentitySet<JsName>()
-
-    val references: List<JsName>
-        get() = referenceSet.toList()
-
-    override fun endVisit(x: JsNameRef, ctx: JsContext<*>) {
-        val name = x.name
-        if (name != null) {
-            referenceSet.add(name)
-        }
-    }
-}

js/js.inliner/src/org/jetbrains/kotlin/js/inline/util/namingUtils.kt

@@ -59,7 +59,7 @@ fun renameLocalNames(
         context: NamingContext,
         function: JsFunction
 ) {
-    for (name in collectLocalNames(function)) {
+    for (name in collectDefinedNames(function.body)) {
         val freshName = context.getFreshName(name)
         context.replaceName(name, freshName.makeRef())
     }