Allow index initialisers to be used in fixed size buffers.

[YairHalberstadt]

Fixes #39632

Code Gen requires no changes. We just remove an incorrect error.

[RikkiGibson]

It feels like it would be better to just make this a local above this statement.

[RikkiGibson]

I'm confused, wasn't this meant to test an object initializer?

[YairHalberstadt]

oops

[AlekseyTs]

.VerifyDiagnostics(); [](start = 16, length = 21)

For success cases we prefer executing the code and observing that it behaves as expected.

[gafter]

This should be dependent on language version 7.3 (or probably preview) due to the fact that it depends on https://github.com/dotnet/csharplang/blob/master/proposals/csharp-7.3/indexing-movable-fixed-fields.md, for example in the translation of

class C
{
    unsafe void M()
    {
        C1 c = new C1()
        {
            S = { X = { [0] = 1, [2] = 2 } }
        };
    }
}

unsafe struct S1
{
    public fixed int X[10];
}

class C1
{
    public S1 S;
}

[AlekseyTs]

{ Prop = 1 } [](start = 24, length = 12)

Consider also testing with an empty initializer and with an initializer that mixes assignments (indexed followed by non-indexed and in reversed order).

[AlekseyTs]

Done with review pass (iteration 1)

[RikkiGibson]

Do we have a test that shows that the following fails: new S { V = { 0, 1, 2 } }

[YairHalberstadt]

Yes in Unsafe tests (Semantic instead of CodeGen tests)

[YairHalberstadt]

@jaredpar

Would you be able to assign someone to this please?

Thanks

[agocke]

I actually don't buy @gafter's reasoning here. The spec has this to say about fixed and moveable variables:

In precise terms, a fixed variable is one of the following:

• A variable resulting from a simple_name (Simple names) that refers to a local variable or a value parameter, unless the variable is captured by an anonymous function.
• A variable resulting from a member_access (Member access) of the form V.I, where V is a fixed variable of a struct_type.
• A variable resulting from a pointer_indirection_expression (Pointer indirection) of the form *P, a pointer_member_access (Pointer member access) of the form P->I, or a pointer_element_access (Pointer element access) of the form P[E].

All other variables are classified as moveable variables.

My read of this is that V, in the context of an object initializer, refers via member access to the containing struct, which is an expression of the struct type. Further, my read of the spec is that this isn't described as a variable at all, which I suppose would leave behavior unspecified. However, it appears that we treat expressions of struct type (appropriately) as fixed.

In this case, the bug here has nothing to do with fixed-size buffer indexing, which was added in C# 7.3, but the simple fact that the unnamed temporary here is being incorrectly referred to as moveable, when it is actually supposed to be fixed.

[YairHalberstadt]

@agocke
Are there any changes that have to be made here?

[YairHalberstadt]

@agocke, @gafter

Should I close this? Or do you still have to discuss the spec?

Thanks

[agocke]

We should definitely take this. Only question is what language version to require. I haven't looked at the implementation yet, I'll try to get to that today.

[agocke]

Sorry for the delay, really messed up my local Git clone on Friday and had to fix it before I could review this

[agocke]

Yeah, this looks wrong. If my read of the spec is correct, the categorization of the receiver is non-moveable. So we shouldn't need to know if these are index expressions in the example

using System;

unsafe struct S
{
    public fixed double V[3];

    static void Main()
    {
        var s = new S { V = 
        {
            [0] = 0,
            [1] = 1,
            [2] = 2,
        } };
        
        Console.Write(s.V[0]);
        Console.Write(s.V[1]);
        Console.Write(s.V[2]);
    }
}

I think the problem may be in Binder.IsMoveableVariable. Initializers should create a BoundObjectOrCollectionValuePlaceholder and, by my read, that placeholder is always a local temporary and thus immoveable.

[CyrusNajmabadi]

@jaredpar @agocke any thoughts on what shoudl be done with this outstanding PR? THanks!