[KxSerialization] Fix NPE when accessing delegated property

Kotlin 1.7.20 added optimizations for delegated properties on the JVM,
which broke serialization for optimized properties. Commit bfeff81
tried to fix that, but broke non-optimized delegated properties. This
commit restores correct serialization for optimized and non-optimized
properties, also ensuring that it only affects the JVM target.

 #KT-58954 Fixed
 #KT-59113 Fixed

compiler/ir/backend.jvm/lower/src/org/jetbrains/kotlin/backend/jvm/lower/JvmIrLowerUtils.kt

@@ -0,0 +1,70 @@
+/*
+ * Copyright 2010-2023 JetBrains s.r.o. and Kotlin Programming Language contributors.
+ * Use of this source code is governed by the Apache 2.0 license that can be found in the license/LICENSE.txt file.
+ */
+
+package org.jetbrains.kotlin.backend.jvm.lower
+
+import org.jetbrains.kotlin.builtins.StandardNames
+import org.jetbrains.kotlin.descriptors.Modality
+import org.jetbrains.kotlin.ir.IrStatement
+import org.jetbrains.kotlin.ir.declarations.IrDeclarationOrigin
+import org.jetbrains.kotlin.ir.declarations.IrProperty
+import org.jetbrains.kotlin.ir.declarations.IrSimpleFunction
+import org.jetbrains.kotlin.ir.expressions.*
+import org.jetbrains.kotlin.ir.util.getPackageFragment
+import org.jetbrains.kotlin.ir.util.statements
+
+internal val IrSimpleFunction.returnsResultOfStdlibCall: Boolean
+    get() {
+        fun IrStatement.isStdlibCall() =
+            this is IrCall && symbol.owner.getPackageFragment().packageFqName == StandardNames.BUILT_INS_PACKAGE_FQ_NAME
+
+        return when (val body = body) {
+            is IrExpressionBody -> body.expression.isStdlibCall()
+            is IrBlockBody -> body.statements.singleOrNull()
+                ?.let { it.isStdlibCall() || (it is IrReturn && it.value.isStdlibCall()) } == true
+            else -> false
+        }
+    }
+
+// Criteria for delegate optimizations on the JVM.
+// All cases must be reflected in isJvmOptimizableDelegate() to inform the kotlinx-serialization plugin.
+internal fun IrProperty.getPropertyReferenceForOptimizableDelegatedProperty(): IrPropertyReference? {
+    if (!isDelegated || isFakeOverride || backingField == null) return null
+
+    val delegate = backingField?.initializer?.expression
+    if (delegate !is IrPropertyReference ||
+        getter?.returnsResultOfStdlibCall == false ||
+        setter?.returnsResultOfStdlibCall == false
+    ) return null
+
+    return delegate
+}
+
+internal fun IrProperty.getSingletonOrConstantForOptimizableDelegatedProperty(): IrExpression? {
+    fun IrExpression.isInlineable(): Boolean =
+        when (this) {
+            is IrConst<*>, is IrGetSingletonValue -> true
+            is IrCall ->
+                dispatchReceiver?.isInlineable() != false
+                        && extensionReceiver?.isInlineable() != false
+                        && valueArgumentsCount == 0
+                        && symbol.owner.run {
+                    modality == Modality.FINAL
+                            && origin == IrDeclarationOrigin.DEFAULT_PROPERTY_ACCESSOR
+                            && ((body?.statements?.singleOrNull() as? IrReturn)?.value as? IrGetField)?.symbol?.owner?.isFinal == true
+                }
+            is IrGetValue ->
+                symbol.owner.origin == IrDeclarationOrigin.INSTANCE_RECEIVER
+            else -> false
+        }
+
+    if (!isDelegated || isFakeOverride || backingField == null) return null
+    return backingField?.initializer?.expression?.takeIf { it.isInlineable() }
+}
+
+/** Returns true if a delegate is optimizable on the JVM, omitting a `$delegate` auxiliary property */
+fun IrProperty.isJvmOptimizableDelegate(): Boolean =
+    isDelegated && !isFakeOverride && backingField != null && // fast path
+            (getPropertyReferenceForOptimizableDelegatedProperty() != null || getSingletonOrConstantForOptimizableDelegatedProperty() != null)

