dotnet/roslyn

Unnecessary defensive copy in collection spread for ROS nullability change

Closed this issue · 1 comments

Version Used:
17.13.0 Preview 1.0

Steps to Reproduce:

internal static string CombineWithSlash(ReadOnlySpan<string> pathElements)
{
    return string.Join('/', [.. pathElements]);
}

Where string.Join's second parameter is a ReadOnlySpan<string?>

Sharplab.

Expected Behavior:

It is always safe to convert a ReadOnlySpan<U> to a ReadOnlySpan<T> when U inherits from T (and neither are value types).
On .NET 9, this can be turned into string.Join('/', ReadOnlySpan<string?>.CastUp(pathElements)). The nullability case here can be suppressed, since it's known to be safe.

Actual Behavior:

As per sharplab above: return string.Join('/', new ReadOnlySpan<string>(pathElements.ToArray()));

Thanks for filing this issue.

The first class spans feature adds a covariant implicit conversion for ReadOnlySpan which is equivalent to calling CastUp. With this, you will be able to just change [.. pathElements] to pathElements and carry on.

We are not planning to change codegen here for a few reasons, most prominent being that ReadOnlySpan only means you can't mutate it. Someone with a mutable reference to the same memory can, e.g.:

int[] arr = [1,2,3];
ReadOnlySpan<int> ros1 = arr; // reference
ReadOnlySpan<int> ros2 = [..arr]; // copy
arr[0] = 4;
Console.Write(ros1[0]); // expect '4'
Console.Write(ros2[0]); // expect '1'

Closing as by design for the above reasons.