JVM_IR: fix NPE in interface companion initializers

* When referencing the companion itself, they should use the $$INSTANCE field, not the (null until <clinit> returns) Companion field of the interface. * Interface companion init blocks should be made static.
2019-11-05 13:23:29 +01:00
parent 7c015564ce
commit 939a9ff53e
11 changed files with 83 additions and 129 deletions
@@ -17,7 +17,6 @@
 package org.jetbrains.kotlin.backend.common.lower

 import org.jetbrains.kotlin.backend.common.BackendContext
-import org.jetbrains.kotlin.backend.common.CommonBackendContext
 import org.jetbrains.kotlin.descriptors.*
 import org.jetbrains.kotlin.descriptors.impl.ValueParameterDescriptorImpl
 import org.jetbrains.kotlin.ir.IrElement
@@ -32,8 +31,6 @@ import org.jetbrains.kotlin.ir.symbols.IrSymbol
 import org.jetbrains.kotlin.ir.types.IrSimpleType
 import org.jetbrains.kotlin.ir.types.IrType
 import org.jetbrains.kotlin.ir.types.classifierOrFail
-import org.jetbrains.kotlin.ir.util.defaultType
-import org.jetbrains.kotlin.ir.visitors.IrElementTransformer
 import org.jetbrains.kotlin.ir.visitors.IrElementTransformerVoid
 import org.jetbrains.kotlin.ir.visitors.IrElementVisitorVoid
 import org.jetbrains.kotlin.ir.visitors.acceptChildrenVoid
@@ -213,30 +210,3 @@ fun ParameterDescriptor.copyAsValueParameter(newOwner: CallableDescriptor, index
    )
    else -> throw Error("Unexpected parameter descriptor: $this")
 }
-
-fun IrBody.replaceThisByStaticReference(
-    context: CommonBackendContext,
-    irClass: IrClass,
-    oldThisReceiverParameter: IrValueParameter
-): IrBody =
-    transform(ReplaceThisByStaticReference(context, irClass, oldThisReceiverParameter), null)
-
-private class ReplaceThisByStaticReference(
-    val context: CommonBackendContext,
-    val irClass: IrClass,
-    val oldThisReceiverParameter: IrValueParameter
-) : IrElementTransformer<Nothing?> {
-    override fun visitGetValue(expression: IrGetValue, data: Nothing?): IrExpression {
-        val irGetValue = expression
-        if (irGetValue.symbol == oldThisReceiverParameter.symbol) {
-            val instanceField = context.declarationFactory.getFieldForObjectInstance(irClass)
-            return IrGetFieldImpl(
-                expression.startOffset,
-                expression.endOffset,
-                instanceField.symbol,
-                irClass.defaultType
-            )
-        }
-        return super.visitGetValue(irGetValue, data)
-    }
-}
@@ -46,5 +46,6 @@ interface JvmLoweredDeclarationOrigin : IrDeclarationOrigin {
    object GENERATED_ASSERTION_ENABLED_FIELD : IrDeclarationOriginImpl("GENERATED_ASSERTION_ENABLED_FIELD", isSynthetic = true)
    object GENERATED_MAIN_FOR_PARAMETERLESS_MAIN_METHOD : IrDeclarationOriginImpl("GENERATED_MAIN_FOR_PARAMETERLESS_MAIN_METHOD", isSynthetic = true)
    object SUSPEND_FUNCTION_VIEW : IrDeclarationOriginImpl("SUSPEND_FUNCTION_VIEW")
-    object SUSPEND_IMPL_STATIC_FUNCTION :  IrDeclarationOriginImpl("SUSPEND_IMPL_STATIC_FUNCTION", isSynthetic = true)
+    object SUSPEND_IMPL_STATIC_FUNCTION : IrDeclarationOriginImpl("SUSPEND_IMPL_STATIC_FUNCTION", isSynthetic = true)
+    object INTERFACE_COMPANION_PRIVATE_INSTANCE : IrDeclarationOriginImpl("INTERFACE_COMPANION_PRIVATE_INSTANCE", isSynthetic = true)
 }
@@ -34,6 +34,7 @@ class JvmDeclarationFactory(
    private val methodSignatureMapper: MethodSignatureMapper
 ) : DeclarationFactory {
    private val singletonFieldDeclarations = HashMap<IrSymbolOwner, IrField>()
+    private val interfaceCompanionFieldDeclarations = HashMap<IrSymbolOwner, IrField>()
    private val outerThisDeclarations = HashMap<IrClass, IrField>()
    private val innerClassConstructors = HashMap<IrConstructor, IrConstructor>()

@@ -134,6 +135,23 @@ class JvmDeclarationFactory(
            }
        }

