runtime: [Analyzer]: Unexpected allocation when converting arrays to spans

The C# compiler has a valuable optimization that enables code like:

ReadOnlySpan<byte> span = new byte[] { 1, 2, 3 };

to be converted into code that emits that constant data into the assembly, and then this code just creates a span directly referencing that data. However, this optimization is limited to single byte primitive types, to all of the initialization data being constant, and to immediately storing into a ReadOnlySpan<byte>. If you do:

ReadOnlySpan<byte> span = new byte[] { 1, 2, someStaticField };

or

Span<byte> span = new byte[] { 1, 2, 3 };

or

ReadOnlySpan<char> span = new char[] { 'a', 'b', 'c' };

those will all allocate.

With https://github.com/dotnet/runtime/issues/60948 and compiler support via https://github.com/dotnet/roslyn/pull/61414 (or something similar), this optimization will helpfully be extended as well to char, short, ushort, int, uint, long, ulong, single, and double, such that code like:

ReadOnlySpan<char> span = new char[] { 'a', 'b', 'c' };

will similarly become non-allocating… but only when targeting .NET 7 or newer such that RuntimeHelpers.CreateSpan is available. When that methods isn’t available, this same line of code will again regress to allocating.

It’s thus really easy for a developer to accidentally allocate, as this syntax is something a developer typically only writes when they’re actively trying to optimize and avoid the allocation. Hopefully language syntax will help to make this simpler, clearer, and less error prone in the future (https://github.com/dotnet/csharplang/issues/5295), but for now we can add an analyzer to detect these cases and warn the developer about the unexpected allocation.

The analyzer would warn when:

  • Casting a new T[] expression to a ReadOnlySpan<T> either implicitly or explicitly, and
  • Either T isn’t byte/sbyte/bool or CreateSpan is available and T isn’t byte/sbyte/bool/char/ushort/short/uint/int/ulong/long/single/double, or
  • Some of the data being used to initialize the array isn’t const

We could restrict this further to only applying to properties/method following the pattern:

static ReadOnlySpan<T> Data => new T[] { ... };

or we could extend it to statement bodies as well.

The default level should probably be suggestion.

For a fixer, we could write one for the property case, changing it to instead be a static readonly array. Or we could just not have a fixer at all.

About this issue

  • Original URL
  • State: closed
  • Created 2 years ago
  • Reactions: 3
  • Comments: 18 (11 by maintainers)

Most upvoted comments

I’m not sure this analyzer is useful for the general public, it seems like something only dotnet/runtime cares about. Or if does ship with .NET and is available to everyone, I wouldn’t have the analyzer trigger automatically, only when you give a hint that you intend for this to be constant, for example:

// constalloc
static ReadOnlySpan<byte> Data => new byte[] { ... };

The analyzer would see e.g. the “constalloc” comment and would know that you intend for this to not be an array allocation but rather constant data embedded in the assembly. Since you gave an explicit hint, it could even be a warning by default. And it could catch the Span<> case as well like this.

Because what if you really need to have something non constant in there, let’s say a field, and don’t care about an allocation? The analyzer shouldn’t bother people unless they explicitly want this to be constant, not even as a suggestion, because they can do nothing about that suggestion if they really need something non-constant. I think it would be much better if the analyzer only triggered when you explicitly mark the allocation somehow, for example with a comment like that. Alternatively, it could be an attribute that isn’t emitted.

Was this intentional?

It was. I used it as an example, but I think this is likely to be intentional on the part of a developer, and so warning about this would be likely to be noisy. I’d be more inclined to see a separate analyzer that sees you’re never writing to a Span<T> (or passing it to something that accepts a Span and doesn’t have a ReadOnlySpan overload) and recommends it be changed into a ReadOnlySpan<T>, at which point either the optimization would kick in or this analyzer would start warning.