compiler/ir/backend.jvm/lower/src/org/jetbrains/kotlin/backend/jvm/lower/PropertyReferenceDelegationLowering.kt

@@ -13,7 +13,6 @@ import org.jetbrains.kotlin.backend.jvm.JvmBackendContext
 import org.jetbrains.kotlin.backend.jvm.ir.createJvmIrBuilder
 import org.jetbrains.kotlin.backend.jvm.ir.fileParentOrNull
 import org.jetbrains.kotlin.backend.jvm.lower.JvmPropertiesLowering.Companion.createSyntheticMethodForPropertyDelegate
-import org.jetbrains.kotlin.builtins.StandardNames
 import org.jetbrains.kotlin.descriptors.Modality
 import org.jetbrains.kotlin.ir.IrStatement
 import org.jetbrains.kotlin.ir.builders.*
@@ -78,16 +77,6 @@ private class PropertyReferenceDelegationTransformer(val context: JvmBackendCont
             irExprBody(access)
         }
 
-    private val IrSimpleFunction.returnsResultOfStdlibCall: Boolean
-        get() = when (val body = body) {
-            is IrExpressionBody -> body.expression.isStdlibCall
-            is IrBlockBody -> body.statements.singleOrNull()?.let { it.isStdlibCall || (it is IrReturn && it.value.isStdlibCall) } == true
-            else -> false
-        }
-
-    private val IrStatement.isStdlibCall: Boolean
-        get() = this is IrCall && symbol.owner.getPackageFragment().packageFqName == StandardNames.BUILT_INS_PACKAGE_FQ_NAME
-
     // Some receivers don't need to be stored in fields and can be reevaluated every time an accessor is called:
     private fun IrExpression.canInline(visibleScopes: Set<IrDeclarationParent>): Boolean = when (this) {
         is IrGetValue -> {
@@ -128,14 +117,8 @@ private class PropertyReferenceDelegationTransformer(val context: JvmBackendCont
     }
 
     private fun IrProperty.transform(): List<IrDeclaration>? {
-        if (!isDelegated || isFakeOverride) return null
-
-        val oldField = backingField
-        val delegate = oldField?.initializer?.expression
-        if (delegate !is IrPropertyReference ||
-            getter?.returnsResultOfStdlibCall == false ||
-            setter?.returnsResultOfStdlibCall == false
-        ) return null
+        val delegate = getPropertyReferenceForOptimizableDelegatedProperty() ?: return null
+        val oldField = backingField ?: return null
 
         val receiver = (delegate.dispatchReceiver ?: delegate.extensionReceiver)
             ?.transform(this@PropertyReferenceDelegationTransformer, null)

compiler/ir/backend.jvm/lower/src/org/jetbrains/kotlin/backend/jvm/lower/SingletonOrConstantDelegationLowering.kt

@@ -10,12 +10,17 @@ import org.jetbrains.kotlin.backend.common.phaser.makeIrFilePhase
 import org.jetbrains.kotlin.backend.jvm.JvmBackendContext
 import org.jetbrains.kotlin.backend.jvm.ir.createJvmIrBuilder
 import org.jetbrains.kotlin.backend.jvm.lower.JvmPropertiesLowering.Companion.createSyntheticMethodForPropertyDelegate
-import org.jetbrains.kotlin.descriptors.Modality
 import org.jetbrains.kotlin.ir.builders.irExprBody
 import org.jetbrains.kotlin.ir.declarations.*
-import org.jetbrains.kotlin.ir.expressions.*
+import org.jetbrains.kotlin.ir.expressions.IrConst
+import org.jetbrains.kotlin.ir.expressions.IrExpression
+import org.jetbrains.kotlin.ir.expressions.IrGetField
+import org.jetbrains.kotlin.ir.expressions.IrGetValue
 import org.jetbrains.kotlin.ir.symbols.impl.IrAnonymousInitializerSymbolImpl
-import org.jetbrains.kotlin.ir.util.*
+import org.jetbrains.kotlin.ir.util.isFileClass
+import org.jetbrains.kotlin.ir.util.parentAsClass
+import org.jetbrains.kotlin.ir.util.remapReceiver
+import org.jetbrains.kotlin.ir.util.transformDeclarationsFlat
 import org.jetbrains.kotlin.ir.visitors.IrElementTransformerVoid
 
 internal val singletonOrConstantDelegationPhase = makeIrFilePhase(
@@ -41,8 +46,7 @@ private class SingletonOrConstantDelegationTransformer(val context: JvmBackendCo
     }
 
     private fun IrProperty.transform(): List<IrDeclaration>? {
-        if (!isDelegated || isFakeOverride || backingField == null) return null
-        val delegate = backingField?.initializer?.expression?.takeIf { it.isInlineable() } ?: return null
+        val delegate = getSingletonOrConstantForOptimizableDelegatedProperty() ?: return null
         val originalThis = parentAsClass.thisReceiver
 
         class DelegateFieldAccessTransformer(val newReceiver: IrExpression) : IrElementTransformerVoid() {
@@ -73,21 +77,4 @@ private class SingletonOrConstantDelegationTransformer(val context: JvmBackendCo
 
         return listOfNotNull(this, initializerBlock, delegateMethod)
     }
-
-    private fun IrExpression.isInlineable(): Boolean =
-        when (this) {
-            is IrConst<*>, is IrGetSingletonValue -> true
-            is IrCall ->
-                dispatchReceiver?.isInlineable() != false
-                        && extensionReceiver?.isInlineable() != false
-                        && valueArgumentsCount == 0
-                        && symbol.owner.run {
-                    modality == Modality.FINAL
-                            && origin == IrDeclarationOrigin.DEFAULT_PROPERTY_ACCESSOR
-                            && ((body?.statements?.singleOrNull() as? IrReturn)?.value as? IrGetField)?.symbol?.owner?.isFinal == true
-                }
-            is IrGetValue ->
-                symbol.owner.origin == IrDeclarationOrigin.INSTANCE_RECEIVER
-            else -> false
-        }
 }

plugins/kotlinx-serialization/kotlinx-serialization.backend/src/org/jetbrains/kotlinx/serialization/compiler/backend/ir/SerializableIrGenerator.kt

@@ -6,6 +6,7 @@
 package org.jetbrains.kotlinx.serialization.compiler.backend.ir
 
 import org.jetbrains.kotlin.backend.common.lower.irThrow
+import org.jetbrains.kotlin.backend.jvm.lower.isJvmOptimizableDelegate
 import org.jetbrains.kotlin.descriptors.ClassKind
 import org.jetbrains.kotlin.descriptors.DescriptorVisibilities
 import org.jetbrains.kotlin.descriptors.ValueParameterDescriptor
@@ -21,6 +22,7 @@ import org.jetbrains.kotlin.ir.types.*
 import org.jetbrains.kotlin.ir.util.*
 import org.jetbrains.kotlin.name.ClassId
 import org.jetbrains.kotlin.name.Name
+import org.jetbrains.kotlin.platform.jvm.isJvm
 import org.jetbrains.kotlin.util.OperatorNameConventions
 import org.jetbrains.kotlin.utils.getOrPutNullable
 import org.jetbrains.kotlinx.serialization.compiler.extensions.SerializationPluginContext
@@ -82,8 +84,10 @@ class SerializableIrGenerator(
                     it is IrProperty && it.backingField != null -> {
                         if (it in serialDescs) {
                             current = it
-                        } else if (it.backingField?.initializer != null && !it.isDelegated) {
-                            // skip transient lateinit or deferred properties (with null initializer)
+                        } else if (it.backingField?.initializer != null &&
+                            !(it.isJvmOptimizableDelegate() && compilerContext.platform.isJvm())
+                        ) {
+                            // skip transient lateinit or deferred properties (with null initializer) and optimized delegations
                             val expression = initializerAdapter(it.backingField!!.initializer!!)
 
                             statementsAfterSerializableProperty.getOrPutNullable(current, { mutableListOf() })

plugins/kotlinx-serialization/testData/boxIr/delegatedProperty.kt

@@ -1,5 +1,3 @@
-// TARGET_BACKEND: JVM_IR
-
 // WITH_STDLIB
 
 import kotlinx.serialization.*
@@ -69,6 +67,34 @@ class DelegatedByThis(val realProp: Int) {
     val delegatedProp by this
 }
 
+// delegating directly to another property
+@Serializable
+class DelegatedByDirectProperty(var targetProperty: Int = 5) {
+    var delegatingProperty: Int by ::targetProperty
+}
+
+// generic delegate
+@Serializable
+open class GenericDelegate<Target> {
+    private var target: Target? = null
+
+    operator fun getValue(thisRef: Any?, property: KProperty<*>): Target? = target
+
+    operator fun setValue(thisRef: Any?, property: KProperty<*>, value: Target?) {
+        target = value
+    }
+}
+
+@Serializable
+class GenericDelegateHolder(val id: String) {
+    private var _delegatingProperty = GenericDelegate<String>()
+    var delegatingProperty: String? by _delegatingProperty
+
+    constructor(id: String, delegatingProperty: String?) : this(id) {
+        this.delegatingProperty = delegatingProperty
+    }
+}
+
 fun box(): String {
     val simpleDTO = SimpleDTO(123)
     val simpleDTOJsonStr = Json.encodeToString(simpleDTO)
@@ -105,5 +131,21 @@ fun box(): String {
     if (byThisDecoded.delegatedProp != byThisExp.delegatedProp) return "DelegatedByThis Delegate is incorrect!"
     if (byThisDecoded.realProp !== 123) return "DelegatedByThis Deserialization failed"
 
+    for (original in listOf(
+        GenericDelegateHolder("#1", "stuff"),
+        GenericDelegateHolder("#2", null)
+    )) {
+        val json = Json.encodeToString(original)
+        val deserialized = Json.decodeFromString(GenericDelegateHolder.serializer(), json)
+        if (deserialized.delegatingProperty != original.delegatingProperty) return "Generic delegate fail: $json"
+    }
+
+    val byDirectPropertyExp = DelegatedByDirectProperty(123)
+    val byDirectPropertyJsonStr = Json.encodeToString(byDirectPropertyExp)
+    val byDirectPropertyDecoded = Json.decodeFromString<DelegatedByDirectProperty>(byDirectPropertyJsonStr)
+    if (byDirectPropertyJsonStr != """{"targetProperty":123}""") return simpleDTOJsonStr
+    if (byDirectPropertyDecoded.delegatingProperty != byDirectPropertyExp.delegatingProperty) return "Direct property delegation, delegatingProperty fail: $byDirectPropertyJsonStr"
+    if (byDirectPropertyDecoded.targetProperty !== 123) return "Direct property delegation, targetProperty fail: $byDirectPropertyJsonStr"
+
     return "OK"
-}
+}

plugins/kotlinx-serialization/tests-gen/org/jetbrains/kotlinx/serialization/runners/SerializationIrJsBoxTestGenerated.java

@@ -31,6 +31,12 @@ public class SerializationIrJsBoxTestGenerated extends AbstractSerializationIrJs
         runTest("plugins/kotlinx-serialization/testData/boxIr/constValInSerialName.kt");
     }
 
+    @Test
+    @TestMetadata("delegatedProperty.kt")
+    public void testDelegatedProperty() throws Exception {
+        runTest("plugins/kotlinx-serialization/testData/boxIr/delegatedProperty.kt");
+    }
+
     @Test
     @TestMetadata("excludedFromExport.kt")
     public void testExcludedFromExport() throws Exception {