+    fun getPrivateFieldForObjectInstance(singleton: IrClass): IrField =
+        if (singleton.isCompanion && singleton.parentAsClass.isJvmInterface)
+            interfaceCompanionFieldDeclarations.getOrPut(singleton) {
+                buildField {
+                    name = Name.identifier("\$\$INSTANCE")
+                    type = singleton.defaultType
+                    origin = JvmLoweredDeclarationOrigin.INTERFACE_COMPANION_PRIVATE_INSTANCE
+                    isFinal = true
+                    isStatic = true
+                    visibility = JavaVisibilities.PACKAGE_VISIBILITY
+                }.apply {
+                    parent = singleton
+                }
+            }
+        else
+            getFieldForObjectInstance(singleton)
+
    fun getDefaultImplsFunction(interfaceFun: IrSimpleFunction): IrSimpleFunction {
        val parent = interfaceFun.parentAsClass
        assert(parent.isJvmInterface) { "Parent of ${interfaceFun.dump()} should be interface" }
@@ -18,6 +18,7 @@ import org.jetbrains.kotlin.ir.builders.Scope
 import org.jetbrains.kotlin.ir.declarations.*
 import org.jetbrains.kotlin.ir.descriptors.IrBuiltIns
 import org.jetbrains.kotlin.ir.expressions.*
+import org.jetbrains.kotlin.ir.expressions.impl.IrGetFieldImpl
 import org.jetbrains.kotlin.ir.symbols.IrClassSymbol
 import org.jetbrains.kotlin.ir.symbols.IrSymbol
 import org.jetbrains.kotlin.ir.symbols.IrTypeParameterSymbol
@@ -25,6 +26,7 @@ import org.jetbrains.kotlin.ir.types.*
 import org.jetbrains.kotlin.ir.types.impl.IrSimpleTypeImpl
 import org.jetbrains.kotlin.ir.types.impl.IrStarProjectionImpl
 import org.jetbrains.kotlin.ir.util.*
+import org.jetbrains.kotlin.ir.visitors.IrElementTransformerVoid
 import org.jetbrains.kotlin.resolve.DescriptorUtils
 import org.jetbrains.kotlin.resolve.jvm.annotations.JVM_DEFAULT_FQ_NAME

@@ -148,3 +150,23 @@ fun IrExpression.isSmartcastFromHigherThanNullable(context: JvmBackendContext) =
    this is IrTypeOperatorCall &&
            operator == IrTypeOperator.IMPLICIT_CAST &&
            !this.argument.type.isSubtypeOf(type.makeNullable(), context.irBuiltIns)
+
+fun IrBody.replaceThisByStaticReference(
+    context: JvmBackendContext,
+    irClass: IrClass,
+    oldThisReceiverParameter: IrValueParameter
+): IrBody =
+    transform(object : IrElementTransformerVoid() {
+        override fun visitGetValue(expression: IrGetValue): IrExpression {
+            if (expression.symbol == oldThisReceiverParameter.symbol) {
+                val instanceField = context.declarationFactory.getPrivateFieldForObjectInstance(irClass)
+                return IrGetFieldImpl(
+                    expression.startOffset,
+                    expression.endOffset,
+                    instanceField.symbol,
+                    irClass.defaultType
+                )
+            }
+            return super.visitGetValue(expression)
+        }
+    }, null)
@@ -10,12 +10,12 @@ import org.jetbrains.kotlin.backend.common.FileLoweringPass
 import org.jetbrains.kotlin.backend.common.ir.*
 import org.jetbrains.kotlin.backend.common.lower.createIrBuilder
 import org.jetbrains.kotlin.backend.common.lower.irBlock
-import org.jetbrains.kotlin.backend.common.lower.replaceThisByStaticReference
 import org.jetbrains.kotlin.backend.common.phaser.makeIrFilePhase
 import org.jetbrains.kotlin.backend.common.runOnFilePostfix
 import org.jetbrains.kotlin.backend.jvm.JvmBackendContext
 import org.jetbrains.kotlin.backend.jvm.JvmLoweredDeclarationOrigin
 import org.jetbrains.kotlin.backend.jvm.ir.isInCurrentModule
+import org.jetbrains.kotlin.backend.jvm.ir.replaceThisByStaticReference
 import org.jetbrains.kotlin.descriptors.Modality
 import org.jetbrains.kotlin.descriptors.Visibilities
 import org.jetbrains.kotlin.descriptors.Visibility
@@ -6,10 +6,10 @@
 package org.jetbrains.kotlin.backend.jvm.lower

 import org.jetbrains.kotlin.backend.common.ClassLoweringPass
-import org.jetbrains.kotlin.backend.common.CommonBackendContext
-import org.jetbrains.kotlin.backend.common.lower.replaceThisByStaticReference
 import org.jetbrains.kotlin.backend.common.phaser.makeIrFilePhase
+import org.jetbrains.kotlin.backend.jvm.JvmBackendContext
 import org.jetbrains.kotlin.backend.jvm.codegen.isJvmInterface
+import org.jetbrains.kotlin.backend.jvm.ir.replaceThisByStaticReference
 import org.jetbrains.kotlin.descriptors.Visibilities
 import org.jetbrains.kotlin.ir.IrElement
 import org.jetbrains.kotlin.ir.IrStatement
@@ -30,6 +30,7 @@ import org.jetbrains.kotlin.ir.symbols.impl.IrVariableSymbolImpl
 import org.jetbrains.kotlin.ir.util.deepCopyWithSymbols
 import org.jetbrains.kotlin.ir.util.hasAnnotation
 import org.jetbrains.kotlin.ir.util.isObject
+import org.jetbrains.kotlin.ir.util.patchDeclarationParents
 import org.jetbrains.kotlin.ir.visitors.IrElementTransformerVoid
 import org.jetbrains.kotlin.ir.visitors.transformChildrenVoid
 import org.jetbrains.kotlin.load.java.JvmAbi.JVM_FIELD_ANNOTATION_FQ_NAME
@@ -40,7 +41,7 @@ internal val moveOrCopyCompanionObjectFieldsPhase = makeIrFilePhase(
    description = "Move and/or copy companion object fields to static fields of companion's owner"
 )

-private class MoveOrCopyCompanionObjectFieldsLowering(val context: CommonBackendContext) : ClassLoweringPass {
+private class MoveOrCopyCompanionObjectFieldsLowering(val context: JvmBackendContext) : ClassLoweringPass {
    override fun lower(irClass: IrClass) {
        val fieldReplacementMap = mutableMapOf<IrFieldSymbol, IrFieldSymbol>()
        if (irClass.isObject && !irClass.isCompanion && irClass.visibility != Visibilities.LOCAL) {
@@ -76,7 +77,7 @@ private class MoveOrCopyCompanionObjectFieldsLowering(val context: CommonBackend
        // It is an error to annotate only some of the fields of an interface companion with @JvmField.
        val newParent = if (irClass.isJvmInterface && !companion.allFieldsAreJvmField()) companion else irClass

-        val newDeclarations = companion.declarations.mapNotNull {
+        val newDeclarations = companion.declarations.map {
            when (it) {
                is IrProperty ->
                    movePropertyFieldToStaticParent(it, companion, newParent, fieldReplacementMap)
@@ -87,10 +88,16 @@ private class MoveOrCopyCompanionObjectFieldsLowering(val context: CommonBackend
            }
        }

-        // Move declarations to parent if required
-        if (newParent !== companion) {
+        if (newParent === companion) {
+            // Keep fields as children of `IrProperty`, but replace anonymous initializers with static ones,
+            // preserving the relative ordering of anonymous initializers and property initializers.
+            for ((i, declaration) in newDeclarations.withIndex())
+                if (declaration is IrAnonymousInitializer)
+                    companion.declarations[i] = declaration
+        } else {
+            // Move all touched declarations to the parent.
            companion.declarations.removeAll { it is IrAnonymousInitializer }
-            newParent.declarations += newDeclarations
+            newDeclarations.filterNotNullTo(newParent.declarations)
        }
    }

@@ -153,57 +160,12 @@ private class MoveOrCopyCompanionObjectFieldsLowering(val context: CommonBackend
                isStatic = true
            ).apply {
                parent = newParent
-                body = oldInitializer.body.transferToNewParent(oldParent, newParent)
+                body = oldInitializer.body
+                    .replaceThisByStaticReference(context, oldParent, oldParent.thisReceiver!!)
+                    .patchDeclarationParents(newParent) as IrBlockBody
            }
        }

-    private fun IrBlockBody.transferToNewParent(oldParent: IrClass, newParent: IrClass): IrBlockBody {
-        val objectInstanceField = context.declarationFactory.getFieldForObjectInstance(oldParent)
-        return transform(
-            data = null,
-            transformer = object : IrElementTransformerVoid() {
-                val variableMap = mutableMapOf<IrVariable, IrVariable>()
-
-                override fun visitVariable(declaration: IrVariable): IrStatement {
-                    if (declaration.parent == oldParent) {
-                        val newDescriptor = WrappedVariableDescriptor(declaration.descriptor.annotations, declaration.descriptor.source)
-                        val newVariable = IrVariableImpl(
-                            declaration.startOffset, declaration.endOffset,
-                            declaration.origin, IrVariableSymbolImpl(newDescriptor),
-                            declaration.name, declaration.type, declaration.isVar, declaration.isConst, declaration.isLateinit
-                        ).apply {
-                            newDescriptor.bind(this)
-                            parent = newParent
-                            initializer = declaration.initializer
-                            annotations.addAll(declaration.annotations)
-                        }
-                        variableMap[declaration] = newVariable
-                        return super.visitVariable(newVariable)
-                    }
-                    return super.visitVariable(declaration)
-                }
-
-                override fun visitGetValue(expression: IrGetValue): IrExpression {
-                    if (expression.symbol.owner == oldParent.thisReceiver) {
-                        return IrGetFieldImpl(
-                            expression.startOffset, expression.endOffset,
-                            objectInstanceField.symbol,
-                            expression.type
-                        )
-                    }
-                    variableMap[expression.symbol.owner]?.let { newVariable ->
-                        return IrGetValueImpl(
-                            expression.startOffset, expression.endOffset,
-                            expression.type,
-                            newVariable.symbol,
-                            expression.origin
-                        )
-                    }
-                    return super.visitGetValue(expression)
-                }
-            }) as IrBlockBody
-    }
-
    private fun createStaticBackingField(oldField: IrField, propertyParent: IrClass, fieldParent: IrClass): IrField {
        val descriptor = WrappedFieldDescriptor(oldField.descriptor.annotations, oldField.descriptor.source)
        val field = IrFieldImpl(
@@ -10,17 +10,11 @@ import org.jetbrains.kotlin.backend.common.IrElementTransformerVoidWithContext
 import org.jetbrains.kotlin.backend.common.phaser.makeIrFilePhase
 import org.jetbrains.kotlin.backend.common.lower.createIrBuilder
 import org.jetbrains.kotlin.backend.jvm.JvmBackendContext
-import org.jetbrains.kotlin.backend.jvm.codegen.isJvmInterface
-import org.jetbrains.kotlin.descriptors.*
 import org.jetbrains.kotlin.ir.IrStatement
-import org.jetbrains.kotlin.ir.UNDEFINED_OFFSET
 import org.jetbrains.kotlin.ir.builders.irCall
 import org.jetbrains.kotlin.ir.builders.irExprBody
 import org.jetbrains.kotlin.ir.builders.irGetField
 import org.jetbrains.kotlin.ir.declarations.*
-import org.jetbrains.kotlin.ir.declarations.impl.IrFieldImpl
-import org.jetbrains.kotlin.ir.descriptors.WrappedFieldDescriptor
-import org.jetbrains.kotlin.ir.symbols.impl.IrFieldSymbolImpl
 import org.jetbrains.kotlin.ir.util.*

 internal val objectClassPhase = makeIrFilePhase(
@@ -49,39 +43,20 @@ private class ObjectClassLowering(val context: JvmBackendContext) : IrElementTra
        if (!irClass.isObject) return

        val publicInstanceField = context.declarationFactory.getFieldForObjectInstance(irClass)
+        val privateInstanceField = context.declarationFactory.getPrivateFieldForObjectInstance(irClass)

        val constructor = irClass.constructors.find { it.isPrimary }
            ?: throw AssertionError("Object should have a primary constructor: ${irClass.name}")

-        val publicInstanceOwner = if (irClass.isCompanion) parentScope!!.irElement as IrDeclarationContainer else irClass
-        if (irClass.isCompanion && irClass.parentAsClass.isJvmInterface) {
-            // TODO rename to $$INSTANCE
-            // TODO: mark as synthesized
-            val privateFieldDescriptor = WrappedFieldDescriptor()
-            val privateField = IrFieldImpl(
-                UNDEFINED_OFFSET, UNDEFINED_OFFSET,
-                IrDeclarationOrigin.FIELD_FOR_OBJECT_INSTANCE,
-                IrFieldSymbolImpl(privateFieldDescriptor),
-                publicInstanceField.name,
-                irClass.defaultType,
-                Visibilities.PROTECTED/* TODO package local */,
-                isFinal = true,
-                isExternal = false,
-                isStatic = true,
-                isFakeOverride = false
-            ).apply {
-                privateFieldDescriptor.bind(this)
-                parent = irClass
-                with(context.createIrBuilder(symbol)) {
-                    initializer = irExprBody(
-                        irCall(constructor)
-                    )
-                }
-                pendingTransformations.add { parentAsClass.declarations.add(this) }
+        if (privateInstanceField != publicInstanceField) {
+            with(context.createIrBuilder(privateInstanceField.symbol)) {
+                privateInstanceField.initializer = irExprBody(irCall(constructor.symbol))
            }
-
            with(context.createIrBuilder(publicInstanceField.symbol)) {
-                publicInstanceField.initializer = irExprBody(irGetField(null, privateField))
+                publicInstanceField.initializer = irExprBody(irGetField(null, privateInstanceField))
+            }
+            pendingTransformations.add {
+                (privateInstanceField.parent as IrDeclarationContainer).declarations.add(0, privateInstanceField)
            }
        } else {
            with(context.createIrBuilder(publicInstanceField.symbol)) {
@@ -89,9 +64,8 @@ private class ObjectClassLowering(val context: JvmBackendContext) : IrElementTra
            }
        }

-        publicInstanceField.parent = publicInstanceOwner
        pendingTransformations.add {
-            publicInstanceOwner.declarations.add(publicInstanceField)
+            (publicInstanceField.parent as IrDeclarationContainer).declarations.add(0, publicInstanceField)
        }
    }
 }
@@ -52,18 +52,28 @@ private class SingletonReferencesLowering(val context: JvmBackendContext) : File
    }

    override fun visitGetObjectValue(expression: IrGetObjectValue): IrExpression {
-        val instanceField = context.declarationFactory.getFieldForObjectInstance(expression.symbol.owner)
+        // When in an instance method of the object, use the dispatch receiver. Do not use the dispatch
+        // receiver in the constructor, as in the case of an `object` the only way it can be used is
+        // in a super constructor call argument (=> it has JVM type "uninitialized this" and is not usable)
+        // while for a `companion object` all initializers are in `<clinit>` and have no receiver at all.
+        thisOfClass(expression.symbol.owner, false)?.let {
+            return IrGetValueImpl(expression.startOffset, expression.endOffset, it.symbol)
+        }
+        val instanceField = if (allScopes.any { it.irElement == expression.symbol.owner })
+            context.declarationFactory.getPrivateFieldForObjectInstance(expression.symbol.owner) // Constructor or static method.
+        else
+            context.declarationFactory.getFieldForObjectInstance(expression.symbol.owner) // Not in object scope at all.
        return IrGetFieldImpl(expression.startOffset, expression.endOffset, instanceField.symbol, expression.type)
    }

-    // `this` is generally available while the reference is within the lexical scope of the containing enum entry.
-    private fun thisOfClass(declaration: IrClass?): IrValueParameter? {
+    // `this` is generally available while the reference is within the lexical scope of the containing class.
+    private fun thisOfClass(declaration: IrClass?, allowConstructorReceiver: Boolean = true): IrValueParameter? {
        if (declaration == null) return null
        for (scope in allScopes.reversed()) {
            when (val element = scope.irElement) {
                is IrFunction ->
                    element.dispatchReceiverParameter?.let { if (it.type.classOrNull == declaration.symbol) return it }
-                is IrClass -> if (element == declaration) return element.thisReceiver
+                is IrClass -> if (allowConstructorReceiver && element == declaration) return element.thisReceiver
            }
        }
        return null
@@ -1,5 +1,4 @@
 // IGNORE_BACKEND_FIR: JVM_IR
-// IGNORE_BACKEND: JVM_IR
 // Inside of the companion we have to access the instance through the local Companion field,
 // not by indirection through the Companion field of the enclosing class.
 // Class initialization might not have finished yet.
@@ -1,6 +1,5 @@
 // !LANGUAGE: +NestedClassesInAnnotations
 // IGNORE_BACKEND_FIR: JVM_IR
-// IGNORE_BACKEND: JVM_IR
 // WITH_RUNTIME

 import kotlin.test.*
@@ -1,5 +1,4 @@
 // IGNORE_BACKEND_FIR: JVM_IR
-// IGNORE_BACKEND: JVM_IR
 interface Test {
    companion object {
        fun ok() = "OK"