Roslyn: pattern match fails to produce a useful local variable

[gafter]

@gafter ported this from a customer-reported issue on Microsoft-internal URL https://vsfeedback/comment/736281 and also https://devdiv.visualstudio.com/0bdbc590-a062-4c3f-b0f6-9383f67865ee/_workitems?id=366601

As it stands at the moment, although a pattern match expression can be used in a non-conditional statement, it doesn't produce a local variable whose value can be accessed. That makes some sense, but only just.

In the success1 case, I hope it is just that the code can't currently be analysed well enough to allow it to work.

I'm calling the success2 case an outright bug, because the success3 case works.

    var t = ...;
    var x0 = t as SomeType;
    var success0 = x0 != null;
    var success1 = t is SomeType x1;
    if (success1)
        M(x1); // CS0165 Use of unassigned local variable 'x1'
    bool success2;
    if (success2 = t is SomeType x2)
        M(x2); // CS0165 Use of unassigned local variable 'x2'
    bool success3;
    if (t is SomeType x3)
    {
        success3 = true;
        M(x3); // ok
    }
    else
    {
        success3 = false;
        x3 = null;
    }
    Console.WriteLine(success0 == success3);
    Console.WriteLine(x0 == x3);

[gafter]

Unfortunately, the definite assignment rules of the language do not "see through" the assignment expression. That is, an assignment expression never results in a variable being "definitely assigned when true" or "definitely assigned when false". That explains the behavior of this similar example using previous language features:

using System;

class Program
{
    static void Main(string[] args)
    {
        bool condition = true;
        {
            int x;
            if (condition && M(out x)) Console.WriteLine(x); // ok
        }
        {
            int x;
            bool success;
            if (success = condition && M(out x)) Console.WriteLine(x); // error
        }
        {
            int x;
            bool success = condition && M(out x);
            if (success) Console.WriteLine(x); // error
        }
    }
    static bool M(out int x)
    {
        x = 1;
        return true;
    }
}

[markhurd]

OK, so rather than a lack of analyser smarts or a bug in the implementation, it's just making more obvious an issue with the language itself.

[HaloFour]

Indeed, this kind of problem had been highlighted repeated. It was more important for pattern variables to remain consistent in scoping with out declarations than to make sense.

Perhaps with the type of flow analysis proposed for non-nullable reference tracking the compiler could be smarter about knowing when the pattern variables would be definitely assigned. Until then it'll just be a big wart on the language.

[markhurd]

To be fair, when adding the feature to an existing code base, I've only come across this issue in one place where it was not attempting to replicate x is T y as x.TryGetT(out y), in the source for TryGetT. But it does highlight that an out parameter should be able to be initialised by a pattern match clause, and the lack of being able to copy the boolean result means you need to say if (x is T y) result=true; else result=false; which always looks bad.

SO question that "started" this: http://stackoverflow.com/q/41569072/256431

[DavidArno]

@markhurd,

To be really fair, whilst you have helped @gafter uncover an existing bug/limitation in C# prior to v7, the way it all started out for you highlights how the language team really have opened a nasty can of worms here. All because they insisted on making the out var feature work the way they wanted, rather than the way C#'s scoping previously worked.

Your original code from SO is a case in point:
cs internal bool firstAsSymbol(out Symbol s) { var result = first is Symbol sym; s = sym; return result; }
This won't compile, as you say. Whilst the language now lets sym leak out from the first is Symbol sym expression, if first isn't of type Symbol, then sym won't be initialised. So the compiler won't let you use it, even though it permits it to be in scope. You won't be the last person to be tripped up by this nonsense.

[alrz]

It was more important for pattern variables to remain consistent in scoping with out declarations than to make sense.

And still I'm struggling to understand what this "consistency" is good for, when they are fundamentally different things. I wonder what will happen when declaration expressions (as an standalone feature) join the party.

[markhurd]

@alrz I conceded to Microsoft in a comment in the "is this program ready for release?" form that defaulting to setting the value to null may make the x is T v || getT(out var v) feature harder to allow in the specification, let alone implement.

[gafter]

@MadsTorgersen pointed out to me

This is what the spec says about definite assignment and simple assignment:

5.3.3.23 Simple assignment expressions

For an expression _expr_ of the form _w = expr-rhs_:

• The definite assignment state of v before _expr-rhs_ is the same as the definite assignment state of v before _expr_.
• If w is the same variable as v, then the definite assignment state of v after _expr_ is definitely assigned. Otherwise, if the assignment occurs within the instance constructor of a struct type, if w is a property access designating an automatically implemented property P on the instance being constructed and v is the hidden backing field of P, then the definite assignment state of v after _expr_ is definitely assigned. Otherwise, the definite assignment state of v after _expr_ is the same as the definite assignment state of v after _expr-rhs_.

To me it looks like this rule should provide the propagation of definite assignment state that these folks are asking for, and that the refusal of the compiler to do so is a bug. No?

[gafter]

I doubt that @MadsTorgersen interpretation is correct. It would lead to some counterintuitive results. See, for example, https://github.com/dotnet/csharplang/issues/271. Moving to the csharplang repo.

[gafter]

Issue moved to dotnet/csharplang #507 via ZenHub