Extend definite assignment to understand null and nullable values and ?.

[IEvangelist]

I am currently using the following IDE:

Microsoft Visual Studio Enterprise 2017 
Version 15.3.4
VisualStudio.15.Release/15.3.4+26730.15
Microsoft .NET Framework
Version 4.7.02046

Installed Version: Enterprise

I'm have a simple .NET Core 2.0 application that I have built targeting C# 7.1 from the Build > Advanced > Language Version selection. When I attempt to compile the following program, I get an unexpected compiler error.

using System.Collections.Concurrent;

namespace IEvangelist.Csharp.Seven
{
    class Program
    {
        public static void Main() { }
    }

    public static class ConcurrentExtensions
    {
        public static T TryPeekOrDefault<T>(this ConcurrentQueue<T> queue)
            => (queue?.TryPeek(out T result) ?? false) ? result : default;
    }
}

Error reported:

Severity Code Description Project File Line Suppression State
Error CS0165 Use of unassigned local variable 'result' IEvangelist.Csharp.Seven C:\Users\david.pine\Documents\Visual Studio 2017\Projects\IEvangelist.Csharp.Severn\IEvangelist.Csharp.Severn\Program.cs 33 Active

Expected Behavior

No compiler error. The compiler has all the details it needs to know that this is valid. The true path of the ternary is what the IDE is complaining about; however, I think it shouldn't.

[HaloFour]

The error is correct, the ?. operator short-circuits, so if queue is null then result will never be assigned.

[MkazemAkhgary]

Compiler does not know, if expression returns true, result will be assigned, it can not make assumptions. hence the error you get. I see. you are expecting more intelligence! :-)

you can fix this by

queue == null ? default(T) : queue.TryPeek(out T result) ? result : default(T);

I don't think this can be simplified further in one line.

[MkazemAkhgary]

@HaloFour I guess some of new c#7 does not work very well. this one is good find. you can not use out T value in combination with .? operator. if you do, content of value will be completely unusable unless you assign something else to it in which case you loose other content.

similar issue where you can not use contents of local variable..

switch(obj) 
{     
    case A a:     
    case B b:     
      // impossible to use content of a or b
     break; 
}

There are similar issues too not just with c#7, its just matter of writing right syntax for it.

how ever maybe they could introduce ?= operator, this will be conditional assignment and happens if value is not assigned. then in current post user could do

=> queue?.TryPeek(out T result) == null ? default(T) : result ?= default(T);

just for fun, oh does this require changes in CLR? :-)

[HaloFour]

@MkazemAkhgary

C#7 has nothing to do with it. If the variable was declared in a prior statement (but not assigned) the result would be exactly the same. And that behavior is correct. If queue is null then TryPeek is never called and result is never passed as an out parameter which is the only path to which it is assigned a value. The compiler does not want you to use a variable if it is not definitely assigned by all paths through which that code could flow. That's been the case since C# 1.0.

As the behavior is correct there is nothing to correct here. But, for fun, no, changing that would not require CLR changes. The CLR only has a flat method scope for variables and no concept of definite assignment. Those are strictly C# features.

[svick]

@HaloFour If queue is null, then the condition is false and so the result variable is not accessed. So the variable is assigned whenever it's accessed, even if definite assignment rules from the spec don't allow it to be accessed.

So, there is indeed nothing to correct, but I think there is something to improve. (Whether it's worth improving is another question entirely.)

[HaloFour]

@svick

Maybe. That starts to wade into mixing definite assignment with flow analysis. Even though it all looks like a single expression it expands out to numerous statements and the compiler requires that both operands meet definite assignment since it can't base that assignment on whether the bool expression is true or false. I'm certainly not opposed to such flow analysis, though.

[IEvangelist]

Thank you all for the replies, clarifications and thoughts... much appreciated. I agree with @svick after revisiting this again. I do however see it as an opportunity for improvement. Flow analysis (outside of definite assignment) is starting to make its way into proposals and prototypes for C# 8. I'd love to see this improved as part of that.

[gafter]

@IEvangelist You say you'd like to see this "improved". What would you like to be better? Is the error message unclear?

[gafter]

Oh, I see you'd like definite assignment (which is part of flow analysis) to infer that the variable is only referenced where it was definitely assigned in this case. A change to accomplish that would require a change to the definite assignment spec at https://github.com/dotnet/csharplang/blob/master/spec/variables.md#definite-assignment .

[IEvangelist]

Has there been any internal discussion on this issue?

[TheFlow0360]

Encountered this "problem" today, and I'd really love if definite assignment could be extended to work in this case. In my case I have to preassign my out value which is an enum. Using default(EnumType) at least doesn't look as bad as randomly assigning one value, but it's still not pretty.

static void Main(string[] args)
{
    IDictionary<String, MyType> dictionary = null;
   
   ... // code potentially assigning dictionary        

    var x = default(MyType);           // <-- this is not pretty
    var mappedValue = dictionary?.TryGetValue("abc", out x) ?? false ? Map(x) : OtherType.None;
}

enum MyType  // this enum doesn't really have any default value
{
    Whatever,
    DontCare,
    Irrelevant
}

[RikkiGibson]

A spec proposal which addresses many of these scenarios has been submitted at #4240.