From 5c8368847a49d524ac38a1d943f86814f8c1a3d2 Mon Sep 17 00:00:00 2001 From: Igor Chevdar Date: Thu, 9 Feb 2017 16:53:47 +0500 Subject: [PATCH] Containing declaration elimination (#225) * rewrote AbstractClosureAnnotator not using containingDeclaration * bug fix * bug fix * review fixes + tests * review fixes * refactoring * fix --- .../common/lower/AbstractClosureAnnotator.kt | 78 ++++++++++--------- .../common/lower/LocalDeclarationsLowering.kt | 38 ++++----- .../kotlin/backend/common/lower/LowerUtils.kt | 33 ++++++++ .../kotlin/backend/konan/KonanLower.kt | 6 +- .../backend/konan/lower/InnerClassLowering.kt | 31 +++----- backend.native/tests/build.gradle | 11 ++- .../innerClass/noPrimaryConstructor.kt | 21 +++++ .../localClass/noPrimaryConstructor.kt | 21 +++++ 8 files changed, 159 insertions(+), 80 deletions(-) create mode 100644 backend.native/tests/codegen/innerClass/noPrimaryConstructor.kt create mode 100644 backend.native/tests/codegen/localClass/noPrimaryConstructor.kt diff --git a/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/common/lower/AbstractClosureAnnotator.kt b/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/common/lower/AbstractClosureAnnotator.kt index 68a217f8cee..8b44e0388b4 100644 --- a/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/common/lower/AbstractClosureAnnotator.kt +++ b/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/common/lower/AbstractClosureAnnotator.kt @@ -10,6 +10,7 @@ import org.jetbrains.kotlin.ir.visitors.IrElementVisitorVoid import org.jetbrains.kotlin.ir.visitors.acceptChildrenVoid import org.jetbrains.kotlin.ir.visitors.acceptVoid import org.jetbrains.kotlin.resolve.DescriptorUtils +import org.jetbrains.kotlin.resolve.descriptorUtil.getSuperClassOrAny // TODO: synchronize with JVM BE class Closure(val capturedValues: List) @@ -18,8 +19,9 @@ abstract class AbstractClosureAnnotator : IrElementVisitorVoid { protected abstract fun recordFunctionClosure(functionDescriptor: FunctionDescriptor, closure: Closure) protected abstract fun recordClassClosure(classDescriptor: ClassDescriptor, closure: Closure) - private abstract class ClosureBuilder(open val owner: DeclarationDescriptor) { + private class ClosureBuilder { val capturedValues = mutableSetOf() + private val declaredValues = mutableSetOf() fun buildClosure() = Closure(capturedValues.toList()) @@ -27,34 +29,26 @@ abstract class AbstractClosureAnnotator : IrElementVisitorVoid { fillInNestedClosure(capturedValues, closure.capturedValues) } - private fun fillInNestedClosure(destination: MutableSet, nested: List) { - nested.filterTo(destination) { - isExternal(it) - } + private fun fillInNestedClosure(destination: MutableSet, nested: List) { + nested.filterTo(destination) { isExternal(it) } } - abstract fun isExternal(valueDescriptor: T): Boolean - } - - private class FunctionClosureBuilder(override val owner: FunctionDescriptor) : ClosureBuilder(owner) { - - override fun isExternal(valueDescriptor: T): Boolean = - valueDescriptor.containingDeclaration != owner && valueDescriptor != owner.dispatchReceiverParameter - } - - private class ClassClosureBuilder(override val owner: ClassDescriptor) : ClosureBuilder(owner) { - - override fun isExternal(valueDescriptor: T): Boolean { - // TODO: replace with 'return valueDescriptor.containingDeclaration != owner' after constructors lowering. - var declaration: DeclarationDescriptor? = valueDescriptor.containingDeclaration - while (declaration != null && declaration != owner) { - declaration = declaration.containingDeclaration - } - return declaration != owner + fun declareVariable(valueDescriptor: ValueDescriptor?) { + if (valueDescriptor != null) + declaredValues.add(valueDescriptor) } + fun seeVariable(valueDescriptor: ValueDescriptor) { + if (isExternal(valueDescriptor)) + capturedValues.add(valueDescriptor) + } + + fun isExternal(valueDescriptor: ValueDescriptor): Boolean { + return !declaredValues.contains(valueDescriptor) + } } + private val classClosures = mutableMapOf() private val closuresStack = mutableListOf() private fun MutableList.push(element: E) = this.add(element) @@ -69,16 +63,28 @@ abstract class AbstractClosureAnnotator : IrElementVisitorVoid { override fun visitClass(declaration: IrClass) { val classDescriptor = declaration.descriptor - val closureBuilder = ClassClosureBuilder(classDescriptor) + val closureBuilder = ClosureBuilder() + + closureBuilder.declareVariable(classDescriptor.thisAsReceiverParameter) + if (classDescriptor.isInner) + closureBuilder.declareVariable((classDescriptor.containingDeclaration as ClassDescriptor).thisAsReceiverParameter) closuresStack.push(closureBuilder) declaration.acceptChildrenVoid(this) closuresStack.pop() + val superClassClosure = classClosures[classDescriptor.getSuperClassOrAny()] + if (superClassClosure != null) { + // Capture all values from the super class since we need to call constructor of super class + // with its captured values. + closureBuilder.addNested(superClassClosure) + } + val closure = closureBuilder.buildClosure() if (DescriptorUtils.isLocal(classDescriptor)) { recordClassClosure(classDescriptor, closure) + classClosures[classDescriptor] = closure } closuresStack.peek()?.addNested(closure) @@ -86,7 +92,13 @@ abstract class AbstractClosureAnnotator : IrElementVisitorVoid { override fun visitFunction(declaration: IrFunction) { val functionDescriptor = declaration.descriptor - val closureBuilder = FunctionClosureBuilder(functionDescriptor) + val closureBuilder = ClosureBuilder() + + functionDescriptor.valueParameters.forEach { closureBuilder.declareVariable(it) } + closureBuilder.declareVariable(functionDescriptor.dispatchReceiverParameter) + closureBuilder.declareVariable(functionDescriptor.extensionReceiverParameter) + if (functionDescriptor is ConstructorDescriptor) + closureBuilder.declareVariable(functionDescriptor.constructedClass.thisAsReceiverParameter) closuresStack.push(closureBuilder) declaration.acceptChildrenVoid(this) @@ -107,16 +119,12 @@ abstract class AbstractClosureAnnotator : IrElementVisitorVoid { } override fun visitVariableAccess(expression: IrValueAccessExpression) { - val closureBuilder = closuresStack.peek() - - if (closureBuilder != null) { - val variableDescriptor = expression.descriptor - if (closureBuilder.isExternal(variableDescriptor)) { - closureBuilder.capturedValues.add(variableDescriptor) - } - } - - expression.acceptChildrenVoid(this) + closuresStack.peek()?.seeVariable(expression.descriptor) + super.visitVariableAccess(expression) } + override fun visitVariable(declaration: IrVariable) { + closuresStack.peek()?.declareVariable(declaration.descriptor) + super.visitVariable(declaration) + } } \ No newline at end of file diff --git a/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/common/lower/LocalDeclarationsLowering.kt b/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/common/lower/LocalDeclarationsLowering.kt index fd371547225..e86a373c33e 100644 --- a/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/common/lower/LocalDeclarationsLowering.kt +++ b/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/common/lower/LocalDeclarationsLowering.kt @@ -269,7 +269,8 @@ class LocalDeclarationsLowering(val context: BackendContext) : DeclarationContai capturedValueDescriptor ) ?: // Captured value is directly available for the caller. - IrGetValueImpl(oldExpression.startOffset, oldExpression.endOffset, capturedValueDescriptor) + IrGetValueImpl(oldExpression.startOffset, oldExpression.endOffset, + oldParameterToNew[capturedValueDescriptor] ?: capturedValueDescriptor) } } @@ -334,12 +335,11 @@ class LocalDeclarationsLowering(val context: BackendContext) : DeclarationContai private fun rewriteClassMembers(irClass: IrClass, localClassContext: LocalClassContext) { irClass.transformChildrenVoid(FunctionBodiesRewriter(localClassContext)) - val primaryConstructor = irClass.descriptor.unsubstitutedPrimaryConstructor ?: - TODO("local classes without primary constructor") - - val primaryConstructorContext = localClassConstructors[primaryConstructor]!! - val primaryConstructorBody = primaryConstructorContext.declaration.body as? IrBlockBody - ?: throw AssertionError("Unexpected constructor body: ${primaryConstructorContext.declaration.body}") + val classDescriptor = irClass.descriptor + val constructorsCallingSuper = classDescriptor.constructors + .map { localClassConstructors[it]!! } + .filter { it.declaration.callsSuper() } + assert(constructorsCallingSuper.any(), { "Expected at least one constructor calling super; class: $classDescriptor" }) localClassContext.capturedValueToField.forEach { capturedValue, fieldDescriptor -> val startOffset = irClass.startOffset @@ -352,11 +352,15 @@ class LocalDeclarationsLowering(val context: BackendContext) : DeclarationContai ) ) - val capturedValueExpression = primaryConstructorContext.irGet(startOffset, endOffset, capturedValue)!! - val capturedValueInitializer = IrSetFieldImpl(startOffset, endOffset, fieldDescriptor, - IrGetValueImpl(startOffset, endOffset, irClass.descriptor.thisAsReceiverParameter), - capturedValueExpression, STATEMENT_ORIGIN_INITIALIZER_OF_FIELD_FOR_CAPTURED_VALUE) - primaryConstructorBody.statements.add(0, capturedValueInitializer) + for (constructorContext in constructorsCallingSuper) { + val blockBody = constructorContext.declaration.body as? IrBlockBody + ?: throw AssertionError("Unexpected constructor body: ${constructorContext.declaration.body}") + val capturedValueExpression = constructorContext.irGet(startOffset, endOffset, capturedValue)!! + blockBody.statements.add(0, + IrSetFieldImpl(startOffset, endOffset, fieldDescriptor, + IrGetValueImpl(startOffset, endOffset, classDescriptor.thisAsReceiverParameter), + capturedValueExpression, STATEMENT_ORIGIN_INITIALIZER_OF_FIELD_FOR_CAPTURED_VALUE)) + } } } @@ -514,15 +518,7 @@ class LocalDeclarationsLowering(val context: BackendContext) : DeclarationContai // Do not substitute type parameters for now. val newTypeParameters = oldDescriptor.typeParameters - val capturedValues = mutableListOf() - var classDescriptor = oldDescriptor.containingDeclaration - while (true) { - // Capture all values from the hierarchy since we need to call constructor of super class - // with his captured values. - val context = localClasses[classDescriptor] ?: break - capturedValues.addAll(context.closure.capturedValues) - classDescriptor = classDescriptor.getSuperClassOrAny() - } + val capturedValues = localClasses[oldDescriptor.containingDeclaration]!!.closure.capturedValues val newValueParameters = createTransformedValueParameters(localFunctionContext, capturedValues) diff --git a/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/common/lower/LowerUtils.kt b/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/common/lower/LowerUtils.kt index 309de8eda79..44d756d0a15 100644 --- a/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/common/lower/LowerUtils.kt +++ b/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/common/lower/LowerUtils.kt @@ -6,11 +6,17 @@ import org.jetbrains.kotlin.incremental.components.LookupLocation import org.jetbrains.kotlin.ir.IrElement import org.jetbrains.kotlin.ir.UNDEFINED_OFFSET import org.jetbrains.kotlin.ir.builders.* +import org.jetbrains.kotlin.ir.declarations.IrClass +import org.jetbrains.kotlin.ir.declarations.IrConstructor +import org.jetbrains.kotlin.ir.expressions.IrDelegatingConstructorCall import org.jetbrains.kotlin.ir.expressions.IrExpression import org.jetbrains.kotlin.ir.expressions.IrStatementOrigin +import org.jetbrains.kotlin.ir.visitors.IrElementVisitorVoid +import org.jetbrains.kotlin.ir.visitors.acceptChildrenVoid import org.jetbrains.kotlin.name.Name import org.jetbrains.kotlin.resolve.NonReportingOverrideStrategy import org.jetbrains.kotlin.resolve.OverridingUtil +import org.jetbrains.kotlin.resolve.descriptorUtil.getSuperClassOrAny import org.jetbrains.kotlin.resolve.scopes.DescriptorKindFilter import org.jetbrains.kotlin.resolve.scopes.MemberScopeImpl import org.jetbrains.kotlin.types.KotlinType @@ -91,4 +97,31 @@ class SimpleMemberScope(val members: List) : MemberScopeI override fun printScopeStructure(p: Printer) = TODO("not implemented") +} + +fun IrConstructor.callsSuper(): Boolean { + val constructedClass = descriptor.constructedClass + val superClass = constructedClass.getSuperClassOrAny() + var callsSuper = false + var numberOfCalls = 0 + acceptChildrenVoid(object : IrElementVisitorVoid { + override fun visitElement(element: IrElement) { + element.acceptChildrenVoid(this) + } + + override fun visitClass(declaration: IrClass) { + // Skip nested + } + + override fun visitDelegatingConstructorCall(expression: IrDelegatingConstructorCall) { + assert(++numberOfCalls == 1, { "More than one delegating constructor call: $descriptor" }) + if (expression.descriptor.constructedClass == superClass) + callsSuper = true + else if (expression.descriptor.constructedClass != constructedClass) + throw AssertionError("Expected either call to another constructor of the class being constructed or" + + " call to super class constructor. But was: ${expression.descriptor.constructedClass}") + } + }) + assert(numberOfCalls == 1, { "Expected exactly one delegating constructor call but none encountered: $descriptor" }) + return callsSuper } \ No newline at end of file diff --git a/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/KonanLower.kt b/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/KonanLower.kt index 125c4ba8b48..f4fcf308179 100644 --- a/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/KonanLower.kt +++ b/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/KonanLower.kt @@ -43,15 +43,15 @@ internal class KonanLower(val context: Context) { phaser.phase(KonanPhase.LOWER_SHARED_VARIABLES) { SharedVariablesLowering(context).runOnFilePostfix(irFile) } + phaser.phase(KonanPhase.LOWER_INITIALIZERS) { + InitializersLowering(context).runOnFilePostfix(irFile) + } phaser.phase(KonanPhase.LOWER_LOCAL_FUNCTIONS) { LocalDeclarationsLowering(context).runOnFilePostfix(irFile) } phaser.phase(KonanPhase.LOWER_INNER_CLASSES) { InnerClassLowering(context).runOnFilePostfix(irFile) } - phaser.phase(KonanPhase.LOWER_INITIALIZERS) { - InitializersLowering(context).runOnFilePostfix(irFile) - } phaser.phase(KonanPhase.LOWER_CALLABLES) { CallableReferenceLowering(context).runOnFilePostfix(irFile) } diff --git a/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/lower/InnerClassLowering.kt b/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/lower/InnerClassLowering.kt index 7163609e1c4..8d7df98f408 100644 --- a/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/lower/InnerClassLowering.kt +++ b/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/lower/InnerClassLowering.kt @@ -1,8 +1,10 @@ package org.jetbrains.kotlin.backend.konan.lower import org.jetbrains.kotlin.backend.common.ClassLoweringPass +import org.jetbrains.kotlin.backend.common.lower.callsSuper import org.jetbrains.kotlin.backend.konan.Context import org.jetbrains.kotlin.descriptors.* +import org.jetbrains.kotlin.ir.IrElement import org.jetbrains.kotlin.ir.declarations.IrClass import org.jetbrains.kotlin.ir.declarations.IrConstructor import org.jetbrains.kotlin.ir.declarations.IrDeclarationOriginImpl @@ -12,7 +14,10 @@ import org.jetbrains.kotlin.ir.expressions.impl.* import org.jetbrains.kotlin.ir.util.dump import org.jetbrains.kotlin.ir.util.transformFlat import org.jetbrains.kotlin.ir.visitors.IrElementTransformerVoid +import org.jetbrains.kotlin.ir.visitors.IrElementVisitorVoid +import org.jetbrains.kotlin.ir.visitors.acceptChildrenVoid import org.jetbrains.kotlin.ir.visitors.transformChildrenVoid +import org.jetbrains.kotlin.resolve.descriptorUtil.getSuperClassOrAny import org.jetbrains.kotlin.resolve.scopes.receivers.ImplicitClassReceiver import org.jetbrains.kotlin.utils.addToStdlib.singletonList @@ -57,35 +62,21 @@ internal class InnerClassLowering(val context: Context) : ClassLoweringPass { } private fun lowerConstructor(irConstructor: IrConstructor): IrConstructor { - val descriptor = irConstructor.descriptor - val startOffset = irConstructor.startOffset - val endOffset = irConstructor.endOffset - - val dispatchReceiver = descriptor.dispatchReceiverParameter!! - - val blockBody = irConstructor.body as? IrBlockBody ?: throw AssertionError("Unexpected constructor body: ${irConstructor.body}") - - val instanceInitializerIndex = blockBody.statements.indexOfFirst { it is IrInstanceInitializerCall } - if (instanceInitializerIndex >= 0) { + if (irConstructor.callsSuper()) { // Initializing constructor: initialize 'this.this$0' with '$outer'. + val blockBody = irConstructor.body as? IrBlockBody + ?: throw AssertionError("Unexpected constructor body: ${irConstructor.body}") + val startOffset = irConstructor.startOffset + val endOffset = irConstructor.endOffset blockBody.statements.add( 0, IrSetFieldImpl( startOffset, endOffset, outerThisFieldDescriptor, IrGetValueImpl(startOffset, endOffset, classDescriptor.thisAsReceiverParameter), - IrGetValueImpl(startOffset, endOffset, dispatchReceiver) + IrGetValueImpl(startOffset, endOffset, irConstructor.descriptor.dispatchReceiverParameter!!) ) ) } - else { - // Delegating constructor: invoke old constructor with dispatch receiver '$outer'. - val delegatingConstructorCall = (blockBody.statements.find { it is IrDelegatingConstructorCall } ?: - throw AssertionError("Delegating constructor call expected: ${irConstructor.dump()}") - ) as IrDelegatingConstructorCall - delegatingConstructorCall.dispatchReceiver = IrGetValueImpl( - delegatingConstructorCall.startOffset, delegatingConstructorCall.endOffset, dispatchReceiver - ) - } return irConstructor } diff --git a/backend.native/tests/build.gradle b/backend.native/tests/build.gradle index 22333114e07..35a71dc72d0 100644 --- a/backend.native/tests/build.gradle +++ b/backend.native/tests/build.gradle @@ -128,7 +128,6 @@ task objectInitialization(type: RunKonanTest) { } task objectInitialization1(type: RunKonanTest) { - disabled = true goldValue = "init\nfield\nconstructor1\ninit\nfield\nconstructor1\nconstructor2\n" source = "codegen/object/initialization1.kt" } @@ -379,6 +378,11 @@ task innerClass_superOuter(type: RunKonanTest) { source = "codegen/innerClass/superOuter.kt" } +task innerClass_noPrimaryConstructor(type: RunKonanTest) { + goldValue = "OK\nOK\n" + source = "codegen/innerClass/noPrimaryConstructor.kt" +} + task localClass_localHierarchy(type: RunKonanTest) { goldValue = "OK\n" source = "codegen/localClass/localHierarchy.kt" @@ -409,6 +413,11 @@ task localClass_virtualCallFromConstructor(type: RunKonanTest) { source = "codegen/localClass/virtualCallFromConstructor.kt" } +task localClass_noPrimaryConstructor(type: RunKonanTest) { + goldValue = "OKOK\n" + source = "codegen/localClass/noPrimaryConstructor.kt" +} + task initializers_correctOrder1(type: RunKonanTest) { goldValue = "42\n" source = "codegen/initializers/correctOrder1.kt" diff --git a/backend.native/tests/codegen/innerClass/noPrimaryConstructor.kt b/backend.native/tests/codegen/innerClass/noPrimaryConstructor.kt new file mode 100644 index 00000000000..e0fd1064fdb --- /dev/null +++ b/backend.native/tests/codegen/innerClass/noPrimaryConstructor.kt @@ -0,0 +1,21 @@ +class Outer(val s: String) { + inner class Inner { + constructor(x: Int) { + this.x = x + } + + constructor(z: String) { + x = z.length + } + + val x: Int + + fun foo() = s + } + +} + +fun main(args: Array) { + println(Outer("OK").Inner(42).foo()) + println(Outer("OK").Inner("zzz").foo()) +} \ No newline at end of file diff --git a/backend.native/tests/codegen/localClass/noPrimaryConstructor.kt b/backend.native/tests/codegen/localClass/noPrimaryConstructor.kt new file mode 100644 index 00000000000..4d43dc95619 --- /dev/null +++ b/backend.native/tests/codegen/localClass/noPrimaryConstructor.kt @@ -0,0 +1,21 @@ +fun box(s: String): String { + class Local { + constructor(x: Int) { + this.x = x + } + + constructor(z: String) { + x = z.length + } + + val x: Int + + fun result() = s + } + + return Local(42).result() + Local("zzz").result() +} + +fun main(args : Array) { + println(box("OK")) +} \ No newline at end of file