From c77a1b0fb9e14b38d896247d7a9b13a6ff6f380e Mon Sep 17 00:00:00 2001 From: Zalim Bashorov Date: Mon, 16 Dec 2013 21:32:39 +0400 Subject: [PATCH] JS backend: fix recursive closure local function by inner function or lambda. #KT-4257 fixed --- .../k2js/test/semantics/ClosureTest.java | 17 +++++ .../k2js/translate/context/UsageTracker.java | 17 ++++- .../expression/ExpressionVisitor.java | 7 +- .../InnerDeclarationTranslator.java | 25 ++++---- .../expression/InnerFunctionTranslator.java | 19 +++++- .../expression/LiteralFunctionTranslator.java | 64 +++++++++++++++++-- .../cases/closureFunctionByInnerFunction.kt | 46 +++++++++++++ .../closureLocalFunctionByInnerFunction.kt | 44 +++++++++++++ ...calFunctionByInnerFunctionInConstructor.kt | 42 ++++++++++++ .../closure/cases/simpleRecursion.kt | 37 +++++++++++ 10 files changed, 291 insertions(+), 27 deletions(-) create mode 100644 js/js.translator/testFiles/closure/cases/closureFunctionByInnerFunction.kt create mode 100644 js/js.translator/testFiles/closure/cases/closureLocalFunctionByInnerFunction.kt create mode 100644 js/js.translator/testFiles/closure/cases/closureLocalFunctionByInnerFunctionInConstructor.kt create mode 100644 js/js.translator/testFiles/closure/cases/simpleRecursion.kt diff --git a/js/js.tests/test/org/jetbrains/k2js/test/semantics/ClosureTest.java b/js/js.tests/test/org/jetbrains/k2js/test/semantics/ClosureTest.java index cd655efc88d..2755ff2697e 100644 --- a/js/js.tests/test/org/jetbrains/k2js/test/semantics/ClosureTest.java +++ b/js/js.tests/test/org/jetbrains/k2js/test/semantics/ClosureTest.java @@ -51,4 +51,21 @@ public final class ClosureTest extends SingleFileTranslationTest { public void testWrappedVariableInExtensionFun() throws Exception { fooBoxTest(); } + + public void testSimpleRecursion() throws Exception { + checkFooBoxIsOk(); + } + + public void testClosureFunctionByInnerFunction() throws Exception { + checkFooBoxIsOk(); + } + + public void testClosureLocalFunctionByInnerFunction() throws Exception { + checkFooBoxIsOk(); + } + + // TODO: fix + public void igonre_testClosureLocalFunctionByInnerFunctionInConstrunctor() throws Exception { + checkFooBoxIsOk(); + } } diff --git a/js/js.translator/src/org/jetbrains/k2js/translate/context/UsageTracker.java b/js/js.translator/src/org/jetbrains/k2js/translate/context/UsageTracker.java index 2d21e89c4f9..425389725d2 100644 --- a/js/js.translator/src/org/jetbrains/k2js/translate/context/UsageTracker.java +++ b/js/js.translator/src/org/jetbrains/k2js/translate/context/UsageTracker.java @@ -68,7 +68,7 @@ public final class UsageTracker { capturedVariables.add(descriptor); } - public void triggerUsed(DeclarationDescriptor descriptor) { + public void triggerUsed(@NotNull DeclarationDescriptor descriptor) { if ((descriptor instanceof PropertyDescriptor || descriptor instanceof PropertyAccessorDescriptor)) { checkOuterClass(descriptor); } @@ -165,6 +165,21 @@ public final class UsageTracker { return false; } + public boolean isCaptured(@NotNull CallableDescriptor descriptor) { + if (capturedVariables != null && capturedVariables.contains(descriptor)) { + return true; + } + + if (children != null) { + for (UsageTracker child : children) { + if (child.isCaptured(descriptor)) { + return true; + } + } + } + return false; + } + // differs from DescriptorUtils - fails if reach PackageFragmentDescriptor private static boolean isAncestor( @NotNull DeclarationDescriptor ancestor, diff --git a/js/js.translator/src/org/jetbrains/k2js/translate/expression/ExpressionVisitor.java b/js/js.translator/src/org/jetbrains/k2js/translate/expression/ExpressionVisitor.java index 5131d175c08..dba8659c6c4 100644 --- a/js/js.translator/src/org/jetbrains/k2js/translate/expression/ExpressionVisitor.java +++ b/js/js.translator/src/org/jetbrains/k2js/translate/expression/ExpressionVisitor.java @@ -22,7 +22,6 @@ import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.jetbrains.jet.JetNodeTypes; import org.jetbrains.jet.lang.descriptors.DeclarationDescriptor; -import org.jetbrains.jet.lang.descriptors.FunctionDescriptor; import org.jetbrains.jet.lang.descriptors.VariableDescriptor; import org.jetbrains.jet.lang.psi.*; import org.jetbrains.jet.lang.resolve.BindingContext; @@ -409,11 +408,7 @@ public final class ExpressionVisitor extends TranslatorVisitor { @Override @NotNull public JsNode visitNamedFunction(@NotNull JetNamedFunction expression, @NotNull TranslationContext context) { - JsExpression alias = LiteralFunctionTranslator.translate(expression, context); - FunctionDescriptor descriptor = getFunctionDescriptor(context.bindingContext(), expression); - JsName name = context.scope().declareFreshName(descriptor.getName().asString()); - context.aliasingContext().registerAlias(descriptor, name.makeRef()); - return new JsVars(new JsVars.JsVar(name, alias)).source(expression); + return LiteralFunctionTranslator.translateLocalNamedFunction(expression, context).source(expression); } @Override diff --git a/js/js.translator/src/org/jetbrains/k2js/translate/expression/InnerDeclarationTranslator.java b/js/js.translator/src/org/jetbrains/k2js/translate/expression/InnerDeclarationTranslator.java index c6c790b8bd8..1b6cd8b691f 100644 --- a/js/js.translator/src/org/jetbrains/k2js/translate/expression/InnerDeclarationTranslator.java +++ b/js/js.translator/src/org/jetbrains/k2js/translate/expression/InnerDeclarationTranslator.java @@ -21,7 +21,6 @@ import com.intellij.util.Consumer; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.jetbrains.jet.lang.descriptors.CallableDescriptor; -import org.jetbrains.jet.lang.descriptors.VariableDescriptor; import org.jetbrains.k2js.translate.context.TranslationContext; import org.jetbrains.k2js.translate.context.UsageTracker; @@ -55,24 +54,24 @@ abstract class InnerDeclarationTranslator { usageTracker.forEachCaptured(new Consumer() { @Override public void consume(CallableDescriptor descriptor) { - JsName name; - if (descriptor instanceof VariableDescriptor) { - name = context.getNameForDescriptor(descriptor); - } - else { - JsExpression alias = context.getAliasForDescriptor(descriptor); - assert alias != null : "Alias not found for captured descriptor: " + descriptor; - name = ((JsNameRef) alias).getName(); - assert name != null : "Descriptor's alias don't have name: " + descriptor; - } - fun.getParameters().add(new JsParameter(name)); - invocationArguments.add(name.makeRef()); + fun.getParameters().add(new JsParameter(getParameterNameFor(descriptor))); + invocationArguments.add(getParameterNameRefFor(descriptor)); } }); } return invocation; } + @NotNull + protected JsName getParameterNameFor(@NotNull CallableDescriptor descriptor) { + return context.getNameForDescriptor(descriptor); + } + + @NotNull + protected JsNameRef getParameterNameRefFor(@NotNull CallableDescriptor descriptor) { + return getParameterNameFor(descriptor).makeRef(); + } + @NotNull protected abstract JsExpression createExpression(@NotNull JsNameRef nameRef, @Nullable JsExpression self); diff --git a/js/js.translator/src/org/jetbrains/k2js/translate/expression/InnerFunctionTranslator.java b/js/js.translator/src/org/jetbrains/k2js/translate/expression/InnerFunctionTranslator.java index c451c2d10da..95ac97b687b 100644 --- a/js/js.translator/src/org/jetbrains/k2js/translate/expression/InnerFunctionTranslator.java +++ b/js/js.translator/src/org/jetbrains/k2js/translate/expression/InnerFunctionTranslator.java @@ -20,20 +20,24 @@ import com.google.dart.compiler.backend.js.ast.*; import com.intellij.util.SmartList; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; +import org.jetbrains.jet.lang.descriptors.CallableDescriptor; import org.jetbrains.jet.lang.descriptors.ClassDescriptor; import org.jetbrains.jet.lang.descriptors.FunctionDescriptor; import org.jetbrains.k2js.translate.context.TranslationContext; class InnerFunctionTranslator extends InnerDeclarationTranslator { - private final FunctionDescriptor descriptor; + @NotNull private final FunctionDescriptor descriptor; + @Nullable private final JsNameRef funRef; public InnerFunctionTranslator( @NotNull FunctionDescriptor descriptor, @NotNull TranslationContext context, - @NotNull JsFunction fun + @NotNull JsFunction fun, + @Nullable JsNameRef funRef ) { super(context, fun); this.descriptor = descriptor; + this.funRef = funRef; } @SuppressWarnings("MethodOverloadsMethodOfSuperclass") @@ -42,6 +46,17 @@ class InnerFunctionTranslator extends InnerDeclarationTranslator { return translate(nameRef, getThis(outerContext)); } + @Override + @NotNull + protected JsNameRef getParameterNameRefFor(@NotNull CallableDescriptor descriptor) { + if (descriptor == this.descriptor) { + assert funRef != null; + return funRef; + } + + return super.getParameterNameRefFor(descriptor); + } + @Override @NotNull protected JsExpression createExpression(@NotNull JsNameRef nameRef, @Nullable JsExpression self) { diff --git a/js/js.translator/src/org/jetbrains/k2js/translate/expression/LiteralFunctionTranslator.java b/js/js.translator/src/org/jetbrains/k2js/translate/expression/LiteralFunctionTranslator.java index d95282a9369..c9d2b2463b1 100644 --- a/js/js.translator/src/org/jetbrains/k2js/translate/expression/LiteralFunctionTranslator.java +++ b/js/js.translator/src/org/jetbrains/k2js/translate/expression/LiteralFunctionTranslator.java @@ -17,6 +17,7 @@ package org.jetbrains.k2js.translate.expression; import com.google.dart.compiler.backend.js.ast.*; +import com.intellij.util.SmartList; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.jetbrains.jet.lang.descriptors.ClassDescriptor; @@ -28,12 +29,12 @@ import org.jetbrains.jet.lang.psi.JetClassOrObject; import org.jetbrains.jet.lang.psi.JetDeclarationWithBody; import org.jetbrains.jet.lang.resolve.DescriptorUtils; import org.jetbrains.k2js.translate.LabelGenerator; -import org.jetbrains.k2js.translate.context.AliasingContext; -import org.jetbrains.k2js.translate.context.Namer; -import org.jetbrains.k2js.translate.context.TranslationContext; -import org.jetbrains.k2js.translate.context.UsageTracker; +import org.jetbrains.k2js.translate.context.*; import org.jetbrains.k2js.translate.declaration.ClassTranslator; import org.jetbrains.k2js.translate.general.AbstractTranslator; +import org.jetbrains.k2js.translate.utils.JsAstUtils; + +import java.util.List; import static org.jetbrains.k2js.translate.utils.BindingUtils.getFunctionDescriptor; import static org.jetbrains.k2js.translate.utils.FunctionBodyTranslator.translateFunctionBody; @@ -41,6 +42,7 @@ import static org.jetbrains.k2js.translate.utils.JsDescriptorUtils.getExpectedRe public class LiteralFunctionTranslator extends AbstractTranslator { private static final LabelGenerator FUNCTION_NAME_GENERATOR = new LabelGenerator('f'); + private static final String CAPTURED_VALUE_FIELD = "v"; private final JetDeclarationWithBody declaration; private final FunctionDescriptor descriptor; @@ -49,6 +51,7 @@ public class LiteralFunctionTranslator extends AbstractTranslator { private final boolean inConstructorOrTopLevel; private final ClassDescriptor outerClass; private final JsName receiverName; + private JsNameRef tempRef = null; private LiteralFunctionTranslator(@NotNull JetDeclarationWithBody declaration, @NotNull TranslationContext context) { super(context); @@ -115,7 +118,7 @@ public class LiteralFunctionTranslator extends AbstractTranslator { else { JsNameRef funReference = context().define(FUNCTION_NAME_GENERATOR.generate(), jsFunction); - InnerFunctionTranslator innerTranslator = new InnerFunctionTranslator(descriptor, functionContext, jsFunction); + InnerFunctionTranslator innerTranslator = new InnerFunctionTranslator(descriptor, functionContext, jsFunction, tempRef); result = innerTranslator.translate(funReference, context()); } @@ -130,6 +133,52 @@ public class LiteralFunctionTranslator extends AbstractTranslator { return finish(); } + @NotNull + public JsVars translateLocalNamedFunction() { + // Add ability to capture this named function. + // Will be available like `foo.v` (for function `foo`) + // Can not generate direct call because function may have some closures. + JsName funName = functionContext.getNameForDescriptor(descriptor); + JsNameRef alias = new JsNameRef(CAPTURED_VALUE_FIELD, funName.makeRef()); + functionContext.aliasingContext().registerAlias(descriptor, alias); + + translateBody(); + + UsageTracker funTracker = functionContext.usageTracker(); + assert funTracker != null; + boolean funIsCaptured = funTracker.isCaptured(descriptor); + + // Create temporary variable name which will be contain reference to the function. + JsName temp; + if (funIsCaptured) { + assert !inConstructorOrTopLevel : "A recursive closure in constructor is unsupported."; + // Use `context()` because it should be created in the scope which contain call. + temp = context().scope().declareTemporary(); + tempRef = temp.makeRef(); + } + else { + temp = null; + } + + JsExpression result = finish(); + + List vars = new SmartList(); + + if (funIsCaptured) { + JsVars.JsVar tempVar = new JsVars.JsVar(temp, new JsObjectLiteral()); + vars.add(tempVar); + + // Save `result` to the field of temporary variable if the function is captured. + result = JsAstUtils.assignment(new JsNameRef(CAPTURED_VALUE_FIELD, temp.makeRef()), result); + + } + + JsVars.JsVar fun = new JsVars.JsVar(funName, result); + vars.add(fun); + + return new JsVars(vars, /*mulitline =*/ false); + } + private static void addRegularParameters( @NotNull FunctionDescriptor descriptor, @NotNull JsFunction fun, @@ -142,6 +191,11 @@ public class LiteralFunctionTranslator extends AbstractTranslator { FunctionTranslator.addParameters(fun.getParameters(), descriptor, funContext); } + @NotNull + public static JsVars translateLocalNamedFunction(@NotNull JetDeclarationWithBody declaration, @NotNull TranslationContext outerContext) { + return new LiteralFunctionTranslator(declaration, outerContext).translateLocalNamedFunction(); + } + @NotNull public static JsExpression translate(@NotNull JetDeclarationWithBody declaration, @NotNull TranslationContext outerContext) { return new LiteralFunctionTranslator(declaration, outerContext).translate(); diff --git a/js/js.translator/testFiles/closure/cases/closureFunctionByInnerFunction.kt b/js/js.translator/testFiles/closure/cases/closureFunctionByInnerFunction.kt new file mode 100644 index 00000000000..e50d88dad4d --- /dev/null +++ b/js/js.translator/testFiles/closure/cases/closureFunctionByInnerFunction.kt @@ -0,0 +1,46 @@ +/* + * Copyright 2010-2013 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 foo + +fun run(f: () -> T) = f() + +val r = "OK" + +fun simple(s: String? = null): String { + if (s != null) return s + + return run { + simple("OK") + } +} + +val ok = "OK" +fun withClosure(s: String? = null): String { + if (s != null) return s + + return ok + run { + withClosure(ok) + } +} + +fun box(): String { + if (simple("OK") != "OK") return "failed on simple recursion" + + if (withClosure() != ok + ok) return "failed when closure something" + + return "OK" +} diff --git a/js/js.translator/testFiles/closure/cases/closureLocalFunctionByInnerFunction.kt b/js/js.translator/testFiles/closure/cases/closureLocalFunctionByInnerFunction.kt new file mode 100644 index 00000000000..d00e2d3e274 --- /dev/null +++ b/js/js.translator/testFiles/closure/cases/closureLocalFunctionByInnerFunction.kt @@ -0,0 +1,44 @@ +/* + * Copyright 2010-2013 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 foo + +fun run(f: () -> T) = f() + +fun box(): String { + fun simple(s: String? = null): String { + if (s != null) return s + + return run { + simple("OK") + } + } + + if (simple("OK") != "OK") return "failed on simple recursion" + + val ok = "OK" + fun withClosure(s: String? = null): String { + if (s != null) return s + + return ok + run { + withClosure(ok) + } + } + + if (withClosure() != ok + ok) return "failed when closure something" + + return "OK" +} diff --git a/js/js.translator/testFiles/closure/cases/closureLocalFunctionByInnerFunctionInConstructor.kt b/js/js.translator/testFiles/closure/cases/closureLocalFunctionByInnerFunctionInConstructor.kt new file mode 100644 index 00000000000..42d94b6ff75 --- /dev/null +++ b/js/js.translator/testFiles/closure/cases/closureLocalFunctionByInnerFunctionInConstructor.kt @@ -0,0 +1,42 @@ +/* + * Copyright 2010-2013 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 foo + +fun run(f: () -> T) = f() + +class Foo { + val OK = "OK"; + var result: String = "" + { + fun bar(s: String? = null) { + if (s != null) { + result = s + return + } + + run { + bar(OK) + } + } + bar(); + } + +} + +fun box(): String { + return Foo().result +} diff --git a/js/js.translator/testFiles/closure/cases/simpleRecursion.kt b/js/js.translator/testFiles/closure/cases/simpleRecursion.kt new file mode 100644 index 00000000000..020ea1a83c2 --- /dev/null +++ b/js/js.translator/testFiles/closure/cases/simpleRecursion.kt @@ -0,0 +1,37 @@ +/* + * Copyright 2010-2013 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 foo + +fun bar(i: Int = 0): Int = if (i == 7) i else bar(i - 1) + +fun box(): String { + val a = bar(10) + if (a != 7) return "bar(10) = $a, but expected 7" + + fun boo(i: Int = 0): Int = if (i == 4) i else boo(i - 1) + val b = boo(17) + if (b != 4) return "boo(17) = $b, but expected 4" + + fun f() = 1 + val v = 3 + fun baz(i: Int = 0): Int = if (i == v) f() + v else baz(i - 1) + + val c = baz(10) + if (c != 4) return "baz(10) = $c, but expected 4" + + return "OK" +}