From 13844bd08ecae8261b466aa181f40aaf771e0e05 Mon Sep 17 00:00:00 2001 From: Anton Bannykh Date: Tue, 18 Dec 2018 18:32:57 +0300 Subject: [PATCH] JS: FunctionReader cleanup --- .../kotlin/js/inline/FunctionReader.kt | 30 +++++-------------- .../jetbrains/kotlin/js/inline/InlinerImpl.kt | 4 --- .../kotlin/js/inline/InliningScope.kt | 8 +---- .../jetbrains/kotlin/js/inline/JsInliner.kt | 1 - 4 files changed, 8 insertions(+), 35 deletions(-) diff --git a/js/js.inliner/src/org/jetbrains/kotlin/js/inline/FunctionReader.kt b/js/js.inliner/src/org/jetbrains/kotlin/js/inline/FunctionReader.kt index 3ec389ec98b..111653df67c 100644 --- a/js/js.inliner/src/org/jetbrains/kotlin/js/inline/FunctionReader.kt +++ b/js/js.inliner/src/org/jetbrains/kotlin/js/inline/FunctionReader.kt @@ -83,8 +83,8 @@ class FunctionReader( val offsetToSourceMapping by lazy(offsetToSourceMappingProvider) val wrapFunctionRegex = specialFunctions.entries - .filter { (_, v) -> v == SpecialFunction.WRAP_FUNCTION } // TODO This is a hack! Investigate duplicates! - .map { Regex("\\s*${it.key}\\s*\\(\\s*").toPattern() } + .singleOrNull { (_, v) -> v == SpecialFunction.WRAP_FUNCTION }?.key + ?.let { Regex("\\s*$it\\s*\\(\\s*").toPattern() } } private val moduleNameToInfo by lazy { @@ -224,11 +224,9 @@ class FunctionReader( return null } - // TODO move renamings to a proper place private fun readFunctionFromSource(descriptor: CallableDescriptor, info: ModuleInfo): FunctionWithWrapper? { val source = info.fileContent var tag = Namer.getFunctionTag(descriptor, config) -// val tagForModule = tag var index = source.indexOf(tag) // Hack for compatibility with old versions of stdlib @@ -247,30 +245,21 @@ class FunctionReader( } val sourcePart = ShallowSubSequence(source, offset, source.length) - var isWrapped = false - for (regex in info.wrapFunctionRegex) { - val wrapFunctionMatcher = regex.matcher(sourcePart) - isWrapped = wrapFunctionMatcher.lookingAt() == true - if (isWrapped) { - offset += wrapFunctionMatcher!!.end() - break - } + val wrapFunctionMatcher = info.wrapFunctionRegex?.matcher(sourcePart) + val isWrapped = wrapFunctionMatcher?.lookingAt() == true + if (isWrapped) { + offset += wrapFunctionMatcher!!.end() } val position = info.offsetToSourceMapping[offset] val jsScope = JsRootScope(JsProgram()) - val functionExpr = try { - parseFunction(source, info.filePath, position, offset, ThrowExceptionOnErrorReporter, jsScope) ?: return null - } catch (t: Throwable) { - throw Error("Exception while reading function '$tag' from ${info.filePath}", t) - } + val functionExpr = parseFunction(source, info.filePath, position, offset, ThrowExceptionOnErrorReporter, jsScope) ?: return null functionExpr.fixForwardNameReferences() val (function, wrapper) = if (isWrapped) { InlineMetadata.decomposeWrapper(functionExpr) ?: return null } else { FunctionWithWrapper(functionExpr, null) } -// val moduleReference = moduleNameMap[tagForModule]?.deepCopy() ?: currentModuleName.makeRef() val wrapperStatements = wrapper?.statements?.filter { it !is JsReturn } val sourceMap = info.sourceMap @@ -283,14 +272,9 @@ class FunctionReader( } val allDefinedNames = collectDefinedNamesInAllScopes(function) -// val replacements = hashMapOf(info.moduleVariable to moduleReference, -// info.kotlinVariable to Namer.kotlinObject()) -// replaceExternalNames(function, replacements, allDefinedNames) -// wrapperStatements?.forEach { replaceExternalNames(it, replacements, allDefinedNames) } function.markInlineArguments(descriptor) markDefaultParams(function) - // TODO maybe move elsewhere? markSpecialFunctions(function, allDefinedNames, info, jsScope) val namesWithoutSideEffects = wrapperStatements.orEmpty().asSequence() diff --git a/js/js.inliner/src/org/jetbrains/kotlin/js/inline/InlinerImpl.kt b/js/js.inliner/src/org/jetbrains/kotlin/js/inline/InlinerImpl.kt index 8ec7926e5a8..bca3dc6d848 100644 --- a/js/js.inliner/src/org/jetbrains/kotlin/js/inline/InlinerImpl.kt +++ b/js/js.inliner/src/org/jetbrains/kotlin/js/inline/InlinerImpl.kt @@ -15,7 +15,6 @@ import org.jetbrains.kotlin.js.inline.util.refreshLabelNames import org.jetbrains.kotlin.js.translate.expression.InlineMetadata import org.jetbrains.kotlin.js.translate.utils.JsAstUtils -// TODO stateless? class InlinerImpl( val jsInliner: JsInliner, val scope: InliningScope @@ -89,8 +88,6 @@ class InlinerImpl( } } - // TODO This could be extracted into a separate pass. - // TODO Don't forget to run it on the freshly inlined code! override fun doAcceptStatementList(statements: MutableList) { var i = 0 @@ -114,7 +111,6 @@ class InlinerImpl( private fun patchReturnsFromSecondaryConstructor(function: JsFunction) { // Support non-local return from secondary constructor // Returns from secondary constructors should return `$this` object. - // TODO This seems brittle function.forcedReturnVariable?.let { returnVariable -> function.body.accept(object : RecursiveJsVisitor() { override fun visitReturn(x: JsReturn) { 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 26288da7df2..d76b8c5cd62 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 @@ -45,8 +45,6 @@ sealed class InliningScope { fun importFunctionDefinition(definition: InlineFunctionDefinition): JsFunction { // Apparently we should avoid this trick when we implement fair support for crossinline // That's because crossinline lambdas inline into the declaration block and specialize those. - - // TODO cache the function itself instead val result = computeIfAbsent(definition.tag, definition.fn.function) { val newReplacements = HashMap() @@ -146,9 +144,7 @@ class ProgramFragmentInliningScope( private val additionalDeclarations = mutableListOf() override fun update() { - // TODO fix the order - // TODO this probably will be replaced with a special tag -> block map for the imported stuff, so that we can merge same imports. - // TODO in that case this method will become obsolete + // TODO fix the order? fragment.declarationBlock.statements.addAll(0, additionalDeclarations) // post-processing @@ -161,7 +157,6 @@ class ProgramFragmentInliningScope( removeUnusedImports(fragment) } - // TODO !!!!! This localAlias thing will get copied, inlined, and lost during IC =( override fun hasImport(name: JsName, tag: String): JsName? { return name.localAlias?.let {(name, tag) -> if (tag != null) { @@ -217,7 +212,6 @@ class ProgramFragmentInliningScope( private fun addInlinedModule(module: JsImportedModule): JsName { return existingModules.computeIfAbsent(module.key) { // Copy so that the Merger.kt doesn't operate on the same instance in different fragments. - // TODO What about nameBindings? JsImportedModule(module.externalName, module.internalName, module.plainReference).also { fragment.importedModules.add(it) } diff --git a/js/js.inliner/src/org/jetbrains/kotlin/js/inline/JsInliner.kt b/js/js.inliner/src/org/jetbrains/kotlin/js/inline/JsInliner.kt index f7c937110a6..4aa792f3b87 100644 --- a/js/js.inliner/src/org/jetbrains/kotlin/js/inline/JsInliner.kt +++ b/js/js.inliner/src/org/jetbrains/kotlin/js/inline/JsInliner.kt @@ -82,7 +82,6 @@ class JsInliner( InlinerImpl(this@JsInliner, this).accept(node) } - // TODO a lot of code... Probably a bad sign fun inline(scope: InliningScope, call: JsInvocation, currentStatement: JsStatement?): InlineableResult { val definition = functionContext.getFunctionDefinition(call, scope)