Roslyn: Nullable warning on `default(T) == null`

[stephentoub]

Version Used:
3.1.0-beta1-19172-05+edd2de88fb3e84a097fb30b4070e0f219f624e40

Repro:
This program
```C#

nullable enable

using System;

public class Program
{
public static void Main() => Foo(new object());

private static void Foo<T>(T item)
{
    Console.WriteLine(default(T) == null);
    Console.WriteLine(item == null);
    Console.WriteLine(item.GetHashCode());
}

}
```
produces two warnings:

• default(T): "A default expression introduces a null value when 'T' is a non-nullable reference type. "
• item.GetHashCode(): "Possible dereference of a null reference."

Several questions:

• Why does default(T) == null warn, and why is that more warn-worthy than item == null? Is that a bug?
• If it's by design, is the appropriate action here to just ! away the warning?
• It's confusing to me that the warning on default(T) says T is a "non-nullable reference type", but then trying to dereference item, which is of type T, says it's a "possible dereference of a null reference". Are those messages correct?

cc: @gafter, @jaredpar, @dotnet/nullablefc

[gafter]

The warning is on default(T). If T ends up being a non-nullable reference type, this would introduce a null value of a type T which isn't supposed to have null values.

The diagnostic isn't telling you that T is a non-nullable reference type. It says "when T is a non-nullable reference type".

Having said that, this is an idiom for testing whether or not T is a reference type (or nullable value type?) and probably does not deserve a diagnostic.

[stephentoub]

Thanks for the explanation. So the difference from the item == null case is that this case ends up potentially introducing a null where as that case just tests for one.

[gafter]

Yes, that is exactly right. But I agree with you that we should be able to see that we are immediately testing and then discarding that value, so no harm arises.

[jnm2]

I'm seeing a lot of this and am excited to see that you're planning to remove the warning!

[jcouv]

This no longer repro's as of 16.5p3. I think this was fixed by Chuck's tracking of [MaybeNull]T state in an earlier 16.5 preview.
I'll go ahead and close. Thanks