ListDecorator doesn't handle lists of nullables correctly

[GoogleCodeExporter]

First, my thanks for this library. I find it extremely useful, and I appreciate 
the sheer amount of time/effort you have obviously spent on it.

Now to the issue...

Consider the following case:

    [ProtoContract]
    public class NullableIntList
    {
        [ProtoMember(1)]
        public List<int?> List { get; set; }
    }

    class Program
    {
        static void Main(string[] args)
        {
            NullableIntList l = new NullableIntList();
            l.List = new List<int?>(new int?[] {2, 3, null});
            NullableIntList clone = Serializer.DeepClone(l);
            Console.WriteLine(String.Join(",", clone.List.Select(item => Convert.ToString(item)).ToArray()));
        }
    }

Running this with 2.0.0.452 (VS2010 on windows) will yield:

System.InvalidOperationException was unhandled
  Message=Conflicting item/add type
  Source=protobuf-net
  StackTrace:
       at ProtoBuf.Serializers.ListDecorator.EmitReadAndAddItem(CompilerContext ctx, Local list, IProtoSerializer tail, MethodInfo add) in c:\Dev\protobuf-net\protobuf-net\Serializers\ListDecorator.cs:line 257
       at ProtoBuf.Serializers.ListDecorator.EmitReadList(CompilerContext ctx, Local list, IProtoSerializer tail, MethodInfo add, WireType packedWireType) in c:\Dev\protobuf-net\protobuf-net\Serializers\ListDecorator.cs:line 167
...TRUNCATED...

This is because the itemType will be typeof(int) and the addParamType will be 
typeof(int?)

I upgraded to this version to try and avoid a stackoverflowexception (from 1.x) 
but this broke 8 of my critical serializable types. 
As this is a blocking issue for something I am "on the hook" for, I will be 
looking at writing a patch, but if you have suggestions (or a  fix), I would 
love to (hear them/get it).

Original issue reported on code.google.com by elliott....@gmail.com on 28 Oct 2011 at 10:29

[GoogleCodeExporter]

I can tidy the code such that a Nullable<int> works correctly (done and tested 
locally), however - ultimately protobuf itself (the wire format) has no concept 
of null and no metaphor for representing a null value in a list. This is, you 
could say, an underlying limitation of the wire spec. If you urgently need 
this, I could *invent* such, using an artificial wrapper (I have something in 
mind), but that might take a couple of hours. Let me know so I can prioritise.

Original comment by marc.gravell on 31 Oct 2011 at 8:35

[GoogleCodeExporter]

It wasn't trivial, but the core does now support this. I haven't hooked it up 
to the attributes yet (long day, and I'm tired), but it can ultimately work. If 
you want to play with this using the current SVN code feel free; see in 
Issue243.cs, GetModelWithSupportForNulls() for code to configure a type-model 
manually (which could be RuntimeTypeModel.Default if you want it to impact 
Serializer.*).

Any use?

Original comment by marc.gravell on 31 Oct 2011 at 11:55

• Changed state: Accepted

[GoogleCodeExporter]

Spent the evening handing out candy to costumed urchins. Taking a look now. 
Thanks for pushing that through.

Original comment by elliott....@gmail.com on 1 Nov 2011 at 3:00

[GoogleCodeExporter]

OK. So in v1.x, I had been able to serialize List<T?> properties, but it 
appears mostly by accident. I noticed that null values were getting "eaten", so 
I serialized a parallel list of integer indexes that would be inserted (after 
serialization) as nulls.  So really I had a kluge on a kluge. Obviously the 
v2.x broke my kluged kluge, but in retrospect, it looks like this was never 
intended to be a supported case.

Issue243.cs looks like a great way to handle this in the simple case, so thank 
you for that. Unfortunately my use cases are leaves in an inheritance tree (I 
am serializing a System.Linq.Expression tree for filtering purposes), and so to 
use this method I would have to migrate the entire tree to the runtime model 
approach (as opposed to the attribution approach it currently uses).

However the discussion has informed a simpler workaround that doesn't require a 
new turn of the protobuf-net library (or hosting/compiling a one-off version in 
my project's source control). The change can be localized to a single type that 
should be covered thoroughly by my unit tests.

In short, now that I understand the underlying issue better, I can redesign to 
avoid the problem and use the currently released binaries.

Thanks for your time and attention. FWIW I would consider this issue resolved 
given the Issue243.cs approach.

Original comment by elliott....@gmail.com on 1 Nov 2011 at 4:04

[GoogleCodeExporter]

Original comment by marc.gravell on 2 Nov 2011 at 9:57

• Changed state: Verified