CS8604 Null reference false positive with two nullable objects

[LeeReid1]

Version Used: VS Version 16.11.4 .NET 5.0

Steps to Reproduce:

Sorry if I've missed something basic here but it seems there's a null reference warning/error with some fairly basic logic when two variables are involved:

        public void Foo(string? m1, string? m2)
        {
            if((m1 == null) == (m2 == null))
            {
                // Either both null or neither are null
                return;
            }

            if(m1 == null)
            {
                // m2 cannot be null because m1 is null
                NeedsNonNull(m2); // CS8604 that should not be flagged
            }
        }

        public void NeedsNonNull(string m)
        {
            // do stuff...
        }

EDIT: similar one-liner

        public void Foo(string? m1, string? m2)
        {
            if((m1 == null) && ((m1 != null) || (m2 != null)))
            {
                // If m1 == null the above becomes
                // TRUE && (FALSE || m2!=null) 
                // which simplifies to 
                // TRUE && (m2!=null)
                // So this block should only execute if m1 == null and m2 != null

                NeedsNonNull(m2); // CS8604 that should not be flagged
            }
        }

        public void NeedsNonNull(string m)
        {
            // do stuff...
        }

Expected Behavior:

No CS8604 warning

Actual Behavior:

CS8604 warning. See code.

[svick]

I believe the compiler intentionally does not perform this kind of deductive reasoning about nullability of variables, so this behavior is by-design. To get rid of the warning, you can use the null-forgiving operator, !.

[LeeReid1]

Hi Svick,

Do you mean 'by-design' or 'not-yet-implemented'? I understand to some degree if the compiler simply isn't advanced enough yet, but I don't understand why one would explicitly choose to have false positive warning/errors in such simple scenarios?

Presumably the CPU overhead for this kind of checking is substantially less that the developer's overhead of second guessing the compiler, particularly if they have to check for downstream ! whenever they make changes to preceding code. I currently have a code base riddled with warnings like this

[CyrusNajmabadi]

why one would explicitly choose to have false positive

Because the space of analysis we could perform is effectively infinite, while our time and resources are not. So we make decisions on how far to go for any particular release. Otherwise we'd be spending all our time in every release on just this problem space :-)

[RikkiGibson]

https://github.com/dotnet/roslyn/issues/36927#issuecomment-508595947

[LeeReid1]

Thanks everyone for getting back to me and clarifying that it's a matter of resources and the analysis approach rather than this kind of behaviour being the 'end goal'.

If anyone is skimming through this in the future and doesn't want to read through the link above, the issue goes deeper than the nullability checks - it's how the compiler is analysing the code at a deeper level. For example, the following compiles:

static void MethodA()
{
    bool booleanVal = true;

    string b;
    if (booleanVal)
    {
        b = "has value";
    }
    else
    {
        return;
    }
    if (booleanVal)
    {
        Console.WriteLine(b);
    }
}

As does this:

static void MethodB()
{
    const bool booleanVal = true;

    string b;
    if (booleanVal)
    {
        b = "has value";
    }
    if (booleanVal)
    {
        Console.WriteLine(b);
    }
}

But this does not:

static void MethodC()
{
    bool booleanVal = true;

    string b;
    if (booleanVal)
    {
        b = "has value";
    }
    if (booleanVal)
    {
        Console.WriteLine(b); // Error CS0165 - b is considered unassigned
    }
}

even though were the code truly optimised the method would essentially inline to:

Console.WriteLine("has value");

Fingers crossed that at some stage the team have the time/opportunity to consider these flow-related issues and things like compile-time nullability and assignment checks improve together.

Thanks again.