Nested collection initializer doesn't capture the collection in a temp

[gafter]

@alrz commented on Mon Dec 10 2018

Version Used: 7.3

Steps to Reproduce:

using System.Collections.Generic;
public class C {
    List<int> P {get;}
    public void M() {
        new C {P={1,2}};
    }
}

Expected Behavior:

C c = new C();
List<int> p = c.P;
p.Add(1);
p.Add(2);

Actual Behavior:

C c = new C();
c.P.Add(1);
c.P.Add(2);

This was surprising to me, because P is appeared once in the source, so I'd expect a single call to it.

---

@CyrusNajmabadi commented on Mon Dec 10 2018

Spec says:

A member initializer that specifies an object initializer after the equals sign is a nested object initializer, i.e. an initialization of an embedded object. Instead of assigning a new value to the field or property, the assignments in the nested object initializer are treated as assignments to members of the field or property. Nested object initializers cannot be applied to properties with a value type, or to read-only fields with a value type.

This is unfortunately pretty vague. However, the spec also happens to give some examples that shows that the behavior of the compiler is working as expected. i.e.:

var c = new C {
    x = true,
    y = { a = "Hello" },
    z = { 1, 2, 3 },
...

// is equivalent to this series of assignments:

C __c = new C();
__c.x = true;
__c.y.a = "Hello";
__c.z.Add(1); 
__c.z.Add(2);
__c.z.Add(3);
...

So i'd say the compiler is working exactly as expected given that this is exactly how the spec says it should translate.

---

@alrz commented on Mon Dec 10 2018

Thanks for the pointer.

I'm wondering what's the rationale behind this, since we already ruled out value types:

Nested object initializers cannot be applied to properties with a value type

---

@CyrusNajmabadi commented on Mon Dec 10 2018

Note: this is likely necessary for the case of initializing a mutable struct with fields. If a temp was made, then nested assignments wouldn't actually update the actual struct in-situ.

---

@CyrusNajmabadi commented on Mon Dec 10 2018

Nested object initializers cannot be applied to properties with a value type

Because the property would return a copy, hence, any assignments/mutations would be effectively lost in nearly all cases.

---

@CyrusNajmabadi commented on Mon Dec 10 2018

@alrz Imagine if we had:

struct ComplexNumber
{
    public double Real;
    public double Imaginary;
}

struct ComplexPoint
{
    public ComplexNumber X;
    public ComplexNumber Y;
}

static class Program
{
    static void Main()
    {
        var p = new ComplexPoint
        {
            X = { Real = 0, Imaginary = 0 },
            Y = { Real = Math.Sqrt(2), Imaginary = Math.Sqrt(2) }
        };
    }
}

The compiler translates this to:

private static void Main()
{
	ComplexPoint complexPoint = default(ComplexPoint);
	complexPoint.X.Real = 0.0;
	complexPoint.X.Imaginary = 0.0;
	complexPoint.Y.Real = Math.Sqrt(2.0);
	complexPoint.Y.Imaginary = Math.Sqrt(2.0);
}

This is exactly what we want here, and completely initializes the struct with no unnecessary copies. If we instead had:

private static void Main()
{
	ComplexPoint complexPoint = default(ComplexPoint);
        var x = complexPoint.X;
	x.Real = 0.0;
	x.Imaginary = 0.0;
        var y = complexPoint.Y;
	y.Real = Math.Sqrt(2.0);
	y.Imaginary = Math.Sqrt(2.0);
}

This wouldn't even be correct. We'd have to copy the values back with something like:

complexPoint.X = x;
complexPoint.Y = y;

And this presumes that the location is writable (which it might not be for a readonly-field or prop). So, the simplest approach, which works in all cases, is to simply write things out like how the compiler does things. The only 'bad' side is the potential for a slight amount of extra computation on properties to retrieve a value that will probably be the same value. But trying to optimize that case out somehow likely has never been worth it.

---

@alrz commented on Mon Dec 10 2018

Yeah, so this was only necessary for structs, but for consistent side-effects we still do this for reference types. Unfortunate.

---

@alrz commented on Mon Dec 10 2018

You're using fields (which have no side-effects), but with properties this still produce the call for each value, in which case we get a copy anyways? So it looks like a special lowering for structs (struct+field) has been done for anything else!

---

@CyrusNajmabadi commented on Mon Dec 10 2018

but with properties this still produce the call for each value

With properties, this would not be allowed, as these are value types.

---

@gafter commented on Tue Dec 11 2018

Since we appear to be acting as expected, moving to csharplang to reconsider how we want it to behave.

[alrz]

While I believe the default behavior could be a lowering strategy only on certain circumstances, changing it in any ways is likely considered as a breaking change because side effects will be partially lost. Not sure if that's debatable or worth taking.