diff --git a/js/js.inliner/src/org/jetbrains/kotlin/js/coroutine/CoroutineTransformer.kt b/js/js.inliner/src/org/jetbrains/kotlin/js/coroutine/CoroutineTransformer.kt index 674836aaab2..fb0e34287a8 100644 --- a/js/js.inliner/src/org/jetbrains/kotlin/js/coroutine/CoroutineTransformer.kt +++ b/js/js.inliner/src/org/jetbrains/kotlin/js/coroutine/CoroutineTransformer.kt @@ -19,6 +19,8 @@ package org.jetbrains.kotlin.js.coroutine import org.jetbrains.kotlin.js.backend.ast.* import org.jetbrains.kotlin.js.backend.ast.metadata.coroutineMetadata import org.jetbrains.kotlin.js.backend.ast.metadata.isInlineableCoroutineBody +import org.jetbrains.kotlin.js.inline.InlineSuspendFunctionSplitter +import org.jetbrains.kotlin.js.inline.ProgramFragmentInliningScope import org.jetbrains.kotlin.js.translate.declaration.transformCoroutineMetadataToSpecialFunctions import org.jetbrains.kotlin.js.translate.expression.InlineMetadata import org.jetbrains.kotlin.js.translate.utils.JsAstUtils @@ -70,8 +72,9 @@ class CoroutineTransformer : JsVisitorWithContextImpl() { fun transformCoroutines(fragments: List) { val coroutineTransformer = CoroutineTransformer() for (fragment in fragments) { - fragment.inlinedFunctionWrappers.values.forEach { coroutineTransformer.accept(it) } - coroutineTransformer.accept(fragment.declarationBlock) - coroutineTransformer.accept(fragment.initializerBlock) + val scope = ProgramFragmentInliningScope(fragment) + InlineSuspendFunctionSplitter(scope).accept(scope.allCode) + coroutineTransformer.accept(scope.allCode) + scope.update() } } \ No newline at end of file diff --git a/js/js.inliner/src/org/jetbrains/kotlin/js/inline/DummyAccessorInvocationTransformer.kt b/js/js.inliner/src/org/jetbrains/kotlin/js/inline/DummyAccessorInvocationTransformer.kt deleted file mode 100644 index dfa44e18eb9..00000000000 --- a/js/js.inliner/src/org/jetbrains/kotlin/js/inline/DummyAccessorInvocationTransformer.kt +++ /dev/null @@ -1,68 +0,0 @@ -/* - * Copyright 2010-2017 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 - -import org.jetbrains.kotlin.descriptors.PropertyGetterDescriptor -import org.jetbrains.kotlin.descriptors.PropertySetterDescriptor -import org.jetbrains.kotlin.js.backend.ast.* -import org.jetbrains.kotlin.js.backend.ast.metadata.descriptor -import org.jetbrains.kotlin.js.backend.ast.metadata.inlineStrategy -import org.jetbrains.kotlin.js.backend.ast.metadata.psiElement - -class DummyAccessorInvocationTransformer : JsVisitorWithContextImpl() { - override fun endVisit(x: JsNameRef, ctx: JsContext) { - super.endVisit(x, ctx) - val dummy = tryCreatePropertyGetterInvocation(x) - if (dummy != null) { - ctx.replaceMe(dummy) - } - } - - override fun endVisit(x: JsBinaryOperation, ctx: JsContext) { - super.endVisit(x, ctx) - val dummy = tryCreatePropertySetterInvocation(x) - if (dummy != null) { - ctx.replaceMe(dummy) - } - } - - private fun tryCreatePropertyGetterInvocation(x: JsNameRef): JsInvocation? { - if (x.inlineStrategy != null && x.descriptor is PropertyGetterDescriptor) { - val dummyInvocation = JsInvocation(x) - copyInlineMetadata(x, dummyInvocation) - return dummyInvocation - } - return null - } - - private fun tryCreatePropertySetterInvocation(x: JsBinaryOperation): JsInvocation? { - if (!x.operator.isAssignment || x.arg1 !is JsNameRef) return null - val name = x.arg1 as JsNameRef - if (name.inlineStrategy != null && name.descriptor is PropertySetterDescriptor) { - val dummyInvocation = JsInvocation(name, x.arg2) - copyInlineMetadata(name, dummyInvocation) - return dummyInvocation - } - return null - } - - private fun copyInlineMetadata(from: JsNameRef, to: JsInvocation) { - to.inlineStrategy = from.inlineStrategy - to.descriptor = from.descriptor - to.psiElement = from.psiElement - } -} diff --git a/js/js.inliner/src/org/jetbrains/kotlin/js/inline/InlinerCycleReporter.kt b/js/js.inliner/src/org/jetbrains/kotlin/js/inline/InlinerCycleReporter.kt index 6a7e033f8fe..31c5c20b30d 100644 --- a/js/js.inliner/src/org/jetbrains/kotlin/js/inline/InlinerCycleReporter.kt +++ b/js/js.inliner/src/org/jetbrains/kotlin/js/inline/InlinerCycleReporter.kt @@ -14,7 +14,6 @@ import org.jetbrains.kotlin.js.backend.ast.metadata.inlineStrategy import org.jetbrains.kotlin.js.backend.ast.metadata.psiElement import org.jetbrains.kotlin.js.inline.context.FunctionContext import org.jetbrains.kotlin.js.inline.util.FunctionWithWrapper -import org.jetbrains.kotlin.js.inline.util.IdentitySet import org.jetbrains.kotlin.resolve.inline.InlineStrategy import java.util.* @@ -30,18 +29,19 @@ class InlinerCycleReporter( private val functionContext: FunctionContext ) { - private val processedFunctions = IdentitySet() - private val inProcessFunctions = IdentitySet() + private enum class VisitedState { IN_PROCESS, PROCESSED } + + private val functionVisitingState = mutableMapOf() + // these are needed for error reporting, when inliner detects cycle private val namedFunctionsStack = Stack() - private val inlineCallInfos = LinkedList() - - // TODO This looks like a hack - val currentNamedFunction: JsFunction? + private val currentNamedFunction: JsFunction? get() = if (namedFunctionsStack.empty()) null else namedFunctionsStack.peek() + private val inlineCallInfos = LinkedList() + // Puts `function` on the `namedFunctionsStack` for inline call cycles reporting fun withFunction(function: JsFunction, body: () -> T): T { if (function in functionContext.functionsByFunctionNodes.keys) { namedFunctionsStack.push(function) @@ -49,34 +49,41 @@ class InlinerCycleReporter( val result = body() - if (!namedFunctionsStack.empty() && namedFunctionsStack.peek() == function) { + if (currentNamedFunction == function) { namedFunctionsStack.pop() } return result } - fun withInlineFunctionDefinition(function: JsFunction, body: () -> T): T { - assert(!inProcessFunctions.contains(function)) { "Inliner has revisited function" } - inProcessFunctions.add(function) + fun processInlineFunction(definition: FunctionWithWrapper, call: JsInvocation?, doProcess: () -> Unit) { - val result = withFunction(function, body) + when (functionVisitingState[definition.function]) { + VisitedState.IN_PROCESS -> { + reportInlineCycle(call, definition.function) + return + } + VisitedState.PROCESSED -> return + } - processedFunctions.add(function) + val function = definition.function - assert(function in inProcessFunctions) - inProcessFunctions.remove(function) + functionVisitingState[function] = VisitedState.IN_PROCESS + + val result = withFunction(function, doProcess) + + functionVisitingState[function] = VisitedState.PROCESSED return result } - fun withInlining(call: JsInvocation, body: () -> T): T { + fun inlineCall(call: JsInvocation, doInline: () -> T): T { currentNamedFunction?.let { inlineCallInfos.add(JsCallInfo(call, it)) } - val result = body() + val result = doInline() if (!inlineCallInfos.isEmpty()) { if (inlineCallInfos.last.call == call) { @@ -87,17 +94,6 @@ class InlinerCycleReporter( return result } - // Return true iff the definition should be visited by the inliner - fun shouldProcess(definition: FunctionWithWrapper, call: JsInvocation?): Boolean { - if (definition.function in inProcessFunctions) { - reportInlineCycle(call, definition.function) - } else if (definition.function !in processedFunctions) { - return true - } - - return false - } - private fun reportInlineCycle(call: JsInvocation?, calledFunction: JsFunction) { call?.inlineStrategy = InlineStrategy.NOT_INLINE val it = inlineCallInfos.descendingIterator() 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 bca3dc6d848..7d822637fbc 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 @@ -5,9 +5,13 @@ package org.jetbrains.kotlin.js.inline +import org.jetbrains.kotlin.descriptors.PropertyGetterDescriptor +import org.jetbrains.kotlin.descriptors.PropertySetterDescriptor import org.jetbrains.kotlin.js.backend.ast.* +import org.jetbrains.kotlin.js.backend.ast.metadata.descriptor import org.jetbrains.kotlin.js.backend.ast.metadata.forcedReturnVariable import org.jetbrains.kotlin.js.backend.ast.metadata.inlineStrategy +import org.jetbrains.kotlin.js.backend.ast.metadata.psiElement import org.jetbrains.kotlin.js.inline.clean.FunctionPostProcessor import org.jetbrains.kotlin.js.inline.clean.removeUnusedLocalFunctionDeclarations import org.jetbrains.kotlin.js.inline.util.extractFunction @@ -72,6 +76,18 @@ class InlinerImpl( } + override fun endVisit(x: JsNameRef, ctx: JsContext) { + tryCreatePropertyGetterInvocation(x)?.let { + endVisit(it, ctx) + } + } + + override fun endVisit(x: JsBinaryOperation, ctx: JsContext) { + tryCreatePropertySetterInvocation(x)?.let { + endVisit(it, ctx) + } + } + override fun endVisit(call: JsInvocation, ctx: JsContext) { if (hasToBeInlined(call)) { val (inlineableBody, resultExpression) = jsInliner.inline(scope, call, lastStatementLevelContext.currentNode) @@ -92,9 +108,7 @@ class InlinerImpl( var i = 0 while (i < statements.size) { - val additionalStatements = ExpressionDecomposer.preserveEvaluationOrder(statements[i]) { node -> - node is JsInvocation && hasToBeInlined(node) - } + val additionalStatements = ExpressionDecomposer.preserveEvaluationOrder(statements[i], ::hasToBeInlined) statements.addAll(i, additionalStatements) i += additionalStatements.size + 1 } @@ -102,6 +116,16 @@ class InlinerImpl( super.doAcceptStatementList(statements) } + private fun hasToBeInlined(node: JsNode): Boolean { + return when (node) { + is JsInvocation -> hasToBeInlined(node) + is JsNameRef -> node.inlineStrategy != null && tryCreatePropertyGetterInvocation(node)?.let { hasToBeInlined(it) } ?: false + is JsBinaryOperation -> node.operator.isAssignment && node.arg1?.let { left -> + left is JsNameRef && left.inlineStrategy != null && tryCreatePropertySetterInvocation(node)?.let { hasToBeInlined(it) } ?: false + } ?: false + else -> false + } + } private fun hasToBeInlined(call: JsInvocation): Boolean { val strategy = call.inlineStrategy @@ -119,4 +143,30 @@ class InlinerImpl( }) } } + + private fun tryCreatePropertyGetterInvocation(x: JsNameRef): JsInvocation? { + if (x.inlineStrategy != null && x.descriptor is PropertyGetterDescriptor) { + val dummyInvocation = JsInvocation(x) + copyInlineMetadata(x, dummyInvocation) + return dummyInvocation + } + return null + } + + private fun tryCreatePropertySetterInvocation(x: JsBinaryOperation): JsInvocation? { + if (!x.operator.isAssignment || x.arg1 !is JsNameRef) return null + val name = x.arg1 as JsNameRef + if (name.inlineStrategy != null && name.descriptor is PropertySetterDescriptor) { + val dummyInvocation = JsInvocation(name, x.arg2) + copyInlineMetadata(name, dummyInvocation) + return dummyInvocation + } + return null + } + + private fun copyInlineMetadata(from: JsNameRef, to: JsInvocation) { + to.inlineStrategy = from.inlineStrategy + to.descriptor = from.descriptor + to.psiElement = from.psiElement + } } \ No newline at end of file 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 d22ecdf6dd0..3f6c3cbf73d 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 @@ -137,6 +137,17 @@ class ProgramFragmentInliningScope( override val fragment: JsProgramFragment ) : InliningScope() { + val allCode: JsBlock + get() = JsBlock( + JsBlock(fragment.inlinedFunctionWrappers.values.toList()), + fragment.declarationBlock, + fragment.exportBlock, + JsExpressionStatement(JsFunction(JsDynamicScope, fragment.initializerBlock, "")) + ).also { block -> + fragment.tests?.let { block.statements.add(it) } + fragment.mainFunction?.let { block.statements.add(it) } + } + private val existingModules = fragment.importedModules.associateTo(mutableMapOf()) { it.key to it } private val existingBindings = fragment.nameBindings.associateTo(mutableMapOf()) { it.key to it.name } @@ -148,18 +159,9 @@ class ProgramFragmentInliningScope( fragment.declarationBlock.statements.addAll(0, additionalDeclarations) // post-processing - val block = JsBlock( - JsBlock(fragment.inlinedFunctionWrappers.values.toList()), - fragment.declarationBlock, - fragment.initializerBlock, - fragment.exportBlock - ) - fragment.tests?.let { block.statements.add(it) } - fragment.mainFunction?.let { block.statements.add(it) } - - simplifyWrappedFunctions(block) - removeUnusedFunctionDefinitions(block, collectNamedFunctions(block)) - removeUnusedImports(fragment) + simplifyWrappedFunctions(allCode) + removeUnusedFunctionDefinitions(allCode, collectNamedFunctions(allCode)) + removeUnusedImports(fragment, allCode) } override fun hasImport(name: JsName, tag: String): JsName? { 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 4aa792f3b87..b403f14ab49 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 @@ -21,16 +21,6 @@ class JsInliner( val translationResult: AstGenerationResult ) { - init { - // TODO Isn't there a better way to achieve this? Also there is a bug with private inline properties - DummyAccessorInvocationTransformer().let { - for (fragment in translationResult.newFragments) { - it.accept(fragment.declarationBlock) - it.accept(fragment.initializerBlock) - } - } - } - val functionContext = FunctionContext(this) val cycleReporter = InlinerCycleReporter(trace, functionContext) @@ -44,36 +34,21 @@ class JsInliner( fun process(fragment: JsProgramFragment) { val fragmentScope = functionContext.scopeForFragment(fragment) ?: return - // TODO any way and/or need to visit everything inside the fragment? - fragmentScope.process(fragment.declarationBlock) - - // TODO Atm it's placed after inliner in order not to perform the body inlining twice. Is that OK? - // Ideally it could be moved to the coroutine transformers. The info regarding which inline function wrappers have been imported - // on top level should be persisted for that sake. Also it going to be needed in order to avoid duplicate code. - InlineSuspendFunctionSplitter(fragmentScope).accept(fragment.declarationBlock) - - // Mostly for the sake of post-processor - // TODO are inline function marked with @Test possible? - fragment.tests?.let { fragmentScope.process(it) } - - // TODO wrap in a function in order to do the post-processing - fragmentScope.process(fragment.initializerBlock) + fragmentScope.process(fragmentScope.allCode) fragmentScope.update() } fun process(inlineFn: InlineFunctionDefinition, call: JsInvocation?, containingScope: InliningScope) { - if (cycleReporter.shouldProcess(inlineFn.fn, call)) { + cycleReporter.processInlineFunction(inlineFn.fn, call) { val (function, wrapperBody) = inlineFn.fn - cycleReporter.withInlineFunctionDefinition(function) { - if (wrapperBody != null) { - val scope = PublicInlineFunctionInliningScope(function, wrapperBody, containingScope.fragment) - scope.process(wrapperBody) - scope.update() - } else { - containingScope.process(function) - } + if (wrapperBody != null) { + val scope = PublicInlineFunctionInliningScope(function, wrapperBody, containingScope.fragment) + scope.process(wrapperBody) + scope.update() + } else { + containingScope.process(function) } } } @@ -85,7 +60,7 @@ class JsInliner( fun inline(scope: InliningScope, call: JsInvocation, currentStatement: JsStatement?): InlineableResult { val definition = functionContext.getFunctionDefinition(call, scope) - return cycleReporter.withInlining(call) { + return cycleReporter.inlineCall(call) { val function = scope.importFunctionDefinition(definition) 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 294bdf1d1c0..2c761b0738f 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 @@ -21,19 +21,10 @@ import org.jetbrains.kotlin.js.backend.ast.metadata.coroutineMetadata import org.jetbrains.kotlin.js.backend.ast.metadata.imported -fun removeUnusedImports(fragment: JsProgramFragment) { +fun removeUnusedImports(fragment: JsProgramFragment, code: JsBlock) { val usedImports = mutableSetOf() - with(fragment) { - inlinedFunctionWrappers.values.forEach { - collectUsedImports(it, usedImports) - } - collectUsedImports(declarationBlock, usedImports) - collectUsedImports(initializerBlock, usedImports) - tests?.let { collectUsedImports(it, usedImports) } - mainFunction?.let { collectUsedImports(it, usedImports) } - collectUsedImports(exportBlock, usedImports) - } + collectUsedImports(code, usedImports) fragment.nameBindings.retainAll { !it.name.imported || it.name in usedImports } diff --git a/js/js.inliner/src/org/jetbrains/kotlin/js/inline/context/FunctionContext.kt b/js/js.inliner/src/org/jetbrains/kotlin/js/inline/context/FunctionContext.kt index 492c98cdae2..615e2ac1d01 100644 --- a/js/js.inliner/src/org/jetbrains/kotlin/js/inline/context/FunctionContext.kt +++ b/js/js.inliner/src/org/jetbrains/kotlin/js/inline/context/FunctionContext.kt @@ -27,20 +27,6 @@ import java.util.HashMap class FunctionContext( val inliner: JsInliner ) { - private val functionReader = FunctionReader(inliner.reporter, inliner.config, inliner.translationResult.innerModuleName) - - private data class FunctionsAndAccessors( - val functions: Map, - val accessors: Map) - - private val fragmentInfo = mutableMapOf() - - private fun lookUpStaticFunction(functionName: JsName?, fragment: JsProgramFragment): FunctionWithWrapper? = - fragmentInfo[fragment]?.run { functions[functionName] } - - private fun lookUpStaticFunctionByTag(functionTag: String, fragment: JsProgramFragment): FunctionWithWrapper? = - fragmentInfo[fragment]?.run { accessors[functionTag] } - fun getFunctionDefinition(call: JsInvocation, scope: InliningScope): InlineFunctionDefinition { return getFunctionDefinitionImpl(call, scope)!! } @@ -51,6 +37,12 @@ class FunctionContext( val functionsByFunctionNodes = HashMap() + fun scopeForFragment(fragment: JsProgramFragment) = if (fragment in newFragments) { + inliningScopeCache.computeIfAbsent(fragment) { + ProgramFragmentInliningScope(fragment) + } + } else null + /** * Gets function definition by invocation. * @@ -89,6 +81,21 @@ class FunctionContext( return lookUpFunctionDirect(call) ?: lookUpFunctionIndirect(call, scope) ?: lookUpFunctionExternal(call, scope.fragment) } + private val functionReader = FunctionReader(inliner.reporter, inliner.config, inliner.translationResult.innerModuleName) + + private data class FunctionsAndAccessors( + val functions: Map, + val accessors: Map) + + private val fragmentInfo = mutableMapOf() + + private fun lookUpStaticFunction(functionName: JsName?, fragment: JsProgramFragment): FunctionWithWrapper? = + fragmentInfo[fragment]?.run { functions[functionName] } + + private fun lookUpStaticFunctionByTag(functionTag: String, fragment: JsProgramFragment): FunctionWithWrapper? = + fragmentInfo[fragment]?.run { accessors[functionTag] } + + private fun lookUpFunctionIndirect(call: JsInvocation, scope: InliningScope): InlineFunctionDefinition? { /** remove ending `()` */ val callQualifier: JsExpression = if (isCallInvocation(call)) { @@ -124,11 +131,6 @@ class FunctionContext( private val newFragments = inliner.translationResult.newFragments.toIdentitySet() - fun scopeForFragment(fragment: JsProgramFragment) = if (fragment in newFragments) { - inliningScopeCache.computeIfAbsent(fragment) { - ProgramFragmentInliningScope(fragment) - } - } else null private fun loadFragment(fragment: JsProgramFragment) { fragmentInfo.computeIfAbsent(fragment) { @@ -143,27 +145,32 @@ class FunctionContext( } } - fun fragmentByTag(tag: String): JsProgramFragment? { + private fun fragmentByTag(tag: String): JsProgramFragment? { return inliner.translationResult.inlineFunctionTagMap[tag]?.let { unit -> inliner.translationResult.translate(unit).fragment.also { loadFragment(it) } } } - private fun lookUpFunctionDirect(call: JsInvocation): InlineFunctionDefinition? = - call.descriptor?.let { descriptor -> - Namer.getFunctionTag(descriptor, inliner.config).let { tag -> - fragmentByTag(tag)?.let { definitionFragment -> - return lookUpStaticFunctionByTag(tag, definitionFragment)?.let { fn -> - InlineFunctionDefinition(fn, tag).also { definition -> - scopeForFragment(definitionFragment)?.let { definitionScope -> - inliner.process(definition, call, definitionScope) - } - } - } - } - } + private fun lookUpFunctionDirect(call: JsInvocation): InlineFunctionDefinition? { + val descriptor = call.descriptor ?: return null + + val tag = Namer.getFunctionTag(descriptor, inliner.config) + + val definitionFragment = fragmentByTag(tag) ?: return null + + val fn = lookUpStaticFunctionByTag(tag, definitionFragment) ?: return null + + val definition = InlineFunctionDefinition(fn, tag) + + // Process definition if it is in the new fragments + scopeForFragment(definitionFragment)?.let { definitionScope -> + inliner.process(definition, call, definitionScope) } + return definition + } + + private fun lookUpFunctionExternal(call: JsInvocation, fragment: JsProgramFragment): InlineFunctionDefinition? = call.descriptor?.let { descriptor -> functionReader[descriptor, fragment]?.let {