From b7e26c3bf70ff4686475f6e24be78be283b8169c Mon Sep 17 00:00:00 2001 From: Alexey Tsvetkov Date: Wed, 8 Apr 2015 14:45:18 +0300 Subject: [PATCH] JS: extract expressions preceding inline call --- .../kotlin/js/inline/ExpressionDecomposer.kt | 191 ++++++++++++++++++ .../jetbrains/kotlin/js/inline/JsInliner.java | 74 +++++-- .../kotlin/js/inline/util/namingUtils.kt | 2 +- .../util/rewriters/ReturnReplacingVisitor.kt | 2 +- .../kotlin/js/inline/util/sideEffectUtils.kt | 78 ++----- .../kotlin/js/translate/utils/astUtils.kt | 25 ++- 6 files changed, 289 insertions(+), 83 deletions(-) create mode 100644 js/js.inliner/src/org/jetbrains/kotlin/js/inline/ExpressionDecomposer.kt diff --git a/js/js.inliner/src/org/jetbrains/kotlin/js/inline/ExpressionDecomposer.kt b/js/js.inliner/src/org/jetbrains/kotlin/js/inline/ExpressionDecomposer.kt new file mode 100644 index 00000000000..e04663b12c1 --- /dev/null +++ b/js/js.inliner/src/org/jetbrains/kotlin/js/inline/ExpressionDecomposer.kt @@ -0,0 +1,191 @@ +/* + * 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 + +import com.google.dart.compiler.backend.js.ast.* +import com.google.dart.compiler.backend.js.ast.metadata.* +import org.jetbrains.kotlin.js.translate.utils.JsAstUtils.* + +import kotlin.platform.platformStatic + +import com.intellij.util.SmartList + +/** + * If inline function consists of multiple statements, + * its statements will be extracted before last visited statement. + * This action can potentially break evaluation order. + * + * An example: + * var x = foo() + inlineBox(); + * Inliner could extract statements from inlineBox(): + * // inlineBox body.. + * var x = foo() + inlineBox$result; + * So foo is evaluated before inlineBox() after inline. + * + * So we need to extract all nodes, that evaluate before inline call, + * to temporary vars (preserving evaluation order). + * + * For the example, defined before, evaluation order could be preserved this way: + * var tmp = foo(); + * // inlineBox body.. + * var x = tmp + inlineBox$result; + * + * It's desirable to create temporary var only if node can have side effect, + * and precedes inline call (in JavaScript evaluation order). + */ +class ExpressionDecomposer private ( + private val scope: JsScope, + private val containsExtractable: Set, + private val containsNodeWithSideEffect: Set +) : JsExpressionVisitor() { + + private var additionalStatements: MutableList = SmartList() + + companion object { + platformStatic + public fun preserveEvaluationOrder( + scope: JsScope, statement: JsStatement, canBeExtractedByInliner: (JsNode)->Boolean + ): List { + val decomposer = with (statement) { + val extractable = match(canBeExtractedByInliner) + val containsExtractable = withParentsOfNodes(extractable) + val nodesWithSideEffect = match { it is JsExpression && it.canHaveOwnSideEffect() } + val containsNodeWithSideEffect = withParentsOfNodes(nodesWithSideEffect) + + ExpressionDecomposer(scope, containsExtractable, containsNodeWithSideEffect) + } + + decomposer.accept(statement) + return decomposer.additionalStatements + } + } +} + +/** + * Visits only expressions that can potentially be extracted by [JsInliner], + * that directly referenced by statements. + * + * For example, won't visit [JsBlock] statements, but will visit test expression of [JsWhile]. + */ +private open class JsExpressionVisitor() : JsVisitorWithContextImpl() { + + override fun visit(x: JsBlock, ctx: JsContext<*>): Boolean = false + override fun visit(x: JsTry, ctx: JsContext<*>): Boolean = false + override fun visit(x: JsDebugger, ctx: JsContext<*>): Boolean = false + override fun visit(x: JsLabel, ctx: JsContext<*>): Boolean = false + override fun visit(x: JsFunction, ctx: JsContext<*>): Boolean = false + override fun visit(x: JsObjectLiteral, ctx: JsContext<*>): Boolean = false + override fun visit(x: JsPropertyInitializer, ctx: JsContext<*>): Boolean = false + override fun visit(x: JsProgramFragment, ctx: JsContext<*>): Boolean = false + override fun visit(x: JsProgram, ctx: JsContext<*>): Boolean = false + override fun visit(x: JsParameter, ctx: JsContext<*>): Boolean = false + override fun visit(x: JsCatch, ctx: JsContext<*>): Boolean = false + override fun visit(x: JsBreak, ctx: JsContext<*>): Boolean = false + override fun visit(x: JsContinue, ctx: JsContext<*>): Boolean = false + override fun visit(x: JsCase, ctx: JsContext<*>): Boolean = false + override fun visit(x: JsDefault, ctx: JsContext<*>): Boolean = false + override fun visit(x: JsEmpty, ctx: JsContext<*>): Boolean = false + override fun visit(x: JsLiteral.JsBooleanLiteral, ctx: JsContext<*>): Boolean = false + override fun visit(x: JsLiteral.JsThisRef, ctx: JsContext<*>): Boolean = false + override fun visit(x: JsNullLiteral, ctx: JsContext<*>): Boolean = false + override fun visit(x: JsNumberLiteral, ctx: JsContext<*>): Boolean = false + override fun visit(x: JsRegExp, ctx: JsContext<*>): Boolean = false + override fun visit(x: JsStringLiteral, ctx: JsContext<*>): Boolean = false + override fun visit(x: JsName, ctx: JsContext<*>): Boolean = false + + // TODO: support these + // Not generated by compiler now, (can be generated in future or used in js() block) + override fun visit(x: JsForIn, ctx: JsContext<*>): Boolean = false + override fun visit(x: JsSwitch, ctx: JsContext<*>): Boolean = false + + // Compiler generates restricted version of for, + // where init and test do not contain inline calls. + override fun visit(x: JsFor, ctx: JsContext<*>): Boolean = false + + override fun visit(x: JsIf, ctx: JsContext<*>): Boolean = false + override fun visit(x: JsWhile, ctx: JsContext<*>): Boolean = false + override fun visit(x: JsDoWhile, ctx: JsContext<*>): Boolean = false + + override fun visit(x: JsArrayAccess, ctx: JsContext<*>): Boolean = true + override fun visit(x: JsArrayLiteral, ctx: JsContext<*>): Boolean = true + override fun visit(x: JsBinaryOperation, ctx: JsContext<*>): Boolean = true + override fun visit(x: JsConditional, ctx: JsContext<*>): Boolean = true + override fun visit(x: JsInvocation, ctx: JsContext<*>): Boolean = true + override fun visit(x: JsNameRef, ctx: JsContext<*>): Boolean = true + override fun visit(x: JsNew, ctx: JsContext<*>): Boolean = true + override fun visit(x: JsVars.JsVar, ctx: JsContext<*>): Boolean = true + override fun visit(x: JsPostfixOperation, ctx: JsContext<*>): Boolean = true + override fun visit(x: JsPrefixOperation, ctx: JsContext<*>): Boolean = true + override fun visit(x: JsExpressionStatement, ctx: JsContext<*>): Boolean = true + override fun visit(x: JsReturn, ctx: JsContext<*>): Boolean = true + override fun visit(x: JsThrow, ctx: JsContext<*>): Boolean = true + override fun visit(x: JsVars, ctx: JsContext<*>): Boolean = true +} + +/** + * Returns descendants of receiver, matched by [predicate]. + */ +private fun JsNode.match(predicate: (JsNode) -> Boolean): Set { + val visitor = object : JsExpressionVisitor() { + val matched = IdentitySet() + + override fun doTraverse(node: R, ctx: JsContext<*>?) { + super.doTraverse(node, ctx) + + if (node !in matched && predicate(node)) { + matched.add(node) + } + } + } + + visitor.accept(this) + return visitor.matched +} + +/** + * Returns set of nodes, that satisfy transitive closure of `is parent` relation, starting from [nodes]. + */ +private fun JsNode.withParentsOfNodes(nodes: Set): Set { + val visitor = object : JsExpressionVisitor() { + private val stack = SmartList() + val matched = IdentitySet() + + override fun doTraverse(node: R, ctx: JsContext<*>?) { + stack.add(node) + super.doTraverse(node, ctx) + + if (node in nodes) { + addAllUntilMatchedOrStatement(stack) + } + + stack.remove(stack.lastIndex) + } + + fun addAllUntilMatchedOrStatement(nodesOnStack: List) { + for (i in nodesOnStack.lastIndex downTo 0) { + val currentNode = nodesOnStack[i] + if (currentNode in matched) break + + matched.add(currentNode) + if (currentNode is JsStatement) break + } + } + } + + visitor.accept(this) + return visitor.matched +} diff --git a/js/js.inliner/src/org/jetbrains/kotlin/js/inline/JsInliner.java b/js/js.inliner/src/org/jetbrains/kotlin/js/inline/JsInliner.java index a1c5695624f..a7beef734f0 100644 --- a/js/js.inliner/src/org/jetbrains/kotlin/js/inline/JsInliner.java +++ b/js/js.inliner/src/org/jetbrains/kotlin/js/inline/JsInliner.java @@ -23,11 +23,13 @@ import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.jetbrains.kotlin.diagnostics.DiagnosticSink; import org.jetbrains.kotlin.js.inline.context.*; +import org.jetbrains.kotlin.js.inline.util.ExpressionDecomposer; import org.jetbrains.kotlin.js.resolve.diagnostics.ErrorsJs; import org.jetbrains.kotlin.js.translate.context.TranslationContext; import org.jetbrains.kotlin.resolve.inline.InlineStrategy; import java.util.*; +import kotlin.Function1; import static org.jetbrains.kotlin.js.inline.FunctionInlineMutator.getInlineableCallReplacement; import static org.jetbrains.kotlin.js.inline.clean.CleanPackage.removeUnusedFunctionDefinitions; @@ -47,6 +49,15 @@ public class JsInliner extends JsVisitorWithContextImpl { // these are needed for error reporting, when inliner detects cycle private final Stack namedFunctionsStack = new Stack(); private final LinkedList inlineCallInfos = new LinkedList(); + private final Function1 canBeExtractedByInliner = new Function1() { + @Override + public Boolean invoke(JsNode node) { + if (!(node instanceof JsInvocation)) return false; + + JsInvocation call = (JsInvocation) node; + return hasToBeInlined(call); + } + }; public static JsProgram process(@NotNull TranslationContext context) { JsProgram program = context.program(); @@ -100,24 +111,21 @@ public class JsInliner extends JsVisitorWithContextImpl { @Override public boolean visit(@NotNull JsInvocation call, @NotNull JsContext context) { - if (shouldInline(call) && canInline(call)) { - JsFunction containingFunction = getCurrentNamedFunction(); - if (containingFunction != null) { - inlineCallInfos.add(new JsCallInfo(call, containingFunction)); - } + if (!hasToBeInlined(call)) return true; - JsFunction definition = getFunctionContext().getFunctionDefinition(call); + JsFunction containingFunction = getCurrentNamedFunction(); - if (inProcessFunctions.contains(definition)) { - reportInlineCycle(call, definition); - return false; - } + if (containingFunction != null) { + inlineCallInfos.add(new JsCallInfo(call, containingFunction)); + } - if (!processedFunctions.contains(definition)) { - accept(definition); - } + JsFunction definition = getFunctionContext().getFunctionDefinition(call); - inline(call, context); + if (inProcessFunctions.contains(definition)) { + reportInlineCycle(call, definition); + } + else if (!processedFunctions.contains(definition)) { + accept(definition); } return true; @@ -125,6 +133,10 @@ public class JsInliner extends JsVisitorWithContextImpl { @Override public void endVisit(@NotNull JsInvocation x, @NotNull JsContext ctx) { + if (hasToBeInlined(x)) { + inline(x, ctx); + } + JsCallInfo lastCallInfo = null; if (!inlineCallInfos.isEmpty()) { @@ -136,6 +148,25 @@ public class JsInliner extends JsVisitorWithContextImpl { } } + @Override + protected void doAcceptStatementList(List statements) { + // at top level of js ast, contexts stack can be empty, + // but there is no inline calls anyway + if(!inliningContexts.isEmpty()) { + JsScope scope = getFunctionContext().getScope(); + int i = 0; + + while (i < statements.size()) { + List additionalStatements = + ExpressionDecomposer.preserveEvaluationOrder(scope, statements.get(i), canBeExtractedByInliner); + statements.addAll(i, additionalStatements); + i += additionalStatements.size() + 1; + } + } + + super.doAcceptStatementList(statements); + } + private void inline(@NotNull JsInvocation call, @NotNull JsContext context) { JsInliningContext inliningContext = getInliningContext(); FunctionContext functionContext = getFunctionContext(); @@ -146,7 +177,9 @@ public class JsInliner extends JsVisitorWithContextImpl { JsExpression resultExpression = inlineableResult.getResultExpression(); JsContext statementContext = inliningContext.getStatementContext(); // body of inline function can contain call to lambdas that need to be inlined - accept(inlineableBody); + JsStatement statement = accept(inlineableBody); + assert inlineableBody == statement; + statementContext.addPrevious(flattenStatement(inlineableBody)); /** @@ -195,14 +228,11 @@ public class JsInliner extends JsVisitorWithContextImpl { } } - private boolean canInline(@NotNull JsInvocation call) { - FunctionContext functionContext = getFunctionContext(); - return functionContext.hasFunctionDefinition(call); - } - - private static boolean shouldInline(@NotNull JsInvocation call) { + public boolean hasToBeInlined(@NotNull JsInvocation call) { InlineStrategy strategy = MetadataPackage.getInlineStrategy(call); - return strategy != null && strategy.isInline(); + if (strategy == null || !strategy.isInline()) return false; + + return getFunctionContext().hasFunctionDefinition(call); } private class JsInliningContext implements InliningContext { 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 c47cfa33323..294da80db18 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 @@ -32,7 +32,7 @@ public fun aliasArgumentsIfNeeded( for ((arg, param) in arguments zip parameters) { val paramName = param.getName() val replacement = - if (needToAlias(arg)) { + if (arg.needToAlias()) { val freshName = context.getFreshName(paramName) context.newVar(freshName, arg) freshName.makeRef() diff --git a/js/js.inliner/src/org/jetbrains/kotlin/js/inline/util/rewriters/ReturnReplacingVisitor.kt b/js/js.inliner/src/org/jetbrains/kotlin/js/inline/util/rewriters/ReturnReplacingVisitor.kt index 982849ade75..6a9725d33e2 100644 --- a/js/js.inliner/src/org/jetbrains/kotlin/js/inline/util/rewriters/ReturnReplacingVisitor.kt +++ b/js/js.inliner/src/org/jetbrains/kotlin/js/inline/util/rewriters/ReturnReplacingVisitor.kt @@ -59,7 +59,7 @@ class ReturnReplacingVisitor(private val resultRef: JsNameRef?, private val brea if (resultRef != null) return JsAstUtils.assignment(resultRef, returnExpression) - if (canHaveSideEffect(returnExpression)) + if (returnExpression.canHaveSideEffect()) return returnExpression } diff --git a/js/js.inliner/src/org/jetbrains/kotlin/js/inline/util/sideEffectUtils.kt b/js/js.inliner/src/org/jetbrains/kotlin/js/inline/util/sideEffectUtils.kt index c79cab3746b..550d22ee1eb 100644 --- a/js/js.inliner/src/org/jetbrains/kotlin/js/inline/util/sideEffectUtils.kt +++ b/js/js.inliner/src/org/jetbrains/kotlin/js/inline/util/sideEffectUtils.kt @@ -16,67 +16,33 @@ package org.jetbrains.kotlin.js.inline.util -import com.google.dart.compiler.backend.js.ast.JsArrayAccess -import com.google.dart.compiler.backend.js.ast.JsArrayLiteral -import com.google.dart.compiler.backend.js.ast.JsBinaryOperation -import com.google.dart.compiler.backend.js.ast.JsConditional -import com.google.dart.compiler.backend.js.ast.JsExpression -import com.google.dart.compiler.backend.js.ast.JsInvocation -import com.google.dart.compiler.backend.js.ast.JsLiteral.JsThisRef -import com.google.dart.compiler.backend.js.ast.JsLiteral.JsValueLiteral -import com.google.dart.compiler.backend.js.ast.JsNameRef -import com.google.dart.compiler.backend.js.ast.JsNode -import com.google.dart.compiler.backend.js.ast.RecursiveJsVisitor +import com.google.dart.compiler.backend.js.ast.JsLiteral.* +import com.google.dart.compiler.backend.js.ast.* +import org.jetbrains.kotlin.js.translate.utils.ast.any -public fun canHaveSideEffect(x: JsExpression): Boolean { - return with(SideEffectVisitor()) { - accept(x) - !sideEffectFree - } -} +public fun JsExpression.canHaveSideEffect(): Boolean = + any { it is JsExpression && it.canHaveOwnSideEffect() } -public fun needToAlias(x: JsExpression): Boolean { - return with(NeedToAliasVisitor()) { - accept(x) - !sideEffectFree - } -} - -private open class SideEffectVisitor() : RecursiveJsVisitor() { - public var sideEffectFree: Boolean = true - protected set - - override fun visitElement(node: JsNode) { - sideEffectFree = sideEffectFree && isSideEffectFree(node) - - if (sideEffectFree) { - super.visitElement(node) - } +public fun JsExpression.canHaveOwnSideEffect(): Boolean = + when (this) { + is JsValueLiteral, + is JsConditional, + is JsArrayAccess, + is JsArrayLiteral, + is JsNameRef -> false + is JsBinaryOperation -> getOperator().isAssignment() + else -> true } - protected open fun isSideEffectFree(node: JsNode): Boolean = - when (node) { - is JsValueLiteral, - is JsConditional, - is JsArrayAccess, - is JsArrayLiteral, - is JsNameRef -> - true - is JsBinaryOperation -> - !node.getOperator().isAssignment() - else -> - false - } -} +public fun JsExpression.needToAlias(): Boolean = + any { it is JsExpression && it.shouldHaveOwnAlias() } -private class NeedToAliasVisitor() : SideEffectVisitor() { - override fun isSideEffectFree(node: JsNode): Boolean = - when (node) { +public fun JsExpression.shouldHaveOwnAlias(): Boolean = + when (this) { is JsThisRef, is JsConditional, is JsBinaryOperation, - is JsArrayLiteral -> false - is JsInvocation -> isFunctionCreatorInvocation(node) - else -> super.isSideEffectFree(node) - } -} + is JsArrayLiteral -> true + is JsInvocation -> !isFunctionCreatorInvocation(this) + else -> canHaveOwnSideEffect() + } \ No newline at end of file diff --git a/js/js.translator/src/org/jetbrains/kotlin/js/translate/utils/astUtils.kt b/js/js.translator/src/org/jetbrains/kotlin/js/translate/utils/astUtils.kt index bf52752e847..5f865864c8c 100644 --- a/js/js.translator/src/org/jetbrains/kotlin/js/translate/utils/astUtils.kt +++ b/js/js.translator/src/org/jetbrains/kotlin/js/translate/utils/astUtils.kt @@ -16,9 +16,8 @@ package org.jetbrains.kotlin.js.translate.utils.ast -import com.google.dart.compiler.backend.js.ast.JsFunction -import com.google.dart.compiler.backend.js.ast.JsStatement -import com.google.dart.compiler.backend.js.ast.JsParameter +import com.google.dart.compiler.backend.js.ast.* +import org.jetbrains.kotlin.js.inline.util.IdentitySet public fun JsFunction.addStatement(stmt: JsStatement) { getBody().getStatements().add(stmt) @@ -36,3 +35,23 @@ public fun JsFunction.addParameter(identifier: String, index: Int? = null): JsPa return parameter } + +/** + * Tests, if any node containing in receiver's AST matches, [predicate]. + */ +public fun JsNode.any(predicate: (JsNode) -> Boolean): Boolean { + val visitor = object : RecursiveJsVisitor() { + public var matched: Boolean = false + + override fun visitElement(node: JsNode) { + matched = matched || predicate(node) + + if (!matched) { + super.visitElement(node) + } + } + } + + visitor.accept(this) + return visitor.matched +}