From a1649e9e4db9d66fda268c61e398cbe1264c13fd Mon Sep 17 00:00:00 2001 From: Anton Bannykh Date: Mon, 17 Dec 2018 18:58:10 +0300 Subject: [PATCH] JS: fix import cleaner The tag generated during the source file translation could differ from the tag generated based on the import statement (e.g. "intrinsic:..." during generation becomes "'Kotlin'...." later on). This means that the same function imported from the start and imported during inlining has different tags. Which makes duplicate import removal trickier. The best solution would be to make sure the tag generation algorithms are consistent. Current solution: rewrite the tags in terms of the Inliner tag generation algorithm right after the source file translation. Also it seems that some `var` statements we treated as imports, even though they were not. Supported this behavior for now. TODO: Get rid of these workaround --- .../kotlin/js/inline/InliningScope.kt | 17 +----- .../js/inline/clean/removeUnusedImports.kt | 54 ++++++++++++++++--- .../kotlin/js/inline/util/collectUtils.kt | 6 +++ .../kotlin/js/facade/K2JSTranslator.kt | 3 -- .../js/translate/general/Translation.java | 1 + .../translate/general/normalizeImportTags.kt | 36 +++++++++++++ 6 files changed, 90 insertions(+), 27 deletions(-) create mode 100644 js/js.translator/src/org/jetbrains/kotlin/js/translate/general/normalizeImportTags.kt diff --git a/js/js.inliner/src/org/jetbrains/kotlin/js/inline/InliningScope.kt b/js/js.inliner/src/org/jetbrains/kotlin/js/inline/InliningScope.kt index 44919856463..19e97891e69 100644 --- a/js/js.inliner/src/org/jetbrains/kotlin/js/inline/InliningScope.kt +++ b/js/js.inliner/src/org/jetbrains/kotlin/js/inline/InliningScope.kt @@ -6,7 +6,6 @@ package org.jetbrains.kotlin.js.inline import org.jetbrains.kotlin.js.backend.ast.* -import org.jetbrains.kotlin.js.backend.ast.metadata.imported import org.jetbrains.kotlin.js.backend.ast.metadata.localAlias import org.jetbrains.kotlin.js.backend.ast.metadata.staticRef import org.jetbrains.kotlin.js.inline.clean.removeUnusedFunctionDefinitions @@ -159,21 +158,7 @@ class ProgramFragmentInliningScope( simplifyWrappedFunctions(block) removeUnusedFunctionDefinitions(block, collectNamedFunctions(block)) - - // TODO simplify - val usedImports = removeUnusedImports(block) - fragment.nameBindings.filter { - !it.name.imported || it.name in usedImports - }.let { - fragment.nameBindings.clear() - fragment.nameBindings.addAll(it) - } - val existingTags = fragment.nameBindings.map { it.key }.toSet() - val newImports = fragment.imports.filter { (k, _) -> k in existingTags } - fragment.imports.clear() - for ((k, v) in newImports) { - fragment.imports[k] = v - } + removeUnusedImports(fragment) } // TODO !!!!! This localAlias thing will get copied, inlined, and lost during IC =( diff --git a/js/js.inliner/src/org/jetbrains/kotlin/js/inline/clean/removeUnusedImports.kt b/js/js.inliner/src/org/jetbrains/kotlin/js/inline/clean/removeUnusedImports.kt index 2a085ead7a8..a2aae51f9fd 100644 --- a/js/js.inliner/src/org/jetbrains/kotlin/js/inline/clean/removeUnusedImports.kt +++ b/js/js.inliner/src/org/jetbrains/kotlin/js/inline/clean/removeUnusedImports.kt @@ -20,25 +20,63 @@ import org.jetbrains.kotlin.js.backend.ast.* import org.jetbrains.kotlin.js.backend.ast.metadata.coroutineMetadata import org.jetbrains.kotlin.js.backend.ast.metadata.imported -// Returns used imports -fun removeUnusedImports(root: JsNode): Set { - val collector = UsedImportsCollector() + +fun removeUnusedImports(fragment: JsProgramFragment) { + val usedImports = mutableSetOf() + + with(fragment) { + inlinedFunctionWrappers.values.forEach { + collectUsedImports(it, usedImports) + } + collectUsedImports(declarationBlock, usedImports) + collectUsedImports(initializerBlock, usedImports) + collectUsedImports(exportBlock, usedImports) + } + + fragment.nameBindings.retainAll { !it.name.imported || it.name in usedImports } + + val existingTags = fragment.nameBindings.map { it.key }.toSet() + + fragment.imports.entries.retainAll { (k, _) -> k in existingTags } +} + +private fun collectUsedImports(root: JsNode, to: MutableSet): Set { + val collector = UsedImportsCollector(to) root.accept(collector) + + // See StaticContext.getVariableForPropertyMetadata + val removedPseudoImports = mutableSetOf() NodeRemover(JsVars::class.java) { statement -> if (statement.vars.size == 1) { val name = statement.vars[0].name - name.imported && name !in collector.usedImports - } - else { + (name.imported && name !in collector.usedImports).also { + if (it) removedPseudoImports += name + } + } else { false } }.accept(root) + collector.pseudoImports.forEach { + if (it.name !in removedPseudoImports) { + it.initExpression.accept(collector) + } + } + return collector.usedImports } -private class UsedImportsCollector : RecursiveJsVisitor() { - val usedImports = mutableSetOf() +private class UsedImportsCollector(val usedImports: MutableSet) : RecursiveJsVisitor() { + + val pseudoImports = mutableListOf() + + override fun visit(x: JsVars.JsVar) { + if (x.name.imported) { + pseudoImports += x + } else { + super.visit(x) + } + } override fun visitNameRef(nameRef: JsNameRef) { val name = nameRef.name 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 fdced6b7099..8cf4f56c1e2 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 @@ -395,6 +395,12 @@ fun extractImportTag(jsVar: JsVars.JsVar): String? { return if (extractImportTagImpl(initExpression, sb)) sb.toString() else null } +// Note! Doesn't handle Long's +fun extractImportTag(initExpression: JsExpression): String? { + val sb = StringBuilder() + return if (extractImportTagImpl(initExpression, sb)) sb.toString() else null +} + private fun extractImportTagImpl(expression: JsExpression, sb: StringBuilder): Boolean { when (expression) { is JsNameRef -> { diff --git a/js/js.translator/src/org/jetbrains/kotlin/js/facade/K2JSTranslator.kt b/js/js.translator/src/org/jetbrains/kotlin/js/facade/K2JSTranslator.kt index e80b788f594..b793ea6fa68 100644 --- a/js/js.translator/src/org/jetbrains/kotlin/js/facade/K2JSTranslator.kt +++ b/js/js.translator/src/org/jetbrains/kotlin/js/facade/K2JSTranslator.kt @@ -43,11 +43,8 @@ import java.io.IOException import java.util.ArrayList import org.jetbrains.kotlin.diagnostics.DiagnosticUtils.hasError -import org.jetbrains.kotlin.js.backend.ast.JsGlobalBlock -import org.jetbrains.kotlin.js.backend.ast.JsProgramFragment import org.jetbrains.kotlin.js.coroutine.transformCoroutines import org.jetbrains.kotlin.js.translate.general.AstGenerationResult -import org.jetbrains.kotlin.metadata.js.JsProtoBuf import org.jetbrains.kotlin.resolve.BindingTrace import org.jetbrains.kotlin.serialization.js.ast.JsAstProtoBuf diff --git a/js/js.translator/src/org/jetbrains/kotlin/js/translate/general/Translation.java b/js/js.translator/src/org/jetbrains/kotlin/js/translate/general/Translation.java index 01d6230a3fb..b2480b5d26a 100644 --- a/js/js.translator/src/org/jetbrains/kotlin/js/translate/general/Translation.java +++ b/js/js.translator/src/org/jetbrains/kotlin/js/translate/general/Translation.java @@ -339,6 +339,7 @@ public final class Translation { for (String tag : staticContext.getInlineFunctionTags()) { inlineFunctionTagMap.put(tag, unit); } + NormalizeImportTagsKt.normalizeImportTags(fragment); fragment.setTests(mayBeGenerateTests(context, file, fileMemberScope)); fragment.setMainFunction(maybeGenerateCallToMain(context, config, moduleDescriptor, fileMemberScope, mainCallParameters)); diff --git a/js/js.translator/src/org/jetbrains/kotlin/js/translate/general/normalizeImportTags.kt b/js/js.translator/src/org/jetbrains/kotlin/js/translate/general/normalizeImportTags.kt new file mode 100644 index 00000000000..ed60ec97c66 --- /dev/null +++ b/js/js.translator/src/org/jetbrains/kotlin/js/translate/general/normalizeImportTags.kt @@ -0,0 +1,36 @@ +/* + * Copyright 2010-2018 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license + * that can be found in the license/LICENSE.txt file. + */ + +package org.jetbrains.kotlin.js.translate.general + +import org.jetbrains.kotlin.js.backend.ast.JsExpression +import org.jetbrains.kotlin.js.backend.ast.JsNameBinding +import org.jetbrains.kotlin.js.backend.ast.JsProgramFragment +import org.jetbrains.kotlin.js.inline.util.extractImportTag + +// TODO this is a hack for `intrinsic:` tags +fun JsProgramFragment.normalizeImportTags() { + + val newImports = mutableMapOf() + val replacements = mutableMapOf() + + imports.entries.retainAll { (tag, statement) -> + extractImportTag(statement)?.let { newTag -> + if (newTag != tag) { + newImports[newTag] = statement + replacements[tag] = newTag + } + newTag == tag + } ?: true + } + + imports += newImports + + nameBindings.replaceAll { binding -> + replacements[binding.key]?.let { + JsNameBinding(it, binding.name) + } ?: binding + } +} \ No newline at end of file