C# null-conditional/null-coalescing operator precompiler bug or am I just missing something?

Question

I just pushed some bad code that caused an issue in release and wanted to see if anyone could help clarify what's going on here.

Below is a (contrived but runnable) program that illustrates the issue I'm having. The line in question is the if statement in the foreach loop of the Main method.

using System.Collections.Generic;
using System;

public class Program
{
    static void Main (string[] args)
    {
        string[] arr = new[] { "a","b","c"};
        string s = "y3";
        X x = new X();
        
        foreach(string a in arr)
        {
            if(x.Y?.Contains(a) ?? false || s == x.S)
            {
                Console.WriteLine(a);
            }
        }
    }
}

public class X
{
    public string S = "y3";
    public List<string> Y = new(){ "y1", "y2" };
}

In this code, we have an expression that uses a null-conditional operator alongside a null-coalescing operator to check whether the list Y in x (which could be null) contains a or another external condition is met. If either condition is met, it should execute the block--in this case, print to the console.

Running this code produces no print statements, which is counterintuitive to me. However, if I add parenthesis to the offending expression, for example, if((x.Y?.Contains(a) ?? false) || s == x.S), everything works as intended and items get printed to the console.

So I put this code into SharpLab (against the main (24 Jan 2023) baseline) to see what was going on and found some interesting results.

When the initial code is pre-compiled in its original form, the pre-compiler generates a ternary expression, effectively short-circuiting the || operator whenever Y is not null:

    [System.Runtime.CompilerServices.NullableContext(1)]
    private static void Main(string[] args)
    {
        string[] array = new string[3];
        array[0] = "a";
        array[1] = "b";
        array[2] = "c";
        string text = "y3";
        X x = new X();
        string[] array2 = array;
        int num = 0;
        while (num < array2.Length)
        {
            string text2 = array2[num];
            List<string> y = x.Y;
            if ((y != null) ? y.Contains(text2) : (text == x.S))
            {
                Console.WriteLine(text2);
            }
            num++;
        }
    }

When parentheses are added, the pre-compiler generates the following:

    [System.Runtime.CompilerServices.NullableContext(1)]
    private static void Main(string[] args)
    {
        string[] array = new string[3];
        array[0] = "a";
        array[1] = "b";
        array[2] = "c";
        string text = "y3";
        X x = new X();
        string[] array2 = array;
        int num = 0;
        while (num < array2.Length)
        {
            string text2 = array2[num];
            List<string> y = x.Y;
            if ((y != null && y.Contains(text2)) || text == x.S)
            {
                Console.WriteLine(text2);
            }
            num++;
        }
    }

In this version, the original intent is captured and the lines are printed as expected.

I read some things saying that the null-coalescing operator is right-associative and thought maybe it has something to do with that, but I'm not sure because I would assume that the logic would still work in that case.

I also thought that it could be different across compiler versions but haven't tried to track that down yet.

Just looking for opinions to see if there is something I don't understand or that I'm missing here or if it is just a gotcha I need to look out for in the future.

Answer 1

If you take a look at the C# operator precedence table, you will see why this is happening

Operators	Category or name
...
x && y	Conditional AND
x || y	Conditional OR
x ?? y	Null-coalescing operator
c ? t : f	Conditional operator

What this means is that || has a higher precedence than ??, so false is being bound to s == x.S. Effectively, you have this:

x.Y?.Contains(a) ?? (false || s == x.S)

Which is not your intention. This is why you should not rely on operator precedence except in the most obvious cases.

Aside, there is another syntax that works better

x.Y?.Contains(a) == true || s == x.S

In this case, it's fairly obvious to most that == has a higher precedence than ||, so I think the () could be forgone.