Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Value classes not using encodeInlineElement and inefficient boxing of primitive values #2738

Open
Chuckame opened this issue Jul 3, 2024 · 0 comments

Comments

@Chuckame
Copy link
Contributor

Chuckame commented Jul 3, 2024

Describe the bug
Given a serializable data class containing 2 value class fields (one having a primitive type like int, the other with a structure), both fields are serialized using encodeSerializableElement instead of encodeInlineElement, forcing primitive values to be boxed, and also doesn't follow the expected as said in the docs:

    /**
     * [...]
     * 
     * Namely, for the `@Serializable @JvmInline value class MyInt(val my: Int)`,
     * and `@Serializable class MyData(val myInt: MyInt)` the following sequence is used:
     * ```
     * thisEncoder.encodeInlineElement(MyData.serializer.descriptor, 0).encodeInt(my)
     * ```
     *
     * This method provides an opportunity for the optimization to avoid boxing of a carried value
     * and its invocation should be equivalent to the following:
     * ```
     * thisEncoder.encodeSerializableElement(MyData.serializer.descriptor, 0, MyInt.serializer(), myInt)
     * ```
     *
     * [...]
     *
     * Note that this function returns [Encoder] instead of the [CompositeEncoder]
     * because value classes always have the single property.
     * 
     * [...]
     */
    public fun encodeInlineElement(
        descriptor: SerialDescriptor,
        index: Int
    ): Encoder

To Reproduce

class ReproduceBug {
    @Test
    fun structureContainingValueClassFieldsShouldCall_encodeInlineElement() {
        val value = Structure(
            BooleanValue(true),
            BooleanValue(false),
            SubStructureValue(SubStructure(42)),
        )
        val encoder = MyEncoder()

        encoder.encodeSerializableValue(Structure.serializer(), value)

        assertEquals(actual = encoder.encodeInlineElementCalled, expected = true)
    }

    @Serializable
    data class Structure(
        val primitiveField: BooleanValue,
        val primitiveNullableField: BooleanValue?,
        val field: SubStructureValue,
    )

    @Serializable
    @JvmInline
    value class BooleanValue(val value: Boolean)

    @Serializable
    @JvmInline
    value class SubStructureValue(val value: SubStructure)

    @Serializable
    data class SubStructure(
        val subField: Int,
    )

    class MyEncoder: AbstractEncoder() {
        var encodeInlineElementCalled = false

        override val serializersModule: SerializersModule
            get() = EmptySerializersModule()

        // I removed the `final` modifier in the `AbstractEncoder` to allow overriding it for reproducing the bug and avoid the useless noise of all the other overrides
        override fun encodeInlineElement(descriptor: SerialDescriptor, index: Int): Encoder {
            encodeInlineElementCalled = true
            return super.encodeInlineElement(descriptor, index)
        }

        override fun encodeValue(value: Any) {
        }
    }
}

And here is the generated serializer (only the interesting part) retrieved using Kotlin bytecode IDEA tooling:

output.encodeSerializableElement(serialDesc, 0, (SerializationStrategy)ReproduceBug.BooleanValue.$serializer.INSTANCE, ReproduceBug.BooleanValue.box-impl(self.primitiveField));
output.encodeNullableSerializableElement(serialDesc, 1, (SerializationStrategy)ReproduceBug.BooleanValue.$serializer.INSTANCE, self.primitiveNullableField);
output.encodeSerializableElement(serialDesc, 2, (SerializationStrategy)ReproduceBug.SubStructureValue.$serializer.INSTANCE, ReproduceBug.SubStructureValue.box-impl(self.field));

Expected behavior
Should call encodeInlineElement and generate the following (not sure for the nullable value class' field):

output.encodeInlineElement(serialDesc, 0, (SerializationStrategy)ReproduceBug.BooleanValue.$serializer.INSTANCE).encodeBoolean(self.primitiveField);
output.encodeNullableSerializableElement(serialDesc, 1, (SerializationStrategy)ReproduceBug.BooleanValue.$serializer.INSTANCE, self.primitiveNullableField);
output.encodeInlineElement(serialDesc, 2, (SerializationStrategy)ReproduceBug.SubStructureValue.$serializer.INSTANCE).encodeSerializableValue((SerializationStrategy)ReproduceBug.SubStructure.$serializer.INSTANCE, self.field);

Environment

  • Kotlin version: 1.9.22 & 2.0.0
  • Library version: 1.7.0 & dev (commit 6886e345e06a48c4b0f9014bf85047a1e2f96a29)
  • Kotlin platforms: jvm (and surely the other platforms, but not tested)
  • Gradle version: 8.7
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant