From 4e516d7c94cd72e3cbee4033c22207c4c98d8948 Mon Sep 17 00:00:00 2001 From: Leonid Startsev Date: Fri, 30 Aug 2019 13:26:38 +0300 Subject: [PATCH] Fix IR validation warnings in serialization plugin - Insert patchDeclarationParents after whole processing - Copy IR nodes to avoid duplications - Insert correct type for getter during serialization of properties from parent class - Do not use irThrowIse because it is broken somewhere in Native backend --- .../compiler/backend/ir/GeneratorHelpers.kt | 15 +++--- .../ir/SerializableCompanionIrGenerator.kt | 5 +- .../backend/ir/SerializableIrGenerator.kt | 30 +++++------ .../backend/ir/SerializerIrGenerator.kt | 28 +++++----- .../testData/codegen/Basic.ir.txt | 52 +++++++++---------- 5 files changed, 64 insertions(+), 66 deletions(-) diff --git a/plugins/kotlin-serialization/kotlin-serialization-compiler/src/org/jetbrains/kotlinx/serialization/compiler/backend/ir/GeneratorHelpers.kt b/plugins/kotlin-serialization/kotlin-serialization-compiler/src/org/jetbrains/kotlinx/serialization/compiler/backend/ir/GeneratorHelpers.kt index 88c9e3cb479..02170aa9acb 100644 --- a/plugins/kotlin-serialization/kotlin-serialization-compiler/src/org/jetbrains/kotlinx/serialization/compiler/backend/ir/GeneratorHelpers.kt +++ b/plugins/kotlin-serialization/kotlin-serialization-compiler/src/org/jetbrains/kotlinx/serialization/compiler/backend/ir/GeneratorHelpers.kt @@ -6,6 +6,7 @@ package org.jetbrains.kotlinx.serialization.compiler.backend.ir import org.jetbrains.kotlin.backend.common.BackendContext +import org.jetbrains.kotlin.backend.common.deepCopyWithVariables import org.jetbrains.kotlin.backend.common.lower.at import org.jetbrains.kotlin.backend.common.lower.createIrBuilder import org.jetbrains.kotlin.descriptors.* @@ -423,12 +424,14 @@ interface IrBuilderExtension { original.valueParameters.associate { it.descriptor to it.defaultValue?.expression } return fun(f: IrField): IrExpression? { val i = f.initializer?.expression ?: return null - return if (i is IrGetValueImpl && i.origin == IrStatementOrigin.INITIALIZE_PROPERTY_FROM_PARAMETER) { - // this is a primary constructor property, use corresponding default of value parameter - defaultsMap.getValue(i.descriptor as ParameterDescriptor) - } else { - i - } + val irExpression = + if (i is IrGetValueImpl && i.origin == IrStatementOrigin.INITIALIZE_PROPERTY_FROM_PARAMETER) { + // this is a primary constructor property, use corresponding default of value parameter + defaultsMap.getValue(i.descriptor as ParameterDescriptor) + } else { + i + } + return irExpression?.deepCopyWithVariables() } } diff --git a/plugins/kotlin-serialization/kotlin-serialization-compiler/src/org/jetbrains/kotlinx/serialization/compiler/backend/ir/SerializableCompanionIrGenerator.kt b/plugins/kotlin-serialization/kotlin-serialization-compiler/src/org/jetbrains/kotlinx/serialization/compiler/backend/ir/SerializableCompanionIrGenerator.kt index 7835fb37df2..b609d358367 100644 --- a/plugins/kotlin-serialization/kotlin-serialization-compiler/src/org/jetbrains/kotlinx/serialization/compiler/backend/ir/SerializableCompanionIrGenerator.kt +++ b/plugins/kotlin-serialization/kotlin-serialization-compiler/src/org/jetbrains/kotlinx/serialization/compiler/backend/ir/SerializableCompanionIrGenerator.kt @@ -15,6 +15,7 @@ import org.jetbrains.kotlin.ir.types.defaultType import org.jetbrains.kotlin.ir.util.SymbolTable import org.jetbrains.kotlin.ir.util.TypeTranslator import org.jetbrains.kotlin.ir.util.defaultType +import org.jetbrains.kotlin.ir.util.patchDeclarationParents import org.jetbrains.kotlin.name.ClassId import org.jetbrains.kotlin.name.Name import org.jetbrains.kotlin.resolve.BindingContext @@ -40,8 +41,10 @@ class SerializableCompanionIrGenerator( ) { val companionDescriptor = irClass.descriptor val serializableClass = getSerializableClassDescriptorByCompanion(companionDescriptor) ?: return - if (serializableClass.shouldHaveGeneratedMethodsInCompanion) + if (serializableClass.shouldHaveGeneratedMethodsInCompanion) { SerializableCompanionIrGenerator(irClass, context, bindingContext).generate() + irClass.patchDeclarationParents(irClass.parent) + } } } diff --git a/plugins/kotlin-serialization/kotlin-serialization-compiler/src/org/jetbrains/kotlinx/serialization/compiler/backend/ir/SerializableIrGenerator.kt b/plugins/kotlin-serialization/kotlin-serialization-compiler/src/org/jetbrains/kotlinx/serialization/compiler/backend/ir/SerializableIrGenerator.kt index b1cded3806d..cd5272e80b5 100644 --- a/plugins/kotlin-serialization/kotlin-serialization-compiler/src/org/jetbrains/kotlinx/serialization/compiler/backend/ir/SerializableIrGenerator.kt +++ b/plugins/kotlin-serialization/kotlin-serialization-compiler/src/org/jetbrains/kotlinx/serialization/compiler/backend/ir/SerializableIrGenerator.kt @@ -1,19 +1,23 @@ package org.jetbrains.kotlinx.serialization.compiler.backend.ir import org.jetbrains.kotlin.backend.common.BackendContext +import org.jetbrains.kotlin.backend.common.deepCopyWithVariables import org.jetbrains.kotlin.backend.common.lower.irThrow import org.jetbrains.kotlin.builtins.KotlinBuiltIns import org.jetbrains.kotlin.codegen.CompilationException import org.jetbrains.kotlin.descriptors.ClassConstructorDescriptor import org.jetbrains.kotlin.descriptors.ClassDescriptor import org.jetbrains.kotlin.descriptors.FunctionDescriptor -import org.jetbrains.kotlin.ir.IrElement import org.jetbrains.kotlin.ir.builders.* -import org.jetbrains.kotlin.ir.declarations.* +import org.jetbrains.kotlin.ir.declarations.IrAnonymousInitializer +import org.jetbrains.kotlin.ir.declarations.IrClass +import org.jetbrains.kotlin.ir.declarations.IrProperty +import org.jetbrains.kotlin.ir.declarations.IrValueParameter import org.jetbrains.kotlin.ir.expressions.IrExpression import org.jetbrains.kotlin.ir.expressions.impl.IrDelegatingConstructorCallImpl -import org.jetbrains.kotlin.ir.util.* -import org.jetbrains.kotlin.ir.visitors.acceptVoid +import org.jetbrains.kotlin.ir.util.SymbolTable +import org.jetbrains.kotlin.ir.util.TypeTranslator +import org.jetbrains.kotlin.ir.util.patchDeclarationParents import org.jetbrains.kotlin.js.resolve.diagnostics.findPsi import org.jetbrains.kotlin.resolve.BindingContext import org.jetbrains.kotlin.resolve.descriptorUtil.getSuperClassOrAny @@ -99,22 +103,10 @@ class SerializableIrGenerator( irClass.declarations.asSequence() .filterIsInstance() .forEach { initializer -> - initializer.body.statements.forEach { +it.deepCopyWithSymbols(initialParent = ctor) } + initializer.body.deepCopyWithVariables().statements.forEach { +it } } } - // todo: this is copypaste from DeepCopyIrWithSymbols.kt, - // remove after KT-18563 is fixed and bootstrap updated. - private inline fun T.deepCopyWithSymbols( - initialParent: IrDeclarationParent? = null, - descriptorRemapper: DescriptorsRemapper = DescriptorsRemapper.Default - ): T { - val symbolRemapper = DeepCopySymbolRemapper(descriptorRemapper) - acceptVoid(symbolRemapper) - val typeRemapper = DeepCopyTypeRemapper(symbolRemapper) - return transform(DeepCopyIrTreeWithSymbols(symbolRemapper, typeRemapper), null).patchDeclarationParents(initialParent) as T - } - private fun IrBlockBodyBuilder.generateSuperNonSerializableCall(superClass: ClassDescriptor) { val suitableCtor = superClass.constructors.singleOrNull { it.valueParameters.size == 0 } ?: throw IllegalArgumentException("Non-serializable parent of serializable $serializableDescriptor must have no arg constructor") @@ -165,8 +157,10 @@ class SerializableIrGenerator( ) { val serializableClass = irClass.descriptor - if (serializableClass.isInternalSerializable) + if (serializableClass.isInternalSerializable) { SerializableIrGenerator(irClass, context, bindingContext).generate() + irClass.patchDeclarationParents(irClass.parent) + } else if (serializableClass.hasSerializableAnnotationWithoutArgs && !serializableClass.hasCompanionObjectAsSerializer) { throw CompilationException( "@Serializable annotation on $serializableClass would be ignored because it is impossible to serialize it automatically. " + diff --git a/plugins/kotlin-serialization/kotlin-serialization-compiler/src/org/jetbrains/kotlinx/serialization/compiler/backend/ir/SerializerIrGenerator.kt b/plugins/kotlin-serialization/kotlin-serialization-compiler/src/org/jetbrains/kotlinx/serialization/compiler/backend/ir/SerializerIrGenerator.kt index 93acfa8150c..d52ee6a53b3 100644 --- a/plugins/kotlin-serialization/kotlin-serialization-compiler/src/org/jetbrains/kotlinx/serialization/compiler/backend/ir/SerializerIrGenerator.kt +++ b/plugins/kotlin-serialization/kotlin-serialization-compiler/src/org/jetbrains/kotlinx/serialization/compiler/backend/ir/SerializerIrGenerator.kt @@ -6,6 +6,7 @@ package org.jetbrains.kotlinx.serialization.compiler.backend.ir import org.jetbrains.kotlin.backend.common.BackendContext +import org.jetbrains.kotlin.backend.common.deepCopyWithVariables import org.jetbrains.kotlin.backend.common.lower.createIrBuilder import org.jetbrains.kotlin.backend.common.lower.irIfThen import org.jetbrains.kotlin.backend.common.lower.irThrow @@ -101,14 +102,14 @@ class SerializerIrGenerator(val irClass: IrClass, override val compilerContext: // add property annotations copySerialInfoAnnotationsToDescriptor( classProp.irField.correspondingProperty?.annotations.orEmpty(), - irGet(localDesc), + localDesc, serialDescImplClass.referenceMethod(CallingConventions.addAnnotation) ) } // add class annotations copySerialInfoAnnotationsToDescriptor( serializableIrClass.annotations, - irGet(localDesc), + localDesc, serialDescImplClass.referenceMethod(CallingConventions.addClassAnnotation) ) @@ -129,12 +130,12 @@ class SerializerIrGenerator(val irClass: IrClass, override val compilerContext: private fun IrBlockBodyBuilder.copySerialInfoAnnotationsToDescriptor( annotations: List, - receiver: IrExpression, + receiver: IrVariable, method: IrFunctionSymbol ) { annotations.forEach { annotationCall -> if ((annotationCall.descriptor as? ClassConstructorDescriptor)?.constructedClass?.isSerialInfoAnnotation == true) - +irInvoke(receiver, method, annotationCall) + +irInvoke(irGet(receiver), method, annotationCall.deepCopyWithVariables()) } } @@ -216,8 +217,7 @@ class SerializerIrGenerator(val irClass: IrClass, override val compilerContext: val localOutput = irTemporary(call, "output") fun SerializableProperty.irGet(): IrGetField { - val ownerType = (descriptor.containingDeclaration as? ClassDescriptor)?.defaultType?.toIrType() ?: - throw IllegalStateException("Serializable property must be contained in class") + val ownerType = objectToSerialize.symbol.owner.type return irGetField( irGet( type = ownerType, @@ -303,6 +303,10 @@ class SerializerIrGenerator(val irClass: IrClass, override val compilerContext: } override fun generateLoad(function: FunctionDescriptor) = irClass.contributeFunction(function, fromStubs = true) { loadFunc -> + if (serializableDescriptor.modality == Modality.ABSTRACT || serializableDescriptor.modality == Modality.SEALED) { + return@contributeFunction + } + fun irThis(): IrExpression = IrGetValueImpl(startOffset, endOffset, loadFunc.dispatchReceiverParameter!!.symbol) @@ -351,7 +355,6 @@ class SerializerIrGenerator(val irClass: IrClass, override val compilerContext: +irSetVar(indexVar.symbol, irInvoke(localInput.get(), readElementF, localSerialDesc.get())) +irWhen { // if index == -2 (READ_ALL) todo... - +IrBranchImpl(irEquals(indexVar.get(), irInt(-2)), irThrowIse()) // TODO throw proper exception // if index == -1 (READ_DONE) break loop +IrBranchImpl(irEquals(indexVar.get(), irInt(-1)), irSetVar(flagVar.symbol, irBoolean(false))) @@ -432,12 +435,7 @@ class SerializerIrGenerator(val irClass: IrClass, override val compilerContext: compilerContext.externalSymbols.referenceConstructor(serializableDescriptor.unsubstitutedPrimaryConstructor!!) } - // todo: throw UnsupportedOperationException when feature branch with `open SerializerGenerator` (enums) will be merged - if (serializableDescriptor.modality != Modality.ABSTRACT && serializableDescriptor.modality != Modality.SEALED) { - +irReturn(irInvoke(null, ctor, typeArgs, args)) - } else { - +irThrowIse() // TODO throw proper exception - } + +irReturn(irInvoke(null, ctor, typeArgs, args)) } companion object { @@ -446,8 +444,10 @@ class SerializerIrGenerator(val irClass: IrClass, override val compilerContext: context: BackendContext, bindingContext: BindingContext ) { - if (getSerializableClassDescriptorBySerializer(irClass.descriptor) != null) + if (getSerializableClassDescriptorBySerializer(irClass.descriptor) != null) { SerializerIrGenerator(irClass, context, bindingContext).generate() + irClass.patchDeclarationParents(irClass.parent) + } } } } diff --git a/plugins/kotlin-serialization/kotlin-serialization-compiler/testData/codegen/Basic.ir.txt b/plugins/kotlin-serialization/kotlin-serialization-compiler/testData/codegen/Basic.ir.txt index f9193a241bd..1ba129c00b8 100644 --- a/plugins/kotlin-serialization/kotlin-serialization-compiler/testData/codegen/Basic.ir.txt +++ b/plugins/kotlin-serialization/kotlin-serialization-compiler/testData/codegen/Basic.ir.txt @@ -40,7 +40,8 @@ public final class ListOfUsers$$serializer : java/lang/Object, kotlinx/serializa LINENUMBER (12) ICONST_1 ANEWARRAY - DUP + ASTORE (1) + ALOAD (1) ICONST_0 NEW DUP @@ -48,6 +49,7 @@ public final class ListOfUsers$$serializer : java/lang/Object, kotlinx/serializa CHECKCAST INVOKESPECIAL (kotlinx/serialization/internal/ArrayListSerializer, , (Lkotlinx/serialization/KSerializer;)V) AASTORE + ALOAD (1) ARETURN LABEL (L1) } @@ -86,13 +88,10 @@ public final class ListOfUsers$$serializer : java/lang/Object, kotlinx/serializa ILOAD (4) TABLESWITCH LABEL (L4) - INVOKESTATIC (kotlin/jvm/internal/Intrinsics, throwNpe, ()V) - GOTO (L5) - LABEL (L6) ICONST_0 ISTORE (3) GOTO (L5) - LABEL (L7) + LABEL (L6) ALOAD (7) ALOAD (2) ICONST_0 @@ -111,7 +110,7 @@ public final class ListOfUsers$$serializer : java/lang/Object, kotlinx/serializa IOR ISTORE (5) GOTO (L5) - LABEL (L8) + LABEL (L7) NEW DUP ILOAD (4) @@ -124,7 +123,7 @@ public final class ListOfUsers$$serializer : java/lang/Object, kotlinx/serializa LABEL (L3) ALOAD (7) ALOAD (2) - LABEL (L9) + LABEL (L8) LINENUMBER (12) INVOKEINTERFACE (kotlinx/serialization/CompositeDecoder, endStructure, (Lkotlinx/serialization/SerialDescriptor;)V) NEW @@ -136,7 +135,7 @@ public final class ListOfUsers$$serializer : java/lang/Object, kotlinx/serializa CHECKCAST INVOKESPECIAL (ListOfUsers, , (ILjava/util/List;Lkotlinx/serialization/SerializationConstructorMarker;)V) ARETURN - LABEL (L10) + LABEL (L9) } public java.lang.Object deserialize(kotlinx.serialization.Decoder decoder) { @@ -330,11 +329,13 @@ public final class OptionalUser$$serializer : java/lang/Object, kotlinx/serializ LINENUMBER (9) ICONST_1 ANEWARRAY - DUP + ASTORE (1) + ALOAD (1) ICONST_0 GETSTATIC (INSTANCE, LUser$$serializer;) CHECKCAST AASTORE + ALOAD (1) ARETURN LABEL (L1) } @@ -373,13 +374,10 @@ public final class OptionalUser$$serializer : java/lang/Object, kotlinx/serializ ILOAD (4) TABLESWITCH LABEL (L4) - INVOKESTATIC (kotlin/jvm/internal/Intrinsics, throwNpe, ()V) - GOTO (L5) - LABEL (L6) ICONST_0 ISTORE (3) GOTO (L5) - LABEL (L7) + LABEL (L6) ALOAD (7) ALOAD (2) ICONST_0 @@ -394,7 +392,7 @@ public final class OptionalUser$$serializer : java/lang/Object, kotlinx/serializ IOR ISTORE (5) GOTO (L5) - LABEL (L8) + LABEL (L7) NEW DUP ILOAD (4) @@ -407,7 +405,7 @@ public final class OptionalUser$$serializer : java/lang/Object, kotlinx/serializ LABEL (L3) ALOAD (7) ALOAD (2) - LABEL (L9) + LABEL (L8) LINENUMBER (9) INVOKEINTERFACE (kotlinx/serialization/CompositeDecoder, endStructure, (Lkotlinx/serialization/SerialDescriptor;)V) NEW @@ -419,7 +417,7 @@ public final class OptionalUser$$serializer : java/lang/Object, kotlinx/serializ CHECKCAST INVOKESPECIAL (OptionalUser, , (ILUser;Lkotlinx/serialization/SerializationConstructorMarker;)V) ARETURN - LABEL (L10) + LABEL (L9) } public java.lang.Object deserialize(kotlinx.serialization.Decoder decoder) { @@ -624,6 +622,7 @@ public final class OptionalUser : java/lang/Object { CHECKCAST ICONST_1 ACONST_NULL + CHECKCAST INVOKESPECIAL (OptionalUser, , (LUser;ILkotlin/jvm/internal/DefaultConstructorMarker;)V) RETURN LABEL (L1) @@ -678,16 +677,18 @@ public final class User$$serializer : java/lang/Object, kotlinx/serialization/in LINENUMBER (6) ICONST_2 ANEWARRAY - DUP + ASTORE (1) + ALOAD (1) ICONST_0 GETSTATIC (INSTANCE, Lkotlinx/serialization/internal/StringSerializer;) CHECKCAST AASTORE - DUP + ALOAD (1) ICONST_1 GETSTATIC (INSTANCE, Lkotlinx/serialization/internal/StringSerializer;) CHECKCAST AASTORE + ALOAD (1) ARETURN LABEL (L1) } @@ -728,13 +729,10 @@ public final class User$$serializer : java/lang/Object, kotlinx/serialization/in ILOAD (4) TABLESWITCH LABEL (L4) - INVOKESTATIC (kotlin/jvm/internal/Intrinsics, throwNpe, ()V) - GOTO (L5) - LABEL (L6) ICONST_0 ISTORE (3) GOTO (L5) - LABEL (L7) + LABEL (L6) ALOAD (8) ALOAD (2) ICONST_0 @@ -746,7 +744,7 @@ public final class User$$serializer : java/lang/Object, kotlinx/serialization/in IOR ISTORE (5) GOTO (L5) - LABEL (L8) + LABEL (L7) ALOAD (8) ALOAD (2) ICONST_1 @@ -758,7 +756,7 @@ public final class User$$serializer : java/lang/Object, kotlinx/serialization/in IOR ISTORE (5) GOTO (L5) - LABEL (L9) + LABEL (L8) NEW DUP ILOAD (4) @@ -771,7 +769,7 @@ public final class User$$serializer : java/lang/Object, kotlinx/serialization/in LABEL (L3) ALOAD (8) ALOAD (2) - LABEL (L10) + LABEL (L9) LINENUMBER (6) INVOKEINTERFACE (kotlinx/serialization/CompositeDecoder, endStructure, (Lkotlinx/serialization/SerialDescriptor;)V) NEW @@ -785,7 +783,7 @@ public final class User$$serializer : java/lang/Object, kotlinx/serialization/in CHECKCAST INVOKESPECIAL (User, , (ILjava/lang/String;Ljava/lang/String;Lkotlinx/serialization/SerializationConstructorMarker;)V) ARETURN - LABEL (L11) + LABEL (L10) } public java.lang.Object deserialize(kotlinx.serialization.Decoder decoder) { @@ -965,4 +963,4 @@ public final class User : java/lang/Object { public final java.lang.String getFirstName() public final java.lang.String getLastName() -} \ No newline at end of file +}