From 170a671a1b69f13f5a03ab12dae9e878f6013312 Mon Sep 17 00:00:00 2001 From: Alexey Andreev Date: Thu, 12 May 2016 18:54:42 +0300 Subject: [PATCH] KT-11960 Fix for case when class instantiates itself --- .../localClassesWithSelfInstantiation.kt | 72 +++++++++++++++++++ .../codegen/BlackBoxCodegenTestGenerated.java | 6 ++ .../SecondaryConstructorTestGenerated.java | 6 ++ .../callTranslator/FunctionCallCases.kt | 25 ++++--- .../js/translate/context/DeferredCallSite.kt | 38 ++++++++++ .../js/translate/context/StaticContext.java | 17 ++++- .../translate/context/TranslationContext.java | 50 +++++++++++-- .../translate/declaration/ClassTranslator.kt | 23 +++++- 8 files changed, 217 insertions(+), 20 deletions(-) create mode 100644 compiler/testData/codegen/box/secondaryConstructors/localClassesWithSelfInstantiation.kt create mode 100644 js/js.translator/src/org/jetbrains/kotlin/js/translate/context/DeferredCallSite.kt diff --git a/compiler/testData/codegen/box/secondaryConstructors/localClassesWithSelfInstantiation.kt b/compiler/testData/codegen/box/secondaryConstructors/localClassesWithSelfInstantiation.kt new file mode 100644 index 00000000000..b559b0d3bab --- /dev/null +++ b/compiler/testData/codegen/box/secondaryConstructors/localClassesWithSelfInstantiation.kt @@ -0,0 +1,72 @@ +// Enable for JVM backend when KT-8120 gets fixed +// TARGET_BACKEND: JS + +fun box(): String { + val capturedInConstructor = 1 + val capturedInBody = 10 + + class A(var x: Int) { + var y = 0 + + fun copy(): A { + val result = A(x) + result.y += capturedInBody + return result + } + + init { + y += x + capturedInConstructor + } + } + + val a = A(100).copy() + if (a.y != 111) return "fail1a: ${a.y}" + if (a.x != 100) return "fail1b: ${a.x}" + + // This does not work in JS backend due to some unrelated issue with lambdas + /* + class B(var x: Int) { + var y = 0 + + fun copier(): () -> B = { + val result = B(x) + result.y += capturedInBody + result + } + + init { + y += x + capturedInConstructor + } + } + + val b = B(100).copier()() + if (b.y != 111) return "fail2a: ${b.y}" + if (b.x != 100) return "fail2b: ${b.x}" + */ + + // It's pretty hard to implement this properly for now. Presumably, one needs to inject closure fields into local classes + // after the entire code gets generated (which is possible, but not easy, in JS and, I believe, nearly impossible in JVM). + /* + class C(var x: Int) { + var y = 0 + + inner class D() { + fun copyOuter(): C { + val result = C(x) + result.y += capturedInBody + return result + } + } + + init { + y += x + capturedInConstructor + } + } + + val c = C(100).D().copyOuter() + if (c.y != 111) return "fail3a: ${c.y}" + if (c.x != 100) return "fail3b: ${c.x}" + */ + + return "OK" +} \ No newline at end of file diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java index b627d4e3933..60225f3968c 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java @@ -13009,6 +13009,12 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest { doTest(fileName); } + @TestMetadata("localClassesWithSelfInstantiation.kt") + public void testLocalClassesWithSelfInstantiation() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/secondaryConstructors/localClassesWithSelfInstantiation.kt"); + doTest(fileName); + } + @TestMetadata("superCallPrimary.kt") public void testSuperCallPrimary() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/secondaryConstructors/superCallPrimary.kt"); diff --git a/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/SecondaryConstructorTestGenerated.java b/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/SecondaryConstructorTestGenerated.java index e2c64f89ada..92832938801 100644 --- a/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/SecondaryConstructorTestGenerated.java +++ b/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/SecondaryConstructorTestGenerated.java @@ -161,6 +161,12 @@ public class SecondaryConstructorTestGenerated extends AbstractSecondaryConstruc doTest(fileName); } + @TestMetadata("localClassesWithSelfInstantiation.kt") + public void testLocalClassesWithSelfInstantiation() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/secondaryConstructors/localClassesWithSelfInstantiation.kt"); + doTest(fileName); + } + @TestMetadata("superCallPrimary.kt") public void testSuperCallPrimary() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/secondaryConstructors/superCallPrimary.kt"); diff --git a/js/js.translator/src/org/jetbrains/kotlin/js/translate/callTranslator/FunctionCallCases.kt b/js/js.translator/src/org/jetbrains/kotlin/js/translate/callTranslator/FunctionCallCases.kt index 068de5c1eed..64a2c2850d4 100644 --- a/js/js.translator/src/org/jetbrains/kotlin/js/translate/callTranslator/FunctionCallCases.kt +++ b/js/js.translator/src/org/jetbrains/kotlin/js/translate/callTranslator/FunctionCallCases.kt @@ -204,20 +204,29 @@ object ConstructorCallCase : FunctionCallCase() { override fun FunctionCallInfo.dispatchReceiver() = doTranslate { argsWithReceiver(dispatchReceiver!!) } - private inline fun FunctionCallInfo.doTranslate(getArguments: CallArgumentTranslator.ArgumentsInfo.() -> List): JsExpression { + private inline fun FunctionCallInfo.doTranslate( + getArguments: CallArgumentTranslator.ArgumentsInfo.() -> List + ): JsExpression { val fqName = context.getQualifiedReference(callableDescriptor) val functionRef = if (isNative()) fqName else context.aliasOrValue(callableDescriptor) { fqName } - val arguments = argumentsInfo.getArguments() + + val invocationArguments = mutableListOf() val constructorDescriptor = callableDescriptor as ConstructorDescriptor - val closure = context.getClassOrConstructorClosure(constructorDescriptor) - val closureArgs = closure?.map { context.getArgumentForClosureConstructor(it) } ?: emptyList() - - if (constructorDescriptor.isPrimary || AnnotationsUtils.isNativeObject(constructorDescriptor)) { - return JsNew(functionRef, closureArgs + arguments) + if (context.isDeferred(constructorDescriptor)) { + context.deferConstructorCall(constructorDescriptor, invocationArguments) } else { - return JsInvocation(functionRef, closureArgs + arguments) + val closure = context.getClassOrConstructorClosure(constructorDescriptor) + invocationArguments += closure?.map { context.getArgumentForClosureConstructor(it) }.orEmpty() + } + + invocationArguments += argumentsInfo.getArguments() + return if (constructorDescriptor.isPrimary || AnnotationsUtils.isNativeObject(constructorDescriptor)) { + JsNew(functionRef, invocationArguments) + } + else { + JsInvocation(functionRef, invocationArguments) } } } diff --git a/js/js.translator/src/org/jetbrains/kotlin/js/translate/context/DeferredCallSite.kt b/js/js.translator/src/org/jetbrains/kotlin/js/translate/context/DeferredCallSite.kt new file mode 100644 index 00000000000..9a3b8177855 --- /dev/null +++ b/js/js.translator/src/org/jetbrains/kotlin/js/translate/context/DeferredCallSite.kt @@ -0,0 +1,38 @@ +/* + * Copyright 2010-2016 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. + */ + +/* + * Copyright 2010-2016 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.translate.context + +import com.google.dart.compiler.backend.js.ast.JsExpression +import org.jetbrains.kotlin.descriptors.ConstructorDescriptor + +class DeferredCallSite(val constructor: ConstructorDescriptor, val invocationArgs: MutableList) diff --git a/js/js.translator/src/org/jetbrains/kotlin/js/translate/context/StaticContext.java b/js/js.translator/src/org/jetbrains/kotlin/js/translate/context/StaticContext.java index b5dd81db64f..6c000a168e9 100644 --- a/js/js.translator/src/org/jetbrains/kotlin/js/translate/context/StaticContext.java +++ b/js/js.translator/src/org/jetbrains/kotlin/js/translate/context/StaticContext.java @@ -23,6 +23,8 @@ import com.google.dart.compiler.backend.js.ast.metadata.HasMetadata; import com.google.dart.compiler.backend.js.ast.metadata.MetadataProperties; import com.intellij.openapi.util.Factory; import com.intellij.util.containers.ContainerUtil; +import com.intellij.util.containers.HashMap; +import com.intellij.util.containers.HashSet; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.jetbrains.kotlin.builtins.ReflectionTypes; @@ -41,6 +43,7 @@ import org.jetbrains.kotlin.resolve.calls.util.FakeCallableDescriptorForObject; import java.util.List; import java.util.Map; +import java.util.Set; import static org.jetbrains.kotlin.js.translate.utils.AnnotationsUtils.*; import static org.jetbrains.kotlin.js.translate.utils.JsAstUtils.fqnWithoutSideEffects; @@ -98,7 +101,10 @@ public final class StaticContext { private final Map scopeToFunction = Maps.newHashMap(); @NotNull - private final Map> localClassesClosure = Maps.newHashMap(); + private final Map> classOrConstructorClosure = Maps.newHashMap(); + + @NotNull + private final Map> deferredCallSites = new HashMap>(); @NotNull private final JsConfig config; @@ -658,12 +664,17 @@ public final class StaticContext { } public void putClassOrConstructorClosure(@NotNull MemberDescriptor localClass, @NotNull List closure) { - localClassesClosure.put(localClass, Lists.newArrayList(closure)); + classOrConstructorClosure.put(localClass, Lists.newArrayList(closure)); } @Nullable public List getClassOrConstructorClosure(@NotNull MemberDescriptor descriptor) { - List result = localClassesClosure.get(descriptor); + List result = classOrConstructorClosure.get(descriptor); return result != null ? Lists.newArrayList(result) : null; } + + @NotNull + public Map> getDeferredCallSites() { + return deferredCallSites; + } } diff --git a/js/js.translator/src/org/jetbrains/kotlin/js/translate/context/TranslationContext.java b/js/js.translator/src/org/jetbrains/kotlin/js/translate/context/TranslationContext.java index 89df0deb991..144f1148ce3 100644 --- a/js/js.translator/src/org/jetbrains/kotlin/js/translate/context/TranslationContext.java +++ b/js/js.translator/src/org/jetbrains/kotlin/js/translate/context/TranslationContext.java @@ -18,6 +18,8 @@ package org.jetbrains.kotlin.js.translate.context; import com.google.dart.compiler.backend.js.ast.*; import com.intellij.psi.PsiElement; +import kotlin.Unit; +import kotlin.jvm.functions.Function1; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.jetbrains.kotlin.builtins.ReflectionTypes; @@ -32,9 +34,7 @@ import org.jetbrains.kotlin.resolve.BindingTrace; import org.jetbrains.kotlin.resolve.DescriptorUtils; import org.jetbrains.kotlin.resolve.scopes.receivers.ExtensionReceiver; -import java.util.HashMap; -import java.util.List; -import java.util.Map; +import java.util.*; import static org.jetbrains.kotlin.js.translate.context.UsageTrackerKt.getNameForCapturedDescriptor; import static org.jetbrains.kotlin.js.translate.utils.BindingUtils.getDescriptorForElement; @@ -454,10 +454,13 @@ public class TranslationContext { } @Nullable - public List getClassOrConstructorClosure(@NotNull MemberDescriptor localClass) { - List result = staticContext.getClassOrConstructorClosure(localClass); - if (result == null && localClass instanceof ConstructorDescriptor && ((ConstructorDescriptor) localClass).isPrimary()) { - result = staticContext.getClassOrConstructorClosure((ClassDescriptor) localClass.getContainingDeclaration()); + public List getClassOrConstructorClosure(@NotNull MemberDescriptor classOrConstructor) { + List result = staticContext.getClassOrConstructorClosure(classOrConstructor); + if (result == null && + classOrConstructor instanceof ConstructorDescriptor && + ((ConstructorDescriptor) classOrConstructor).isPrimary() + ) { + result = staticContext.getClassOrConstructorClosure((ClassDescriptor) classOrConstructor.getContainingDeclaration()); } return result; } @@ -506,4 +509,37 @@ public class TranslationContext { return staticContext.getScopeForDescriptor(descriptor).declareName(Namer.OUTER_FIELD_NAME); } + + public void startDeclaration() { + ClassDescriptor classDescriptor = this.classDescriptor; + if (classDescriptor != null && !(classDescriptor.getContainingDeclaration() instanceof ClassOrPackageFragmentDescriptor)) { + staticContext.getDeferredCallSites().put(classDescriptor, new ArrayList()); + } + } + + @NotNull + public List endDeclaration() { + List result = null; + ClassDescriptor classDescriptor = this.classDescriptor; + if (classDescriptor != null) { + result = staticContext.getDeferredCallSites().remove(classDescriptor); + } + if (result == null) { + result = Collections.emptyList(); + } + return result; + } + + public boolean isDeferred(@NotNull ConstructorDescriptor constructor) { + ClassDescriptor classDescriptor = constructor.getContainingDeclaration(); + return staticContext.getDeferredCallSites().containsKey(classDescriptor); + } + + public void deferConstructorCall(@NotNull ConstructorDescriptor constructor, @NotNull List invocationArgs) { + ClassDescriptor classDescriptor = constructor.getContainingDeclaration(); + List callSites = staticContext.getDeferredCallSites().get(classDescriptor); + if (callSites == null) throw new IllegalStateException("This method should be call only when `isDeferred` method " + + "reports true for given constructor: " + constructor); + callSites.add(new DeferredCallSite(constructor, invocationArgs)); + } } diff --git a/js/js.translator/src/org/jetbrains/kotlin/js/translate/declaration/ClassTranslator.kt b/js/js.translator/src/org/jetbrains/kotlin/js/translate/declaration/ClassTranslator.kt index bf630cde09a..49ddd8e6dde 100644 --- a/js/js.translator/src/org/jetbrains/kotlin/js/translate/declaration/ClassTranslator.kt +++ b/js/js.translator/src/org/jetbrains/kotlin/js/translate/declaration/ClassTranslator.kt @@ -88,6 +88,7 @@ class ClassTranslator private constructor( invocationArguments += getSuperclassReferences(context) val nonConstructorContext = context.innerWithUsageTracker(scope, descriptor) + nonConstructorContext.startDeclaration() val delegationTranslator = DelegationTranslator(classDeclaration, nonConstructorContext) translatePropertiesAsConstructorParameters(nonConstructorContext, properties) val bodyVisitor = DeclarationBodyVisitor(properties, staticProperties, scope) @@ -105,7 +106,7 @@ class ClassTranslator private constructor( val dataClassGenerator = JsDataClassGenerator(classDeclaration, context, properties) - emitConstructors(nonConstructorContext) + emitConstructors(nonConstructorContext, nonConstructorContext.endDeclaration()) for (constructor in allConstructors) { addClosureParameters(constructor, nonConstructorContext, dataClassGenerator) } @@ -240,10 +241,19 @@ class ClassTranslator private constructor( return if (primary != null) sequenceOf(primary) + secondaryConstructors else secondaryConstructors.asSequence() } - private fun emitConstructors(nonConstructorContext: TranslationContext) { + private fun emitConstructors(nonConstructorContext: TranslationContext, callSites: List) { // Build map that maps constructor to all constructors called via this() val constructorMap = allConstructors.map { it.descriptor to it }.toMap() + fun mapConstructor(constructor: ConstructorDescriptor) = + if (constructor.isPrimary) constructor.containingDeclaration else constructor + + val callSiteMap = callSites + .map { mapConstructor(it.constructor) }.distinct() + .map { Pair(it, mutableListOf()) } + .toMap() + callSites.forEach { callSiteMap[mapConstructor(it.constructor)]!! += it } + val thisCalls = secondaryConstructors.map { val set = mutableSetOf() val descriptor = it.descriptor @@ -273,6 +283,15 @@ class ClassTranslator private constructor( val descriptor = constructor.descriptor nonConstructorContext.putClassOrConstructorClosure(descriptor, capturedVars) + + val constructorCallSites = callSiteMap[constructor.descriptor].orEmpty() + for (callSite in constructorCallSites) { + capturedVars.forEach { nonConstructorUsageTracker.used(it) } + val closureArgs = capturedVars + .map { nonConstructorUsageTracker.capturedDescriptorToJsName[it]!! } + .map { JsAstUtils.fqnWithoutSideEffects(it, JsLiteral.THIS) } + callSite.invocationArgs.addAll(0, closureArgs) + } } }