Error in replace-stackalloc-expression-with-collection-expression fix

[bernd5]

Steps to Reproduce:

For the following code:

using System;

scoped Span<int> a = default;

int val = 3;
if (val == 3)
{
    int xxx = 123;
    Span<int> s = stackalloc int[3] { xxx, 1, 1 }; //<-- Here the IDE says: use a collection expression
    //Span<int> s = [xxx, 1, 1];
    a = s;
}
foreach (var item in a)
{
    Console.WriteLine(item);
}

the IDE issues a hint to remove the stackalloc expression and replace it with a collection expression.

Expected Behavior:
It should be okay to use a collection expression in that context (especially because the lifetime of InlineArray-Span-instances are extended to the method scope). The suggested fix should work.

Actual Behavior:
The fix does not work. CS8352 is triggered at a = s

[CyrusNajmabadi]

@RikkiGibson curious about your thoughts on this. I'm not certain why there is a difference here between the lifetimes of 's' depending on if it was stackalloc'ed vs collection-initialized.

[RikkiGibson]

I think the error is by design. Collection-exprs are backed by InlineArrays held in the current block, while stackalloc storage is live for the entire call ("function-member"/CurrentMethod).

Fixing the analyzer to only suggest this when the collection does not escape could be tricky, though. I am unsure if we have a public API which really provides the info that the analyzer/fixer would need here.

[bernd5]

Collection-exprs are backed by InlineArrays held in the current block

this is actually not true – they are moved to the top-scope of a method body (see _additionalLocals in the LocalRewriter).
If we ignore the "lifetime-error" via unsafe-block the code works:

using System;

scoped Span<int> a = default;

int val = 3;
if (val == 3)
{
    int xxx = 123;
    //Span<int> s = stackalloc int[3] { xxx, 1, 1 }; //<-- Here the IDE says: use a collection expression
    Span<int> s = [xxx, 1, 1];
    unsafe
    {
        a = s;
    }
}
foreach (var item in a)
{
    Console.WriteLine(item);
}

I assume the warning is issued because it would not work in a loop / goto case. The reason is that we allocate just a single storage which would be overwritten on every iteration.

[bernd5]

What I mean is code like this:

using System;

scoped Span<int> a1 = default;
scoped Span<int> a2 = default;
scoped Span<int> a3 = default;

foreach (var x in (Span<int>)[1, 2, 3])
{
    ref var span = ref x == 1 ? ref a1
        : ref x == 2 ? ref a2
        : ref a3;

    Span<int> temp = stackalloc int[] { x, x, x }; //Here we can't actually use a collection expression because we need separate memory for each iteration
    //Span<int> temp = [x, x, x]; //would overwrite the collection expression storage from previous iterations

    unsafe
    {
        span = temp;
    }
}

Print(a1);
Print(a2);
Print(a3);

void Print(Span<int> span)
{
    foreach (var i in span)
    {
        Console.WriteLine(i);
    }
}

[RikkiGibson]

this is actually not true – they are moved to the top-scope of a method body (see _additionalLocals in the LocalRewriter).

This may be true, but it is our goal to analyze and diagnose the code such that the distinction isn't observable in safe code. We may make such a change in the implementation in the future to optimize stack usage.