Cysharp / MemoryPack

Zero encoding extreme performance binary serializer for C# and Unity.
MIT License
3.29k stars 193 forks source link

Generated code for deserialization skips assignment of a dictionary prop #132

Closed Phloog closed 1 year ago

Phloog commented 1 year ago

Hey,

Assume the following classes:

[MemoryPackable]
public partial class Example
{
        public string MyString { get; set; }

        public int? MyInt { get; set; }

        public DateTime MyDate { get; set; }

        public bool MyBool { get; set; }

        public Dictionary<string, HashSet<ISomeInterface>> Stuff { get; } = new Dictionary<string, HashSet<ISomeInterface>>();
}

[MemoryPackable(GenerateType.NoGenerate)]
public interface ISomeInterface
{
    string StringInInterface { get; set; }
    int IntInInterface { get; set; }
}

[MemoryPackable]
public partial class ImplA : ISomeInterface
{
    public string StringInInterface { get; set; }
    public int IntInInterface { get; set; }
    public string AnotherStringInA { get; set; }
}

[MemoryPackable]
public partial class ImplB : ISomeInterface
{
    public string StringInInterface { get; set; }
    public int IntInInterface { get; set; }
    public int AnotherIntInB { get; set; }
}

[MemoryPackUnionFormatter(typeof(ISomeInterface))]
[MemoryPackUnion(0, typeof(ImplA))]
[MemoryPackUnion(1, typeof(ImplB))]
public partial class ExampleUnionFormatter
{
}

The following code is generated by MemoryPack 1.9.13:

    [global::MemoryPack.Internal.Preserve]
    public static void Deserialize(ref MemoryPackReader reader, ref Example? value)
    {

        if (!reader.TryReadObjectHeader(out var count))
        {
            value = default!;
            goto END;
        }

        string __MyString;
        int? __MyInt;
        global::System.DateTime __MyDate;
        bool __MyBool;
        global::System.Collections.Generic.Dictionary<string, global::System.Collections.Generic.HashSet<global::Example.ISomeInterface>> __Stuff;

        if (count == 5)
        {
            if (value == null)
            {
                __MyString = reader.ReadString();
                reader.DangerousReadUnmanaged(out __MyInt, out __MyDate, out __MyBool);
                __Stuff = reader.ReadValue<global::System.Collections.Generic.Dictionary<string, global::System.Collections.Generic.HashSet<global::Example.ISomeInterface>>>();

                goto NEW;
            }
            else
            {
                __MyString = value.@MyString;
                __MyInt = value.@MyInt;
                __MyDate = value.@MyDate;
                __MyBool = value.@MyBool;
                __Stuff = value.@Stuff;

                __MyString = reader.ReadString();
                reader.DangerousReadUnmanaged(out __MyInt);
                reader.ReadUnmanaged(out __MyDate);
                reader.ReadUnmanaged(out __MyBool);
                reader.ReadValue(ref __Stuff);

                goto SET;
            }

        }
        else if (count > 5)
        {
            MemoryPackSerializationException.ThrowInvalidPropertyCount(typeof(Example), 5, count);
            goto READ_END;
        }
        else
        {
            if (value == null)
            {
               __MyString = default!;
               __MyInt = default!;
               __MyDate = default!;
               __MyBool = default!;
               __Stuff = default!;
            }
            else
            {
               __MyString = value.@MyString;
               __MyInt = value.@MyInt;
               __MyDate = value.@MyDate;
               __MyBool = value.@MyBool;
               __Stuff = value.@Stuff;
            }

            if (count == 0) goto SKIP_READ;
            __MyString = reader.ReadString(); if (count == 1) goto SKIP_READ;
            reader.DangerousReadUnmanaged(out __MyInt); if (count == 2) goto SKIP_READ;
            reader.ReadUnmanaged(out __MyDate); if (count == 3) goto SKIP_READ;
            reader.ReadUnmanaged(out __MyBool); if (count == 4) goto SKIP_READ;
            reader.ReadValue(ref __Stuff); if (count == 5) goto SKIP_READ;

    SKIP_READ:
            if (value == null)
            {
                goto NEW;
            }
            else            
            {
                goto SET;
            }

        }

    SET:

        value.@MyString = __MyString;
        value.@MyInt = __MyInt;
        value.@MyDate = __MyDate;
        value.@MyBool = __MyBool;
        goto READ_END;

    NEW:
        value = new TransferModelImportPrepData()
        {
            @MyString = __MyString,
            @MyInt = __MyInt,
            @MyDate = __MyDate,
            @MyBool = __MyBool
        };
    READ_END:

    END:

        return;
    }

As you can see, the Dictionary "Stuff" is correctly deserialized; only in the very last block (at SET: and NEW:), its assignment to the target object is omitted. When debugging and stepping through the generated deserialization-method, I can see the data is correctly restored in __Stuff !

Is this a bug or do I need to change my annotations somehow?

Thanks, Tim

Phloog commented 1 year ago

D'oh! Just when submitting the issue, I noticed the "Stuff" Dict only has a get accessor.

I assume there's no possibility to set contents into properties with private setters, right? I'll need to change my Example-class then.

neuecc commented 1 year ago

Yes, a SET is required. MemoryPack allows asymmetric serialize (serialize only). This is because MessagePack and JSON required it. However, this may not be the case for MemoryPack. .......