Roslyn: Bad codegen in call-forward to local function in different switch block

[agocke]

From #15322:

class Program
{
    static void Main(string[] args)
    {
        object o = null;
        switch (o)
        {
            case string x:
                Assign();
                Foo();
                break;
            case int x:
                void Assign() { x = 5; }
                void Foo() => System.Console.WriteLine(x);
                break;
        }
    }
}

The problem seems to be getting the information for the local in CodeGen. It's not clear if this is an issue

[HaloFour]

Should such code even be legal? I understand that the scope of the case bodies themselves are flattened and the local function would be in scope, but it doesn't seem like it should be permitted to invoke that local function from a different case scope referencing any introduced pattern or out variables due to those variables not being in scope nor assigned.

[alrz]

Looks like Duff's device. </sarcasm>

[agocke]

@HaloFour My read of this code is that the local functions are in scope and it captures the int x which is in scope of those local functions. However, that variable is not definitely assigned, so it must first be assigned before it is otherwise used.

If this would be an error, I'm not sure what error it would be.

/cc @gafter since this also involves scoping of pattern variables. Do you agree with my assessment here?

[HaloFour]

@agocke

The local function would be in scope since switch doesn't introduce any scope of its own, but since case does this is a unique circumstance where you'd be able to define a local function that could enclose a variable where the variable would go out of scope but the function would not. The only time int x can ever be assigned is if that case is matched. Attempting to call that function anywhere else could only ever be a compiler error (CS0165), if only because x is not definitely assigned.

[TessenR]

@HaloFour Some types are always initialized. E.g. it isn't possible for a variable of type 'S' to be uninitialized. What error it would be here?

struct S
{
    public void M(object o)
    {
        switch(o)
        {
            case string s:
                Local();
                break;
            case S s:
                void Local() => System.Console.WriteLine(s);
                break;
        }
    }
}

I don't like the crazy rules about local functions, too. But tbh I don't see how it is different from this case:

class C
{
  void M()
  {
    Assign();
    Local();
    int x;
    void Local() => System.Console.WriteLine(x);
    void Assign() { x = 5; }
  }
}

Shouldn't 'out of scope' variables behave exactly like 'not yet declared' variables? What's the difference?

[HaloFour]

@TessenR

What error it would be here?

I imagine it would be the same error. It doesn't matter that at the point of s being in scope that it's definitely assigned. The point at which s would come into scope never happens, thus it is also not assigned.

I don't like the crazy rules about local functions, too.

It's not really local functions that are the issue here. Hoisting has some pretty simple and understandable rules and is kept innocuous by the fact that you can only declare the hoisted function within the same scopes that it can enclose. The hoisted function can never have a wider scope than those enclosed variables, so flow analysis can do a good job of ensuring that those variables are assigned at the point of invocation.

The problem here is switch. This is the first time that I'm aware (in any C-like language) where you can declare identifiers into the parent scope while still referencing identifiers in a narrower scope. This isn't a big deal with normal identifiers like variables since referencing them in the wider scope is relatively innocuous. But with functions and hoisting now you're enclosing over a scope that is possible to never get entered in the first place. If you look at scope as simply a compiler trick to "hide"/reuse local slots (which it is) then you have to consider S s to have been declared initially in the wider scope but never assigned until that case matches.

[agocke]

However, there are currently no rules prohibiting _assigning_ an unassigned variable. I think we would have to either come up with a new type of rule for this situation, or change scoping so this is not possible.

[HaloFour]

@agocke

This is true, and I guess there's technically no harm in enclosing and reusing the mutable pattern variable out of its scope. It's incredibly nasty, though, and I can't imagine a legitimate use case.

[jcouv]

Tracked by VSO https://devdiv.visualstudio.com/DevDiv/_workitems?id=369824