From ff5090136cd3a772998d9c2f8f600ea65960fcbf Mon Sep 17 00:00:00 2001 From: Alexey Tsvetkov Date: Tue, 14 Apr 2015 18:12:25 +0300 Subject: [PATCH] JS: do not extract expressions, when inline call can be expression --- .../js/inline/FunctionInlineMutator.java | 4 ++ .../jetbrains/kotlin/js/inline/JsInliner.java | 9 ++- .../js/test/utils/DirectiveTestUtils.java | 56 ++++++++++++++----- .../inline/cases/noAdditionalVarsCreated.kt | 21 +++++++ 4 files changed, 74 insertions(+), 16 deletions(-) create mode 100644 js/js.translator/testData/inline/cases/noAdditionalVarsCreated.kt diff --git a/js/js.inliner/src/org/jetbrains/kotlin/js/inline/FunctionInlineMutator.java b/js/js.inliner/src/org/jetbrains/kotlin/js/inline/FunctionInlineMutator.java index 842b862488f..a0eedc0fe51 100644 --- a/js/js.inliner/src/org/jetbrains/kotlin/js/inline/FunctionInlineMutator.java +++ b/js/js.inliner/src/org/jetbrains/kotlin/js/inline/FunctionInlineMutator.java @@ -224,6 +224,10 @@ class FunctionInlineMutator { return !thisRefs.isEmpty(); } + public static boolean canBeExpression(JsFunction function) { + return canBeExpression(function.getBody()); + } + private static boolean canBeExpression(JsBlock body) { List statements = body.getStatements(); return statements.size() == 1 && statements.get(0) instanceof JsReturn; 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 a7beef734f0..3d51173a66f 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 @@ -31,6 +31,7 @@ import org.jetbrains.kotlin.resolve.inline.InlineStrategy; import java.util.*; import kotlin.Function1; +import static org.jetbrains.kotlin.js.inline.FunctionInlineMutator.canBeExpression; import static org.jetbrains.kotlin.js.inline.FunctionInlineMutator.getInlineableCallReplacement; import static org.jetbrains.kotlin.js.inline.clean.CleanPackage.removeUnusedFunctionDefinitions; import static org.jetbrains.kotlin.js.inline.clean.CleanPackage.removeUnusedLocalFunctionDeclarations; @@ -55,7 +56,13 @@ public class JsInliner extends JsVisitorWithContextImpl { if (!(node instanceof JsInvocation)) return false; JsInvocation call = (JsInvocation) node; - return hasToBeInlined(call); + + if (hasToBeInlined(call)) { + JsFunction function = getFunctionContext().getFunctionDefinition(call); + return !canBeExpression(function); + } + + return false; } }; diff --git a/js/js.tests/test/org/jetbrains/kotlin/js/test/utils/DirectiveTestUtils.java b/js/js.tests/test/org/jetbrains/kotlin/js/test/utils/DirectiveTestUtils.java index 29b22e8f171..5aa51e8075e 100644 --- a/js/js.tests/test/org/jetbrains/kotlin/js/test/utils/DirectiveTestUtils.java +++ b/js/js.tests/test/org/jetbrains/kotlin/js/test/utils/DirectiveTestUtils.java @@ -16,10 +16,7 @@ package org.jetbrains.kotlin.js.test.utils; -import com.google.dart.compiler.backend.js.ast.JsExpression; -import com.google.dart.compiler.backend.js.ast.JsFunction; -import com.google.dart.compiler.backend.js.ast.JsLabel; -import com.google.dart.compiler.backend.js.ast.JsNode; +import com.google.dart.compiler.backend.js.ast.*; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.jetbrains.kotlin.js.translate.expression.InlineMetadata; @@ -94,29 +91,57 @@ public class DirectiveTestUtils { } }; - private static final DirectiveHandler COUNT_LABELS = new DirectiveHandler("CHECK_LABELS_COUNT") { + private abstract static class CountNodesDirective extends DirectiveHandler { + + @NotNull + private final Class klass; + + CountNodesDirective(@NotNull String directive, @NotNull Class klass) { + super(directive); + this.klass = klass; + } + @Override void processEntry(@NotNull JsNode ast, @NotNull ArgumentsHelper arguments) throws Exception { String functionName = arguments.getNamedArgument("function"); - String labelName = arguments.getNamedArgument("name"); String countStr = arguments.getNamedArgument("count"); int expectedCount = Integer.valueOf(countStr); JsNode scope = AstSearchUtil.getFunction(ast, functionName); - List labels = collectInstances(JsLabel.class, scope); + List nodes = collectInstances(klass, scope); int actualCount = 0; - for (JsLabel label : labels) { - if (label.getName().getIdent().equals(labelName)) { - actualCount++; - } + + for (T node : nodes) { + actualCount += getActualCountFor(node, arguments); } - String message = "Label " + labelName + - " is expected to be counted " + expectedCount + - " times at function " + functionName + - " but was encountered " + actualCount + " times"; + String message = "Function " + functionName + " contains " + actualCount + + " nodes of type " + klass.getName() + + ", but expected count is " + expectedCount; assertEquals(message, expectedCount, actualCount); } + + protected abstract int getActualCountFor(@NotNull T node, @NotNull ArgumentsHelper arguments); + } + + private static final DirectiveHandler COUNT_LABELS = new CountNodesDirective("CHECK_LABELS_COUNT", JsLabel.class) { + @Override + protected int getActualCountFor(@NotNull JsLabel node, @NotNull ArgumentsHelper arguments) { + String labelName = arguments.getNamedArgument("name"); + + if (node.getName().getIdent().equals(labelName)) { + return 1; + } + + return 0; + } + }; + + private static final DirectiveHandler COUNT_VARS = new CountNodesDirective("CHECK_VARS_COUNT", JsVars.class) { + @Override + protected int getActualCountFor(@NotNull JsVars node, @NotNull ArgumentsHelper arguments) { + return node.getVars().size(); + } }; private static final DirectiveHandler HAS_INLINE_METADATA = new DirectiveHandler("CHECK_HAS_INLINE_METADATA") { @@ -146,6 +171,7 @@ public class DirectiveTestUtils { FUNCTION_NOT_CALLED_IN_SCOPE.process(ast, sourceCode); FUNCTIONS_HAVE_SAME_LINES.process(ast, sourceCode); COUNT_LABELS.process(ast, sourceCode); + COUNT_VARS.process(ast, sourceCode); HAS_INLINE_METADATA.process(ast, sourceCode); HAS_NO_INLINE_METADATA.process(ast, sourceCode); } diff --git a/js/js.translator/testData/inline/cases/noAdditionalVarsCreated.kt b/js/js.translator/testData/inline/cases/noAdditionalVarsCreated.kt new file mode 100644 index 00000000000..c2f00bceed5 --- /dev/null +++ b/js/js.translator/testData/inline/cases/noAdditionalVarsCreated.kt @@ -0,0 +1,21 @@ +package foo + +// CHECK_CONTAINS_NO_CALLS: test +// COUNT_VARS: function=test count=0 + +// A copy of stdlib run function. +// Copied to not to depend on run implementation. +// It's important, that the body is just `return fn()`. +inline fun evaluate(fn: ()->T): T = fn() + +fun test(x: Int): Int = + evaluate { + evaluate { 2 } * evaluate { x } + } + +fun box(): String { + assertEquals(6, test(3)) + assertEquals(8, test(4)) + + return "OK" +} \ No newline at end of file