From 7afca74b9412d94cb738822fb3efe67325153410 Mon Sep 17 00:00:00 2001 From: Alexey Andreev Date: Sat, 26 Mar 2016 14:50:57 +0300 Subject: [PATCH] KT-11030 When constructor of object expression calls super constructor, place this call at object creation instead of object's constructor. This allows to capture outer this reference properly in certain cases. JVM backend uses the same approach --- .../codegen/box/classes/selfcreate.kt | 6 ++- .../translate/declaration/ClassTranslator.kt | 18 ++++---- .../declaration/DeclarationBodyVisitor.java | 6 +-- .../translate/declaration/FileDeclaration.kt | 4 +- .../expression/ExpressionVisitor.java | 29 ++++++++++++- .../ClassInitializerTranslator.java | 41 ++++++++----------- .../js/translate/utils/BindingUtils.java | 22 ++++++---- 7 files changed, 81 insertions(+), 45 deletions(-) diff --git a/compiler/testData/codegen/box/classes/selfcreate.kt b/compiler/testData/codegen/box/classes/selfcreate.kt index 4423f0e84a9..95d67471511 100644 --- a/compiler/testData/codegen/box/classes/selfcreate.kt +++ b/compiler/testData/codegen/box/classes/selfcreate.kt @@ -5,6 +5,10 @@ open class A(val b : B) { } fun box() : String { - A(B()).a() + val b = B() + val a = A(b).a() + + if (a.b !== b) return "failed" + return "OK" } 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 6ef2cabb7e8..861edbcc080 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 @@ -33,8 +33,7 @@ import org.jetbrains.kotlin.js.translate.expression.FunctionTranslator import org.jetbrains.kotlin.js.translate.general.AbstractTranslator import org.jetbrains.kotlin.js.translate.initializer.ClassInitializerTranslator import org.jetbrains.kotlin.js.translate.utils.* -import org.jetbrains.kotlin.js.translate.utils.BindingUtils.getClassDescriptor -import org.jetbrains.kotlin.js.translate.utils.BindingUtils.getPropertyDescriptorForConstructorParameter +import org.jetbrains.kotlin.js.translate.utils.BindingUtils.* import org.jetbrains.kotlin.js.translate.utils.JsDescriptorUtils.getSupertypesWithoutFakes import org.jetbrains.kotlin.js.translate.utils.PsiUtils.getPrimaryConstructorParameters import org.jetbrains.kotlin.js.translate.utils.TranslationUtils.simpleReturnFunction @@ -62,15 +61,17 @@ class ClassTranslator private constructor( private val secondaryConstructors = mutableListOf() private val secondaryConstructorProperties = mutableListOf() private var primaryConstructor: ConstructorInfo? = null + private lateinit var definitionPlace: DefinitionPlace - private fun translate(): List { + fun translate(): TranslationResult { invocationArguments.clear() getClassCreateInvocationArguments() val classNameRef = context().getNameForDescriptor(descriptor).makeRef() val classCreation = JsInvocation(context().namer().classCreateInvocation(descriptor), invocationArguments) - return listOf(JsPropertyInitializer(classNameRef, classCreation)) + secondaryConstructorProperties + val properties = listOf(JsPropertyInitializer(classNameRef, classCreation)) + secondaryConstructorProperties + return TranslationResult(properties, definitionPlace) } private fun isTrait(): Boolean = descriptor.kind == ClassKind.INTERFACE @@ -86,7 +87,7 @@ class ClassTranslator private constructor( val definitionPlace = DefinitionPlace(scope as JsObjectScope, qualifiedReference, staticProperties) context = context.newDeclaration(descriptor, definitionPlace) - invocationArguments.add(getSuperclassReferences(context)) + invocationArguments += getSuperclassReferences(context) val nonConstructorContext = context.innerWithUsageTracker(scope, descriptor) val delegationTranslator = DelegationTranslator(classDeclaration, nonConstructorContext) @@ -121,7 +122,7 @@ class ClassTranslator private constructor( dataClassGenerator.generate() } - val hasStaticProperties = !staticProperties.isEmpty() + val hasStaticProperties = !staticProperties.isEmpty() || DescriptorUtils.isAnonymousObject(descriptor) if (!properties.isEmpty() || hasStaticProperties) { if (properties.isEmpty()) { invocationArguments += JsLiteral.NULL @@ -140,6 +141,7 @@ class ClassTranslator private constructor( if (primaryConstructor != null && primaryConstructor!!.function.body.isEmpty) { invocationArguments[invocationArguments.indexOf(primaryConstructor!!.function)] = JsLiteral.NULL } + this.definitionPlace = definitionPlace } private fun translatePrimaryConstructor(classContext: TranslationContext, delegationTranslator: DelegationTranslator) { @@ -426,11 +428,13 @@ class ClassTranslator private constructor( } companion object { - fun translate(classDeclaration: KtClassOrObject, context: TranslationContext): List { + @JvmStatic fun translate(classDeclaration: KtClassOrObject, context: TranslationContext): TranslationResult { return ClassTranslator(classDeclaration, context).translate() } } + class TranslationResult(val properties: List, val definitionPlace: DefinitionPlace) + private class ConstructorInfo( val function: JsFunction, val context: TranslationContext, diff --git a/js/js.translator/src/org/jetbrains/kotlin/js/translate/declaration/DeclarationBodyVisitor.java b/js/js.translator/src/org/jetbrains/kotlin/js/translate/declaration/DeclarationBodyVisitor.java index 233d58d46d5..09dc637f9e9 100644 --- a/js/js.translator/src/org/jetbrains/kotlin/js/translate/declaration/DeclarationBodyVisitor.java +++ b/js/js.translator/src/org/jetbrains/kotlin/js/translate/declaration/DeclarationBodyVisitor.java @@ -63,7 +63,7 @@ public class DeclarationBodyVisitor extends TranslatorVisitor { @Override public Void visitClass(@NotNull KtClass declaration, TranslationContext context) { - staticResult.addAll(ClassTranslator.Companion.translate(declaration, context)); + staticResult.addAll(ClassTranslator.translate(declaration, context).getProperties()); return null; } @@ -73,7 +73,7 @@ public class DeclarationBodyVisitor extends TranslatorVisitor { ClassDescriptor descriptor = getClassDescriptor(data.bindingContext(), enumEntry); List supertypes = getSupertypesWithoutFakes(descriptor); if (enumEntry.getBody() != null || supertypes.size() > 1) { - enumEntryList.addAll(ClassTranslator.Companion.translate(enumEntry, data)); + enumEntryList.addAll(ClassTranslator.translate(enumEntry, data).getProperties()); } else { assert supertypes.size() == 1 : "Simple Enum entry must have one supertype"; jsEnumEntryCreation = new ClassInitializerTranslator(enumEntry, data).generateEnumEntryInstanceCreation(supertypes.get(0)); @@ -84,7 +84,7 @@ public class DeclarationBodyVisitor extends TranslatorVisitor { @Override public Void visitObjectDeclaration(@NotNull KtObjectDeclaration declaration, TranslationContext context) { - staticResult.addAll(ClassTranslator.Companion.translate(declaration, context)); + staticResult.addAll(ClassTranslator.translate(declaration, context).getProperties()); return null; } diff --git a/js/js.translator/src/org/jetbrains/kotlin/js/translate/declaration/FileDeclaration.kt b/js/js.translator/src/org/jetbrains/kotlin/js/translate/declaration/FileDeclaration.kt index f72dac1c425..efa274bc0cb 100644 --- a/js/js.translator/src/org/jetbrains/kotlin/js/translate/declaration/FileDeclaration.kt +++ b/js/js.translator/src/org/jetbrains/kotlin/js/translate/declaration/FileDeclaration.kt @@ -50,7 +50,7 @@ class FileDeclarationVisitor( } override fun visitClass(declaration: KtClass, context: TranslationContext?): Void? { - result.addAll(ClassTranslator.translate(declaration, context!!)) + result += ClassTranslator.translate(declaration, context!!).properties return null } @@ -58,7 +58,7 @@ class FileDeclarationVisitor( context!! // TODO: avoid duplication with superclass - result.addAll(ClassTranslator.translate(declaration, context)) + result += ClassTranslator.translate(declaration, context).properties return null } diff --git a/js/js.translator/src/org/jetbrains/kotlin/js/translate/expression/ExpressionVisitor.java b/js/js.translator/src/org/jetbrains/kotlin/js/translate/expression/ExpressionVisitor.java index b040791626a..382659fa143 100644 --- a/js/js.translator/src/org/jetbrains/kotlin/js/translate/expression/ExpressionVisitor.java +++ b/js/js.translator/src/org/jetbrains/kotlin/js/translate/expression/ExpressionVisitor.java @@ -43,6 +43,7 @@ import org.jetbrains.kotlin.psi.psiUtil.PsiUtilsKt; import org.jetbrains.kotlin.resolve.BindingContext; import org.jetbrains.kotlin.resolve.BindingContextUtils; import org.jetbrains.kotlin.resolve.bindingContextUtil.BindingContextUtilsKt; +import org.jetbrains.kotlin.resolve.calls.model.ResolvedCall; import org.jetbrains.kotlin.resolve.constants.CompileTimeConstant; import org.jetbrains.kotlin.resolve.constants.ConstantValue; import org.jetbrains.kotlin.resolve.constants.NullValue; @@ -524,7 +525,8 @@ public final class ExpressionVisitor extends TranslatorVisitor { JsScope scope = context.getScopeForDescriptor(descriptor); TranslationContext classContext = context.innerWithUsageTracker(scope, descriptor); - List properties = ClassTranslator.Companion.translate(expression.getObjectDeclaration(), classContext); + ClassTranslator.TranslationResult result = ClassTranslator.translate(expression.getObjectDeclaration(), classContext); + List properties = result.getProperties(); context.getDefinitionPlace().getProperties().addAll(properties); JsExpression constructor = context.getQualifiedReference(descriptor); @@ -536,6 +538,29 @@ public final class ExpressionVisitor extends TranslatorVisitor { } } + // In case of object expressions like this: + // object : SuperClass(A, B, ...) + // we may capture local variables in expressions A, B, etc. We don't want to generate local fields for these variables. + // Our ClassTranslator is capable of such thing, but case of object expression is a little special. + // Consider the following: + // + // class A(val x: Int) { + // fun foo() { object : A(x) } + // + // By calling A(x) super constructor we capture `this` explicitly. However, we can't tell which `A::this` we are mentioning, + // either `this` of an object literal or `this` of enclosing `class A`. + // Frontend treats it as `this` of enclosing class declaration, therefore it expects backend to generate + // super call in scope of `fun foo()` rather than define inner scope for object's constructor. + // Thus we generate this call here rather than relying on ClassTranslator. + ResolvedCall superCall = BindingUtils.getSuperCall(context.bindingContext(), + expression.getObjectDeclaration()); + if (superCall != null) { + assert context.getDeclarationDescriptor() != null : "This expression should be inside declaration: " + + PsiUtilsKt.getTextWithLocation(expression); + TranslationContext superCallContext = context.newDeclaration(context.getDeclarationDescriptor(), result.getDefinitionPlace()); + closureArgs.addAll(CallArgumentTranslator.translate(superCall, null, superCallContext).getTranslateArguments()); + } + return new JsNew(constructor, closureArgs); } @@ -579,7 +604,7 @@ public final class ExpressionVisitor extends TranslatorVisitor { JsScope scope = context.getScopeForDescriptor(descriptor); TranslationContext classContext = context.innerWithUsageTracker(scope, descriptor); - context.getDefinitionPlace().getProperties().addAll(ClassTranslator.Companion.translate(klass, classContext)); + context.getDefinitionPlace().getProperties().addAll(ClassTranslator.translate(klass, classContext).getProperties()); return JsEmpty.INSTANCE; } diff --git a/js/js.translator/src/org/jetbrains/kotlin/js/translate/initializer/ClassInitializerTranslator.java b/js/js.translator/src/org/jetbrains/kotlin/js/translate/initializer/ClassInitializerTranslator.java index d75b0890f30..70f558a218c 100644 --- a/js/js.translator/src/org/jetbrains/kotlin/js/translate/initializer/ClassInitializerTranslator.java +++ b/js/js.translator/src/org/jetbrains/kotlin/js/translate/initializer/ClassInitializerTranslator.java @@ -19,7 +19,6 @@ package org.jetbrains.kotlin.js.translate.initializer; 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.kotlin.descriptors.*; import org.jetbrains.kotlin.js.translate.callTranslator.CallTranslator; import org.jetbrains.kotlin.js.translate.context.Namer; @@ -28,13 +27,14 @@ import org.jetbrains.kotlin.js.translate.context.UsageTracker; import org.jetbrains.kotlin.js.translate.declaration.DelegationTranslator; import org.jetbrains.kotlin.js.translate.general.AbstractTranslator; import org.jetbrains.kotlin.js.translate.reference.CallArgumentTranslator; +import org.jetbrains.kotlin.js.translate.utils.BindingUtils; import org.jetbrains.kotlin.js.translate.utils.JsAstUtils; import org.jetbrains.kotlin.js.translate.utils.jsAstUtils.AstUtilsKt; import org.jetbrains.kotlin.lexer.KtTokens; -import org.jetbrains.kotlin.psi.*; -import org.jetbrains.kotlin.psi.psiUtil.PsiUtilsKt; +import org.jetbrains.kotlin.psi.KtClassOrObject; +import org.jetbrains.kotlin.psi.KtEnumEntry; +import org.jetbrains.kotlin.psi.KtParameter; import org.jetbrains.kotlin.resolve.DescriptorUtils; -import org.jetbrains.kotlin.resolve.calls.callUtil.CallUtilKt; import org.jetbrains.kotlin.resolve.calls.model.ResolvedCall; import org.jetbrains.kotlin.types.KotlinType; @@ -132,7 +132,7 @@ public final class ClassInitializerTranslator extends AbstractTranslator { @NotNull public JsExpression generateEnumEntryInstanceCreation(@NotNull KotlinType enumClassType) { - ResolvedCall superCall = getSuperCall(); + ResolvedCall superCall = getSuperCall(bindingContext(), classDeclaration); if (superCall == null) { ClassDescriptor classDescriptor = getClassDescriptorForType(enumClassType); @@ -149,7 +149,7 @@ public final class ClassInitializerTranslator extends AbstractTranslator { return; } if (hasAncestorClass(bindingContext(), classDeclaration)) { - ResolvedCall superCall = getSuperCall(); + ResolvedCall superCall = BindingUtils.getSuperCall(bindingContext(), classDeclaration); if (superCall == null) return; if (classDeclaration instanceof KtEnumEntry) { @@ -161,9 +161,7 @@ public final class ClassInitializerTranslator extends AbstractTranslator { else { List arguments = new ArrayList(); - ClassDescriptor superDescriptor = DescriptorUtils.getSuperClassDescriptor(descriptor); - assert superDescriptor != null : "This class is expected to have super class: " - + PsiUtilsKt.getTextWithLocation(classDeclaration); + ConstructorDescriptor superDescriptor = (ConstructorDescriptor) superCall.getResultingDescriptor(); List superclassClosure = context.getLocalClassClosure(superDescriptor); UsageTracker tracker = context.usageTracker(); @@ -174,11 +172,20 @@ public final class ClassInitializerTranslator extends AbstractTranslator { } } - if (superDescriptor.isInner() && descriptor.isInner()) { + if (superDescriptor.getContainingDeclaration().isInner() && descriptor.isInner()) { arguments.add(fqnWithoutSideEffects(Namer.OUTER_FIELD_NAME, JsLiteral.THIS)); } - arguments.addAll(CallArgumentTranslator.translate(superCall, null, context()).getTranslateArguments()); + if (!DescriptorUtils.isAnonymousObject(descriptor)) { + arguments.addAll(CallArgumentTranslator.translate(superCall, null, context()).getTranslateArguments()); + } + else { + for (ValueParameterDescriptor parameter : superDescriptor.getValueParameters()) { + JsName parameterName = context.getNameForDescriptor(parameter); + arguments.add(parameterName.makeRef()); + initializer.getParameters().add(new JsParameter(parameterName)); + } + } addCallToSuperMethod(arguments, initializer); } @@ -194,18 +201,6 @@ public final class ClassInitializerTranslator extends AbstractTranslator { initializerStatements.add(0, call.makeStmt()); } - @Nullable - private ResolvedCall getSuperCall() { - for (KtSuperTypeListEntry specifier : classDeclaration.getSuperTypeListEntries()) { - if (specifier instanceof KtSuperTypeCallEntry) { - KtSuperTypeCallEntry superCall = (KtSuperTypeCallEntry) specifier; - //noinspection unchecked - return (ResolvedCall) CallUtilKt.getResolvedCallWithAssert(superCall, bindingContext()); - } - } - return null; - } - @NotNull private List translatePrimaryConstructorParameters() { List parameterList = getPrimaryConstructorParameters(classDeclaration); diff --git a/js/js.translator/src/org/jetbrains/kotlin/js/translate/utils/BindingUtils.java b/js/js.translator/src/org/jetbrains/kotlin/js/translate/utils/BindingUtils.java index 0b5a2b87eb8..567f6824f91 100644 --- a/js/js.translator/src/org/jetbrains/kotlin/js/translate/utils/BindingUtils.java +++ b/js/js.translator/src/org/jetbrains/kotlin/js/translate/utils/BindingUtils.java @@ -25,6 +25,7 @@ import org.jetbrains.kotlin.resolve.BindingContext; import org.jetbrains.kotlin.resolve.BindingContextUtils; import org.jetbrains.kotlin.resolve.DescriptorToSourceUtils; import org.jetbrains.kotlin.resolve.DescriptorUtils; +import org.jetbrains.kotlin.resolve.calls.callUtil.CallUtilKt; import org.jetbrains.kotlin.resolve.calls.model.ResolvedCall; import org.jetbrains.kotlin.resolve.constants.CompileTimeConstant; import org.jetbrains.kotlin.resolve.constants.evaluate.ConstantExpressionEvaluator; @@ -95,12 +96,6 @@ public final class BindingUtils { return BindingContextUtils.getNotNull(context, BindingContext.TYPE, typeReference); } - @NotNull - public static ClassDescriptor getClassDescriptorForTypeReference(@NotNull BindingContext context, - @NotNull KtTypeReference typeReference) { - return DescriptorUtils.getClassDescriptorForType(getTypeByReference(context, typeReference)); - } - @Nullable public static PropertyDescriptor getPropertyDescriptorForConstructorParameter(@NotNull BindingContext context, @NotNull KtParameter parameter) { @@ -117,7 +112,7 @@ public final class BindingUtils { } @Nullable - public static DeclarationDescriptor getNullableDescriptorForReferenceExpression(@NotNull BindingContext context, + private static DeclarationDescriptor getNullableDescriptorForReferenceExpression(@NotNull BindingContext context, @NotNull KtReferenceExpression reference) { return context.get(BindingContext.REFERENCE_TARGET, reference); } @@ -215,4 +210,17 @@ public final class BindingUtils { boolean isGet) { return BindingContextUtils.getNotNull(context, isGet ? INDEXED_LVALUE_GET : INDEXED_LVALUE_SET, arrayAccessExpression); } + + + @Nullable + @SuppressWarnings("unchecked") + public static ResolvedCall getSuperCall(@NotNull BindingContext context, KtClassOrObject classDeclaration) { + for (KtSuperTypeListEntry specifier : classDeclaration.getSuperTypeListEntries()) { + if (specifier instanceof KtSuperTypeCallEntry) { + KtSuperTypeCallEntry superCall = (KtSuperTypeCallEntry) specifier; + return (ResolvedCall) CallUtilKt.getResolvedCallWithAssert(superCall, context); + } + } + return null; + } }