From 8ad339836d73de50ac1814ada23698d15ee294ba Mon Sep 17 00:00:00 2001 From: Alexey Andreev Date: Wed, 13 Apr 2016 12:51:12 +0300 Subject: [PATCH] KT-11875 Moves code that uncovers the following JS function: ``` function foo (closureParam, closureParam2, ...) { return function() { // function body } } ``` from FunctionContext to FunctionInlineMutator. The old code causes bug when calling this function like this: `foo(this, ...)`, since first `closureParam` is replaced with `this`. Later, FunctionInlineMutator can't make difference between original `this` inside *function body* and former `closureParam` reference that became `this` after replacement. So, let FunctionInlineMutator replace `this` in *function body* and then substitute closure parameters. KT-11875 Fixed --- .../kotlin/js/inline/FunctionInlineMutator.kt | 48 ++++++++-- .../jetbrains/kotlin/js/inline/JsInliner.java | 4 +- .../js/inline/context/FunctionContext.kt | 88 +------------------ .../test/semantics/InlineJsTestGenerated.java | 6 ++ .../testData/inline/cases/innerOuterThis.kt | 19 ++++ 5 files changed, 68 insertions(+), 97 deletions(-) create mode 100644 js/js.translator/testData/inline/cases/innerOuterThis.kt diff --git a/js/js.inliner/src/org/jetbrains/kotlin/js/inline/FunctionInlineMutator.kt b/js/js.inliner/src/org/jetbrains/kotlin/js/inline/FunctionInlineMutator.kt index 05774e2e052..8308d8f5867 100644 --- a/js/js.inliner/src/org/jetbrains/kotlin/js/inline/FunctionInlineMutator.kt +++ b/js/js.inliner/src/org/jetbrains/kotlin/js/inline/FunctionInlineMutator.kt @@ -17,6 +17,7 @@ package org.jetbrains.kotlin.js.inline import com.google.dart.compiler.backend.js.ast.* +import com.google.dart.compiler.backend.js.ast.metadata.staticRef import com.google.dart.compiler.backend.js.ast.metadata.synthetic import org.jetbrains.kotlin.js.inline.clean.removeDefaultInitializers import org.jetbrains.kotlin.js.inline.context.InliningContext @@ -37,18 +38,16 @@ private constructor( private val currentStatement = inliningContext.statementContext.currentNode init { - - val functionContext = inliningContext.functionContext - invokedFunction = functionContext.getFunctionDefinition(call) - body = invokedFunction.body.deepCopy() namingContext = inliningContext.newNamingContext() + val functionContext = inliningContext.functionContext + invokedFunction = uncoverClosure(functionContext.getFunctionDefinition(call).deepCopy()) + body = invokedFunction.body } private fun process() { val arguments = getArguments() val parameters = getParameters() - replaceThis() removeDefaultInitializers(arguments, parameters, body) aliasArgumentsIfNeeded(namingContext, arguments, parameters) renameLocalNames(namingContext, invokedFunction) @@ -60,8 +59,41 @@ private constructor( } } - private fun replaceThis() { - if (!hasThisReference(body)) return + private fun uncoverClosure(invokedFunction: JsFunction): JsFunction { + val innerFunction = invokedFunction.getInnerFunction() + val innerCall = getInnerCall(call.qualifier) + return if (innerCall != null && innerFunction != null) { + innerFunction.apply { + replaceThis(body) + applyCapturedArgs(innerCall, this, invokedFunction) + } + } + else { + invokedFunction.apply { replaceThis(body) } + } + } + + private fun getInnerCall(qualifier: JsExpression): JsInvocation? { + return when (qualifier) { + is JsInvocation -> qualifier + is JsNameRef -> { + val callee = if (qualifier.ident == "call") qualifier.qualifier else (qualifier.name?.staticRef as? JsExpression) + callee?.let { getInnerCall(it) } + } + else -> null + } + } + + private fun applyCapturedArgs(call: JsInvocation, inner: JsFunction, outer: JsFunction) { + val namingContext = inliningContext.newNamingContext() + val arguments = call.arguments + val parameters = outer.parameters + aliasArgumentsIfNeeded(namingContext, arguments, parameters) + namingContext.applyRenameTo(inner) + } + + private fun replaceThis(block: JsBlock) { + if (!hasThisReference(block)) return var thisReplacement = getThisReplacement(call) if (thisReplacement == null || thisReplacement is JsLiteral.JsThisRef) return @@ -72,7 +104,7 @@ private constructor( thisReplacement = thisName.makeRef() } - replaceThisReference(body, thisReplacement) + replaceThisReference(block, thisReplacement) } private fun processReturns() { 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 f48f1cc8ee2..8e3e5c3ad69 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 @@ -175,8 +175,6 @@ public class JsInliner extends JsVisitorWithContextImpl { private void inline(@NotNull JsInvocation call, @NotNull JsContext context) { JsInliningContext inliningContext = getInliningContext(); - FunctionContext functionContext = getFunctionContext(); - functionContext.declareFunctionConstructorCalls(call.getArguments()); InlineableResult inlineableResult = getInlineableCallReplacement(call, inliningContext); JsStatement inlineableBody = inlineableResult.getInlineableBody(); @@ -256,7 +254,7 @@ public class JsInliner extends JsVisitorWithContextImpl { private final FunctionContext functionContext; JsInliningContext(JsFunction function) { - functionContext = new FunctionContext(function, this, functionReader) { + functionContext = new FunctionContext(function, functionReader) { @Nullable @Override protected JsFunction lookUpStaticFunction(@Nullable JsName functionName) { 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 dd9455077e1..02f0a6ec9c3 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 @@ -19,29 +19,14 @@ package org.jetbrains.kotlin.js.inline.context import com.google.dart.compiler.backend.js.ast.* import com.google.dart.compiler.backend.js.ast.metadata.staticRef -import org.jetbrains.kotlin.js.inline.util.aliasArgumentsIfNeeded -import org.jetbrains.kotlin.js.inline.util.getInnerFunction -import org.jetbrains.kotlin.js.inline.util.getSimpleName -import org.jetbrains.kotlin.js.inline.util.isCallInvocation -import org.jetbrains.kotlin.js.inline.util.isFunctionCreator - -import com.intellij.util.containers.ContainerUtil -import java.util.IdentityHashMap import org.jetbrains.kotlin.js.inline.FunctionReader import com.google.dart.compiler.backend.js.ast.metadata.descriptor +import org.jetbrains.kotlin.js.inline.util.* abstract class FunctionContext( private val function: JsFunction, - private val inliningContext: InliningContext, private val functionReader: FunctionReader ) { - /** - * Caches function with captured arguments applied. - * - * @see getFunctionWithClosure - */ - private val functionsWithClosure = IdentityHashMap() - protected abstract fun lookUpStaticFunction(functionName: JsName?): JsFunction? fun getFunctionDefinition(call: JsInvocation): JsFunction { @@ -56,27 +41,6 @@ abstract class FunctionContext( return function.scope } - fun declareFunctionConstructorCalls(arguments: List) { - val calls = ContainerUtil.findAll(arguments, JsInvocation::class.java) - - for (call in calls) { - val callName = getSimpleName(call) - if (callName == null) continue - - val staticRef = callName.staticRef - if (staticRef !is JsFunction) continue - - val functionCalled = staticRef - if (isFunctionCreator(functionCalled)) { - declareFunctionConstructorCall(call) - } - } - } - - fun declareFunctionConstructorCall(call: JsInvocation) { - functionsWithClosure.put(call, null) - } - /** * Gets function definition by invocation. * @@ -135,57 +99,9 @@ abstract class FunctionContext( /** process cases 2, 3 */ val qualifier = callQualifier return when (qualifier) { - is JsInvocation -> getFunctionWithClosure(qualifier) + is JsInvocation -> lookUpStaticFunction(getSimpleName(qualifier)!!) is JsNameRef -> lookUpStaticFunction(qualifier.name) else -> null } } - - /** - * Gets function body with captured args applied, - * and stores in cache. - * - * Function literals and local functions with closure - * are translated as function, that returns function. - * - * For example, - * val a = 1 - * val f = { a * 2 } - * `f` becomes - * f: function (a) { - * return function () { return a * 2 } - * } - * - * @returns inner function with captured parameters, - * replaced by outer arguments - * - * For invocation `f(10)()` returns - * `function () { return 10 * 2 }` - */ - private fun getFunctionWithClosure(call: JsInvocation): JsFunction { - val constructed = functionsWithClosure.get(call) - - if (constructed is JsFunction) return constructed - - val name = getSimpleName(call)!! - val closureCreator = lookUpStaticFunction(name)!! - val innerFunction = closureCreator.getInnerFunction()!! - - val withCapturedArgs = applyCapturedArgs(call, innerFunction, closureCreator) - functionsWithClosure.put(call, withCapturedArgs) - - return withCapturedArgs - } - - private fun applyCapturedArgs(call: JsInvocation, inner: JsFunction, outer: JsFunction): JsFunction { - val innerClone = inner.deepCopy() - - val namingContext = inliningContext.newNamingContext() - val arguments = call.arguments - val parameters = outer.parameters - aliasArgumentsIfNeeded(namingContext, arguments, parameters) - namingContext.applyRenameTo(innerClone) - - return innerClone - } } diff --git a/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/InlineJsTestGenerated.java b/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/InlineJsTestGenerated.java index 42843f8b577..68cc1c1c8b7 100644 --- a/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/InlineJsTestGenerated.java +++ b/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/InlineJsTestGenerated.java @@ -209,6 +209,12 @@ public class InlineJsTestGenerated extends AbstractInlineJsTest { doTest(fileName); } + @TestMetadata("innerOuterThis.kt") + public void testInnerOuterThis() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("js/js.translator/testData/inline/cases/innerOuterThis.kt"); + doTest(fileName); + } + @TestMetadata("jsCode.kt") public void testJsCode() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("js/js.translator/testData/inline/cases/jsCode.kt"); diff --git a/js/js.translator/testData/inline/cases/innerOuterThis.kt b/js/js.translator/testData/inline/cases/innerOuterThis.kt new file mode 100644 index 00000000000..6e9c1d8ecf9 --- /dev/null +++ b/js/js.translator/testData/inline/cases/innerOuterThis.kt @@ -0,0 +1,19 @@ +package foo + +inline fun with1(value: T, p: T.() -> Unit) = value.p() + +class A(val expected: String) { + val b = B() + + fun foo(): A { + with1(b) { + y = expected + } + return this + } +} +class B() { + var y = "" +} + +fun box() = A("OK").foo().b.y \ No newline at end of file