15

I notice that the following will compile and execute even though the local variables are not initialized. Is this a feature of Span?

void Uninitialized()
{
  Span<char> s1;
  var l1 = s1.Length;

  Span<char> s2;
  UninitializedOut(out s2);
  var l2 = s2.Length;
}

void UninitializedOut(out Span<char> s)
{}
jyoung
  • 5,071
  • 4
  • 30
  • 47
  • Looks like a bug to me, but, as far as I know, `out` is not enforced by CLR anyway. – user4003407 Apr 04 '18 at 14:09
  • `Span` is not special; your own `struct Foo { public int Length => 0; }` will trigger this bug (or "suboptimal handling") as well. – Jeroen Mostert Apr 04 '18 at 14:21
  • 1
    @JeroenMostert that one *isn't* a bug, though; `Foo` *genuinely has no fields*, so in terms of definite assignment: it is fine – Marc Gravell Apr 04 '18 at 14:22
  • @MarcGravell: yes -- and no. It's a bad thing: adding a private field would break unsuspecting code, and that sort of brittleness that the client has no control over is undesirable. Allowing this is looking at things with compiler-colored glasses. It *ought* to complain and force `new Foo()` (still compiling to the same thing, of course), even though you can explain why it won't, by the rules. – Jeroen Mostert Apr 04 '18 at 14:26
  • 1
    @JeroenMostert adding a public or private field to a struct would do that *today*; adding fields to structs is *not* a safe change. Consider: `Position pos; pos.X = 12; pos.Y = 42; Draw(pos);` . This is a *perfectly legal* way of initializing a mutable struct with public fields (I'll ignore for now the question of whether mutable structs with public fields are *themselves* good ideas...). Now add a `Z` and your existing code stops compiling. – Marc Gravell Apr 04 '18 at 14:28
  • @MarcGravell: granted. My only remaining quibble then is the compiler diagnostic: "use of unassigned local variable 'structInstance'", which doesn't tell you the real issue is that not all fields have been assigned, and invites you to slap a `new Struct()` to your declaration (just as you'd do with classes), when that's not necessarily the right thing to do. That's orthogonal to allowing this syntax, though -- fieldless structs are degenerate, but not worth issuing a custom warning for. – Jeroen Mostert Apr 04 '18 at 14:41
  • @JeroenMostert what's really breaking my noodle: I checked the reference assembly being used here in `ildasm`, and... *it has a field* (OK, it is a dummy field - but: still a field - `.field private initonly object _dummy`) – Marc Gravell Apr 04 '18 at 14:43

3 Answers3

18

This looks like an issue caused by reference assemblies, required because of the way that Span<T> has framework-specific internals.

This means that in the reference assembly: there are no fields (edit: this isn't quite true - see footnote).

A struct is considered assigned (for the purposes of "definite assignment") if all fields are assigned, and in this case the compiler is seeing "all zero of zero fields have been assigned: all good - this variable is assigned". But the compiler doesn't seem to know about the actual fields, so it is being misled into allowing something that is not technically valid.

You definitely shouldn't rely on this behaving nicely! Although in most cases .locals init should mean you don't actually get anything too horrible. However, there is currently some work in progress to allow people to suppress .locals init in some cases - I dread to think what could happen in that scenario here - especially since Span<T> works much like a ref T - that could get very very dangerous if the field really isn't initialized to zero.

Interestingly, it might already be fixed: see this example on sharplab. Alternatively, maybe sharplab is using a concrete target framework, rather than reference assemblies.

Edit: very oddly, if I load the reference assembly into ildasm or reflector, I can see:

.field private initonly object _dummy

which is the spoofed field in the reference assembly that is meant to stop this from happening, but... it looks like it isn't working very reliably right now!

Update: apparently the difference here is a subtle but known compiler issue that remains for compatibility reasons; definite assignment of structs considers private fields of types that are known locally, but does not consider private reference-type fields of types in external assemblies.

Marc Gravell
  • 1,026,079
  • 266
  • 2,566
  • 2,900
  • It's even more subtle than that: it does not consider private fields *of reference types* that are in a different assembly, but does consider private fields of value types! – Nick Guerrera Apr 05 '18 at 00:37
16

Marc has a great answer. I wanted to elaborate a bit on the history / context.

First of all this is definitely a compiler bug. By rules of definite assignment this local is not definitely assigned and any usage should be an error. Unfortunately this bug is hard to fix for number of reasons:

  • This bug is old and goes back to at least C# 4.0. That gives customers 7+ years to inadvertently take a dependency on it
  • There are a number of structs in the BCL which have this basic structure. For example CancellationToken.

Those taken together mean fixing this would likely break a large amount of existing code. Despite this the C# team attempted to fix the bug in C# 6.0 when the bug was much younger. But an attempt at compiling the Visual Studio source with this fix showed that the fears around customers taking a dependency on this bug were well founded: there were a number of build breaks. Enough to convince us it would have a negative impact on a significant amount of code. Hence the fix was undone.

The second problem here is this bug wasn't known to all the compiler team members (before today at least). Been ~3 years since the fix was undone and had a bit of turn over since then. The team members who verified how we were generating reference assemblies for Span<T> weren't aware of this bug and recommended the current design based on the language spec. I'm one of those developers :(

Still discussing this but most likely we're going to update the reference assembly strategy for Span<T>, and other types, so that it avoids this compiler bug.

Thanks for reporting this. Sorry about the confusion caused :(

JaredPar
  • 733,204
  • 149
  • 1,241
  • 1,454
  • 1
    That's great context, thanks. I understand the complexities of changing compiler behaviour *even to fix a known bug*, but ... on this occasion it feels that an argument could be made for *at least* making it a compiler *warning*. I know that even that (adding a warning) has problems, and you've probably had that same discussion internally about 8 times. Fascinating issue, though! – Marc Gravell Apr 05 '18 at 08:38
  • Possibly naive question: is there any harm just adding a `private readonly int _evenDummier;` alongside the `private readonly object _dummy;` in the reference assembly, if that would cause the compiler to spot it? although I guess that would cause the exact same hard breaks as changing the compiler to respect ref-type private fields. Maybe in *new* reference assemblies like System.Memory? – Marc Gravell Apr 05 '18 at 08:41
  • 1
    Is this intentional bug also active when using `strict`? – Tragetaschen Aug 02 '18 at 09:26
2

More or less this is by design, since it depends heavily if the underlying struct holds any fields itself.

This code compiles for example:

public struct MySpan<T>
{
    public int Length => 1;
}

static class Program
{
    static void Main(string[] args)
    {
        MySpan<char> s1;
        var l1 = s1.Length;
    }
}

But this code doesn't:

public struct MySpan<T>
{
    public int Length { get; }
}

static class Program
{
    static void Main(string[] args)
    {
        MySpan<char> s1;
        var l1 = s1.Length;
    }
}

It seems that in that case, the struct is defaulted, and that is why it doesn't complain about a missing assignment. That it doesn't detect any fields is a bug, as explained in Marc's answer.

Patrick Hofman
  • 153,850
  • 22
  • 249
  • 325