Lifetime tracking considerations for MemoryMarshal.CreateSpan

[Sergio0694]

Opening this discussion here as suggested by @jaredpar, so it's public to anyone given it's nothing internal.

I've been thinking about the whole "what do we do with MemoryMarshal.CreateSpan issue", and came to the conclusion that we might have made an assumption on its usage that's not necessarily correct. To recap, the current proposal as it stands (see dotnet/csharplang#5602) makes the following distinction between what is legal today and what "needs to remain" legal tomorrow, specifically with respect to CreateSpan. We have the following, as per Jared's proposal:

// This will be legal in C# 11
public ref struct Span<T>
{
    public Span(ref T item);
}

// This needs to remain legal and NOT tracked
public static class MemoryMarshal
{
    public static Span<T> CreateSpan<T>(ref T item, int length);
}

Now, the point I'm trying to make here is that I'm not convinced the second part is actually true. Do we really need CreateSpan not to track the lifetime of the target reference? It seems to me that we might have overlooked the real reason why that API is so crucial to people like eg. @tannergooding, me, and other runtime developers, and that is that this API takes an arbitrary length. This alone would still be enough to make it (1) absolutely crucial to have and (2) not safe, but I don't think the "not tracking the lifetime" bit is necessary a critical component of this. Yes, right now there's also the fact that it lets you do things not expressable in C# (eg. escaping a struct field), but the point is that doing so will already be legal in C# 11 with proper annotations. In other words, I'm thinking that even if this API started properly tracking the lifetime of the target reference, it would still keep being perfectly useful in the scenarios where it's used today, and where you most definitely would want to add proper lifetime annotations in C# 11 as well. To make some examples:

• If you're using it on a stackalloc buffer, then you surely wouldn't return it to the caller, so it being tracked is ok.
• If you're using it to track a single local, then you'd just start using the new Span<T>(ref T) constructor anyway.
• If you're using it to wrap data on the heap (eg. a slice of an MD array), then it being tracked is ok, the ref is on the heap anyway.

In all other cases where people (myself included) are using this API to trick the compiler and return a ref to an instance member of a struct, well then once C# 11 is out you'd most definitely want to refactor all those places anyway to give callers proper annotations and guardrails so they don't end up with dangling references anyway. This would be the case in eg. all cases where people today are handrolling managed fixed buffer types, you'd be perfectly fine with the API also tracking the lifetime in those scenarios as well.

So basically, unless I'm missing something, I'm now getting convinced that breaking and adding proper lifetime tracking to CreateSpan, as its main point (the arbitrary length) would still be available, and the language would allow you to not need it anymore for the tricks where it's needed today. Granted, in case keeping it completely unsafe didn't require additional work then I guess we could also just leave it as is, but in case it was easier to make it consistent with the rules anywhere else in C# 11, I'm thinking it might be actually worth considering taking the breaking change here. I just can't really think of cases right now where this could actually be an issue 🤔

NOTE: wasn't really sure whether to open this here or on the C# repo given it touches both the language and the actual API from the BCL. Opening it here for now but feel free to move to the other repo if that's deemed more appropriate for this of course 😊

cc. @stephentoub @davidwrighton (feel free to tag anyone else if needed)

[tannergooding]

There are always going to be certain users with certain cases where they may want to break or violate the current language rules. There may even be other languages (F#, VB, etc) that don't get support for the new lifetime rules at the same time as C# (or possibly even ever).

I think an API that tracks the lifetime of the ref while also unsafely allowing a length is a potentially different APIs (Span.CreateUnsafe(ref T, int) maybe?)

[Sergio0694]

I can't think of scenarios in C# where you'd still need that, but yeah I can see the point for other languages. I like the idea of adding a Span<T>.CreateUnsafe(ref T, int) API a lot, it'd get all proper lifetime tracking and could be the recommended way to create spans with arbitrary length for C# developers, while still being much better than CreateSpan due to compiler support for tracking. And having it there on Span<T> itself rather than buried under MemoryMarshal would also be nice (after all Span<T> also has a constructor with a pointer and arbitrary length anyway, so don't see why this API couldn't be next to that as well). Should we open an API proposal for that? 😄

[DaZombieKiller]

The existence of the void*, int constructor seems like a valid argument for pushing harder on [RequireUnsafeContext] (or whatever that would be called), as it would allow this to become a constructor too, while still making its status as an unsafe API clear (and being more consistent than a static factory on the type): [RequireUnsafeContext] public Span(ref T, int).

[panost]

They are some examples of methods that use ref T and length. For example CompareStringIgnoreCase or in my CRC32 table computation. In the case you want to call some Span<T> method, (Span<T>.IndexOf for example) inside those methods, what are your options? I mean other than calling MemoryMarshal.CreateSpan<T>

[Sergio0694]

The point I'm making is that in those cases you'd need a way to create a Span<T> from a ref T and an arbitrary length, but it would still be fine (and arguably better) to still have the compiler tracking the lifetime though. In those methods, the resulting Span<T> would simply have the same ref escaping rules as the input ref T value. The change I'm proposing here would not prevent you from using APIs like Span<T>.IndexOf from such methods, like you mentioned, it would simply give you additional safety checks on top 🙂

[jaredpar]

For me there are two separate parts to this discussion.

The first is the language side. The language is taking a minor compatibility break with the ref fields design. While the compat break is tiny there are APIs today that conflict with that change. In order to enable smooth upgrade from C# 10 to 11 the language needs to provide the tools necessary for developers to define APIs in C# 11 that have compatible use cases with their C# 10 counterparts. That comes in the form of scoped annotations. While these are provided in part because of compat concerns they have significant other uses that will make the language more flexible.

The second part is the runtime side who defines APIs like MemoryMarshal.CreateSpan. The specific annotations that should or should not be added to this API is not really a language issue as it has no meaning to the language. It's simply a runtime provided API that the language can interact with.

I share the feeling that the uses of MemoryMarshal.CreateSpan will decrease significantly in the presence of ref fields. It's likely that many uses, particularly when 1 is specified as the length, should be changed to simply new Span<T>. At the same time the runtime team has always strongly favored to compatibility in cases like this and my imagination is they will do so with this decision.

I think the best path forward here would be looking to define analyzers that could spot the cases that would be better off as new Span<T> and push for that to be included in the runtime. I am having trouble thinking of how such an analyzer would be designed. Even looking simply for 1 as the length may not be sufficient as it's possible that it would result in a high false positive rate if the intent was to use it unsafely.

[tannergooding]

Even looking simply for 1 as the length may not be sufficient as it's possible that it would result in a high false positive rate if the intent was to use it unsafely.

I'd be fine with this. If you are doing something which truly can't be handled by the switch to new Span<T>(ref value) then you should probably explicitly need to deal with a warning. I'd speculate this is an accidental bug more often than not.

[tannergooding]

*can't be handled by the switch to new Span<T>(ref value) or some other Span<T>.CreateUnsafe(ref value, length) API where the lifetime is tracked