From 027593cd78345c9ebd0f722438c08911c3e12bed Mon Sep 17 00:00:00 2001 From: OliverO2 Date: Fri, 2 Jun 2023 16:23:06 +0200 Subject: [PATCH] [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 --- .../backend/jvm/lower/JvmIrLowerUtils.kt | 70 +++++++++++++++++++ .../PropertyReferenceDelegationLowering.kt | 21 +----- .../SingletonOrConstantDelegationLowering.kt | 31 +++----- .../backend/ir/SerializableIrGenerator.kt | 8 ++- .../testData/boxIr/delegatedProperty.kt | 48 ++++++++++++- .../SerializationIrJsBoxTestGenerated.java | 6 ++ 6 files changed, 138 insertions(+), 46 deletions(-) create mode 100644 compiler/ir/backend.jvm/lower/src/org/jetbrains/kotlin/backend/jvm/lower/JvmIrLowerUtils.kt diff --git a/compiler/ir/backend.jvm/lower/src/org/jetbrains/kotlin/backend/jvm/lower/JvmIrLowerUtils.kt b/compiler/ir/backend.jvm/lower/src/org/jetbrains/kotlin/backend/jvm/lower/JvmIrLowerUtils.kt new file mode 100644 index 00000000000..9bbb44cf1a1 --- /dev/null +++ b/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) diff --git a/compiler/ir/backend.jvm/lower/src/org/jetbrains/kotlin/backend/jvm/lower/PropertyReferenceDelegationLowering.kt b/compiler/ir/backend.jvm/lower/src/org/jetbrains/kotlin/backend/jvm/lower/PropertyReferenceDelegationLowering.kt index 0b23db79bf7..2a176614cee 100644 --- a/compiler/ir/backend.jvm/lower/src/org/jetbrains/kotlin/backend/jvm/lower/PropertyReferenceDelegationLowering.kt +++ b/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): Boolean = when (this) { is IrGetValue -> { @@ -128,14 +117,8 @@ private class PropertyReferenceDelegationTransformer(val context: JvmBackendCont } private fun IrProperty.transform(): List? { - 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) diff --git a/compiler/ir/backend.jvm/lower/src/org/jetbrains/kotlin/backend/jvm/lower/SingletonOrConstantDelegationLowering.kt b/compiler/ir/backend.jvm/lower/src/org/jetbrains/kotlin/backend/jvm/lower/SingletonOrConstantDelegationLowering.kt index 8265b9bdfa8..5d9e2345d87 100644 --- a/compiler/ir/backend.jvm/lower/src/org/jetbrains/kotlin/backend/jvm/lower/SingletonOrConstantDelegationLowering.kt +++ b/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? { - 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 - } } diff --git a/plugins/kotlinx-serialization/kotlinx-serialization.backend/src/org/jetbrains/kotlinx/serialization/compiler/backend/ir/SerializableIrGenerator.kt b/plugins/kotlinx-serialization/kotlinx-serialization.backend/src/org/jetbrains/kotlinx/serialization/compiler/backend/ir/SerializableIrGenerator.kt index 0d17082b5bc..9c371854525 100644 --- a/plugins/kotlinx-serialization/kotlinx-serialization.backend/src/org/jetbrains/kotlinx/serialization/compiler/backend/ir/SerializableIrGenerator.kt +++ b/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() }) diff --git a/plugins/kotlinx-serialization/testData/boxIr/delegatedProperty.kt b/plugins/kotlinx-serialization/testData/boxIr/delegatedProperty.kt index 7161283c291..9e5c5a2527a 100644 --- a/plugins/kotlinx-serialization/testData/boxIr/delegatedProperty.kt +++ b/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 { + 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() + 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(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" -} \ No newline at end of file +} diff --git a/plugins/kotlinx-serialization/tests-gen/org/jetbrains/kotlinx/serialization/runners/SerializationIrJsBoxTestGenerated.java b/plugins/kotlinx-serialization/tests-gen/org/jetbrains/kotlinx/serialization/runners/SerializationIrJsBoxTestGenerated.java index 145a6d64053..17635da2ae8 100644 --- a/plugins/kotlinx-serialization/tests-gen/org/jetbrains/kotlinx/serialization/runners/SerializationIrJsBoxTestGenerated.java +++ b/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 {