From b5ab8f848876eee3d347bdf4ca0184e594677b3b Mon Sep 17 00:00:00 2001 From: Alexey Andreev Date: Thu, 17 Mar 2016 13:46:33 +0300 Subject: [PATCH] Make JS optimizations respect nested functions --- .../js/inline/clean/RedundantLabelRemoval.kt | 2 + ...RedundantVariableDeclarationElimination.kt | 8 ++ .../clean/TemporaryAssignmentElimination.kt | 12 ++ .../clean/TemporaryVariableElimination.kt | 6 + .../removeUnusedLocalFunctionDeclarations.kt | 4 +- .../kotlin/js/inline/util/collectUtils.kt | 103 +++++++++++++++--- .../inline/util/collectors/NameCollector.kt | 50 --------- .../util/collectors/ReferenceNameCollector.kt | 37 ------- .../kotlin/js/inline/util/namingUtils.kt | 2 +- 9 files changed, 119 insertions(+), 105 deletions(-) delete mode 100644 js/js.inliner/src/org/jetbrains/kotlin/js/inline/util/collectors/NameCollector.kt delete mode 100644 js/js.inliner/src/org/jetbrains/kotlin/js/inline/util/collectors/ReferenceNameCollector.kt diff --git a/js/js.inliner/src/org/jetbrains/kotlin/js/inline/clean/RedundantLabelRemoval.kt b/js/js.inliner/src/org/jetbrains/kotlin/js/inline/clean/RedundantLabelRemoval.kt index 3f62390b180..d7a4161729c 100644 --- a/js/js.inliner/src/org/jetbrains/kotlin/js/inline/clean/RedundantLabelRemoval.kt +++ b/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) } diff --git a/js/js.inliner/src/org/jetbrains/kotlin/js/inline/clean/RedundantVariableDeclarationElimination.kt b/js/js.inliner/src/org/jetbrains/kotlin/js/inline/clean/RedundantVariableDeclarationElimination.kt index bed415a0e8a..999f3b2b97c 100644 --- a/js/js.inliner/src/org/jetbrains/kotlin/js/inline/clean/RedundantVariableDeclarationElimination.kt +++ b/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() @@ -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) } } diff --git a/js/js.inliner/src/org/jetbrains/kotlin/js/inline/clean/TemporaryAssignmentElimination.kt b/js/js.inliner/src/org/jetbrains/kotlin/js/inline/clean/TemporaryAssignmentElimination.kt index e422cda992a..d67ac7b471a 100644 --- a/js/js.inliner/src/org/jetbrains/kotlin/js/inline/clean/TemporaryAssignmentElimination.kt +++ b/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) } 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 0c3d10b6753..fcd995eb41f 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,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) } diff --git a/js/js.inliner/src/org/jetbrains/kotlin/js/inline/clean/removeUnusedLocalFunctionDeclarations.kt b/js/js.inliner/src/org/jetbrains/kotlin/js/inline/clean/removeUnusedLocalFunctionDeclarations.kt index 46f767354a4..1af50764715 100644 --- a/js/js.inliner/src/org/jetbrains/kotlin/js/inline/clean/removeUnusedLocalFunctionDeclarations.kt +++ b/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) } diff --git a/js/js.inliner/src/org/jetbrains/kotlin/js/inline/util/collectUtils.kt b/js/js.inliner/src/org/jetbrains/kotlin/js/inline/util/collectUtils.kt index 9b78d509533..5ff65cc01ae 100644 --- a/js/js.inliner/src/org/jetbrains/kotlin/js/inline/util/collectUtils.kt +++ b/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 = - collectReferencesInside(scope).filter { it.staticRef is JsFunction } + collectReferencedNames(scope).filter { it.staticRef is JsFunction } -fun collectReferencesInside(scope: JsNode): List { - return with(ReferenceNameCollector()) { - accept(scope) - references - } +fun collectReferencedNames(scope: JsNode): Set { + val references = IdentitySet() + + 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 { - val functionScope = function.scope +fun collectUsedNames(scope: JsNode): Set { + val references = IdentitySet() - return with(NameCollector(functionScope)) { - accept(function.body) - names.values.toList() - } + 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 && 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 { + val names: MutableMap = 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 { val collector = PropertyCollector() collector.accept(scope) diff --git a/js/js.inliner/src/org/jetbrains/kotlin/js/inline/util/collectors/NameCollector.kt b/js/js.inliner/src/org/jetbrains/kotlin/js/inline/util/collectors/NameCollector.kt deleted file mode 100644 index 50f2d00f822..00000000000 --- a/js/js.inliner/src/org/jetbrains/kotlin/js/inline/util/collectors/NameCollector.kt +++ /dev/null @@ -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 = 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) - } -} diff --git a/js/js.inliner/src/org/jetbrains/kotlin/js/inline/util/collectors/ReferenceNameCollector.kt b/js/js.inliner/src/org/jetbrains/kotlin/js/inline/util/collectors/ReferenceNameCollector.kt deleted file mode 100644 index 6a72de3abb5..00000000000 --- a/js/js.inliner/src/org/jetbrains/kotlin/js/inline/util/collectors/ReferenceNameCollector.kt +++ /dev/null @@ -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() - - val references: List - get() = referenceSet.toList() - - override fun endVisit(x: JsNameRef, ctx: JsContext<*>) { - val name = x.name - if (name != null) { - referenceSet.add(name) - } - } -} diff --git a/js/js.inliner/src/org/jetbrains/kotlin/js/inline/util/namingUtils.kt b/js/js.inliner/src/org/jetbrains/kotlin/js/inline/util/namingUtils.kt index eafcf0453f0..32fdc1da9b8 100644 --- a/js/js.inliner/src/org/jetbrains/kotlin/js/inline/util/namingUtils.kt +++ b/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()) }