Why do I have to place () around null-conditional expression to use the correct method overload?

Question

I have these extension methods and enum type:

public static bool IsOneOf<T>(this T thing, params T[] things)
{
    return things.Contains(thing);
}

public static bool IsOneOf<T>(this T? thing, params T[] things) where T : struct
{
    return thing.HasValue && things.Contains(thing.Value);
}

public enum Color { Red, Green, Blue }

The first if below compiles; the second does not:

 if ((x.Y?.Color).IsOneOf(Color.Red, Color.Green))
 ;
 if (x.Y?.Color.IsOneOf(Color.Red, Color.Green))
 ;

They only vary by the extra set of parentheses. Why do I have to do this?

At first I suspected it was doing a double implicit cast from bool? to bool and then back to bool?, but when I remove the first extension method, it complains there is no implicit cast from bool to bool?. I then checked the IL and there were no casts. Decompiling back to C# yields something that looks like:

if (!(y != null ? new Color?(y.Color) : new Color?()).IsOneOf<Color>(new Color[2]
{
    Color.Red,
    Color.Green
}));

which is fine for the version of the CLR I'm running, and what I expect. What I didn't expect is that x.Y?.Color.IsOneOf(Color.Red, Color.Green) doesn't compile.

What is going on? Is it simply the way the language was implemented that it requires the ()?

Update

Here's a screen cap showing the error in context. This is getting even more confusing to me. The error actually makes sense; but what doesn't (in my mind now) is why line 18 wouldn't have the same problem.

Futuristic

This is surprising to me; you might have found a bug. What I find most disturbing here is that overload resolution is choosing the second method for the case with parens, and the first method for the case without parens. It's not clear to me why parens should make the difference there. – Profane

All that said, I would push back on your extension methods here. Extension methods that extend all types are frequently confusing. The more usual thing to do is to extend IEnumerable<T> and do it the other way: var colors = new[] { Color.Red, Color.Green }; if (colors.Contains(whatever)) ... – Profane

Also, I note that if you do want to extend all T, it is legal to compare against null. public static bool IsOneOf<T>(this T thing, params T[] things) { return thing != null && things.Contains(thing); } is legal. – Profane

Fair enough on the methods. I'm going to show the minimum possible programming highlighting the error. – Futuristic

Isn't this what the ?. operator is meant to do, it will short-circuit the expression when it encounters a null value? Using () around it will not allow the IsOneOf method to be short-circuited, therefore guaranteeing the expression to be evaluated as a bool. – Lecture

Answer 1

First of all, this behaviour looks intentional to me. It's rare that somebody adds extension methods to nullable types, and it's much common that people mix null-conditional and regular member accesses in one expression, so the language is favoring the latter.

Consider the following examples:

class B { bool c; }
class A { B b; }
...

A a;
var output = a?.b.c; // infers bool?, throws NPE if (a != null && a.b == null)
// roughly translates to
// var output = (a == null) ? null : a.b.c;

while

A a;
var output = (a?.b).c; // infers bool, throws NPE if (a == null || a.b == null)
// roughly translates to
// var output = ((a == null) ? null : a.b).c;

and then there is

A a;
var output = a?.b?.c; // infers bool?, *cannot* throw NPE
// roughly translates to
// var output = (a == null) ? null : (a.b == null) ? null : a.b.c;

// and this is almost the same as
// var output = (a?.b)?.c; // infers bool?, cannot throw NPE
// Only that the second `?.` is forced to evaluate every time.

The design goal here seems to be aiding the distinction between a?.b.c and a?.b?.c. If a is null, we expect getting an NPE in none of the cases. Why? Because there is a null-conditional directly after a. So the .c part must be only evaluated if a was not null, making the member access dependent of the previous null-conditionals outcome. By adding explicit brackets, (a?.b).c we enforce the compiler to try to access .c of (a?.b) regardless of a being null, preventing it to "short circuit" the whole expression to null. (using @JamesBuck -s words)

In your case, x.Y?.Color.IsOneOf(Color.Red, Color.Green) is like a?.b.c. It will call the function with the signature bool IsOneOf(Color red) (so the overload where the param is not nullable, and I stripped the generic part) only when x.Y was not null, thus wrapping the expression's type in Nullable to handle the case when x.Y is null. And because the while evaluates to bool? instead of bool, it cannot be used as test in an if statement.

Hail

Answer 2

The meaning of x.Y?.Color.IsOneOf(Color.Red, Color.Green) is fairly independent of your method overloads. ?.'s pseudo-operands are x.Y and Color.IsOneOf(Color.Red, Color.Green), even though the latter by itself is not a valid expression. First, x.Y is evaluated. Call the result tmp. If tmp == null then the whole expression is null. If tmp != null, then the whole expression evaluates to tmp.Color.IsOneOf(Color.Red, Color.Green).

When IsOneOf is an instance method, that's pretty much always the behaviour you'd want, which is why it's that behaviour that ended up getting into the specification and into the compiler.

When IsOneOf is an extension method, as it is in your example, then it may not be the behaviour you'd want, but the behaviour is the same, for consistency and since there's a readily available workaround that you've already found.

The associativity and why the current result was picked are explained in a thread on the Roslyn CodePlex site.

Emblements

It sounds like you're saying that ?. and . do not have the same precedence. I was not around for the design of the feature, but that would surprise me; I wonder what motivated that decision? I should check the design notes. – Profane

@EricLippert I think they do have the same precedence, but they're right-associative. If it had been specified any other way, there wouldn't be any good way to write what we can now write as a?.b.c, where there isn't and shouldn't be a null check on the value of b. – Emblements

@EricLippert Found the relevant link, added it to my answer. – Emblements

All right, I know what my blog article after I finish my current series is going to be. This is a nasty gotcha! – Profane

Answer 3

Ok, I figured it out, but I'm going with Tamas' answer. I think mine has value here, though, as a thinking aid when working with null-conditionals.

The answer was kind of staring at me in the screen capture in my question

I was tunnel visioning on the Color/Color? conversion, but the actual issue is that bool? can't be implicitly cast to bool.

In a long "dotted" expression containing the ?. operator, you have to think in terms of the thing, and its type, after the rightmost dot. That thing, in this case, is the extension method that returns a bool, but the use of ?. effectively "changes" it to a bool?. In other words think of the rightmost thing as being a reference type that could be null or a Nullable<T>.

Wow, this one had me going.

Futuristic

Answer 4

If you place the following line you'll see that .net is creating a dynamic type System.Nullable<UserQuery+Color> for this null conditional expression inside the parentheses.

Console.WriteLine((x.Y?.Color).GetType().ToString());

If you don't put the parentheses it probably tries to evaluate as a normal expression, thus the error.

Celik

I'm not sure how you got that. I tried Console.WriteLine(x.Y?.Color.GetType()) and Console.WriteLine((x.Y?.Color).GetType()) and got ConsoleApplication1.Color for both (ConsoleApplication1 is the namespace where all these types live in my test project). – Futuristic