Roslyn: Nullable analysis should understand XOR null tests

Created on 2 Jul 2019  路  6Comments  路  Source: dotnet/roslyn

Version Used: 3.3.0-beta1-19327-04

Steps to Reproduce:

```c#
string M1(object? o1, object? o2)
{
if (o1 == null && o2 == null)
return "both null";
if ((o1 == null) ^ (o2 == null))
return "one null";

// at this point neither o1 nor o2 are null

return o1.ToString()  // CS8602 possible null dereference
     + o2.ToString(); // CS8602 possible null dereference

}

string M2(object? o1, object? o2)
{
if (o1 == null && o2 == null)
return "both null";
if (o1 == null)
return "o1 null";
if (o2 == null)
return "o2 null";

return o1.ToString()  // no diagnostics in this case
     + o2.ToString();

}
```

Expected Behavior:

No diagnostics.

Actual Behavior:

Diagnostics.

Area-Compilers Area-Language Design Feature Request New Language Feature - Nullable Reference Types

Most helpful comment

To provide a little background, the compiler does what is usually referred to as a "path-independent flow analysis" for these situations. What this means practically is that conditional states are intersected at the end of the condition.

So

```C#
if (o1 == null && o2 == null) // cond 1
return "both null";
if ((o1 == null) ^ (o2 == null)) // cond 2
return "one null";

Is processed like

```C#
if (o1 == null &&
     ^ split o1 is null and not null
   1) o1 is null
        o2 == null
         ^ split o2 is null and not null
           1) o2 is null
                  return
           2) goto cond 2
   2) goto cond 2
// cond 2
state = (o1 NotNull && o2 MaybeNull) /\ (o1 MaybeNull && o2 NotNull) = o1 MaybeNull && o2 MaybeNull

So because the states are intersected at cond 2, and in each state there is an o1 is MaybeNull and an o2 is MaybeNull, the resulting state is o1 is MaybeNull and o2 is MaybeNull. In other words, the condition doesn't actually tell the flow analysis anything new about the states of the variables.

One way to learn about the states would be to do what's called a "path-dependent" flow analysis, but that can be significantly more expensive and is not in the design of C# 8.

All 6 comments

Use != instead of ^? That's arguably more idiomatic for bools anyway. Although || has the same effect in your sample.

Thanks. Several workarounds exist. This issue is tracking a gap in the product. It's not uncommon to see bitwise operators used on bools.

Our nullable analysis just doesn't learn anything in the WhenFalse case of o1 == null && o2 == null. I don't think it's realistic to start learning the "dependent" null relationship between variables (i.e. "at least one" of these variables was non-null, and "not exactly one" of these variables was non-null, therefore they're both non-null.)

I feel that in a scenario like this we should ask the user to change ^ to ||.

To provide a little background, the compiler does what is usually referred to as a "path-independent flow analysis" for these situations. What this means practically is that conditional states are intersected at the end of the condition.

So

```C#
if (o1 == null && o2 == null) // cond 1
return "both null";
if ((o1 == null) ^ (o2 == null)) // cond 2
return "one null";

Is processed like

```C#
if (o1 == null &&
     ^ split o1 is null and not null
   1) o1 is null
        o2 == null
         ^ split o2 is null and not null
           1) o2 is null
                  return
           2) goto cond 2
   2) goto cond 2
// cond 2
state = (o1 NotNull && o2 MaybeNull) /\ (o1 MaybeNull && o2 NotNull) = o1 MaybeNull && o2 MaybeNull

So because the states are intersected at cond 2, and in each state there is an o1 is MaybeNull and an o2 is MaybeNull, the resulting state is o1 is MaybeNull and o2 is MaybeNull. In other words, the condition doesn't actually tell the flow analysis anything new about the states of the variables.

One way to learn about the states would be to do what's called a "path-dependent" flow analysis, but that can be significantly more expensive and is not in the design of C# 8.

Closing as out of scope of the design.

Duplicate of https://github.com/dotnet/roslyn/issues/37344 (nullability analysis for single and-operator)

Was this page helpful?
0 / 5 - 0 ratings