Suppression on default parameter values for 'null' and 'default' has different effect on type inference

[TessenR]

Version Used:

Branch main (30 May 2021)
Latest commit 6580861 by dotnet-maestro[bot]:
Update dependencies from https://github.com/dotnet/arcade build 20210528.1 (#53780)

[main] Update dependencies from dotnet/arcade

Steps to Reproduce:

#nullable enable
class C<T> where T : class, new()
{
  void M1(T t)
  {
    t = new();
    var res1 = Infer1(t);
    res1.ToString(); // warning (correct)

    var res2 = Infer2(t);
    res1.ToString(); // no warning (correct); suppression forces 'T!' inference

    var res3 = Infer3(t);
    res3.ToString(); // warning (correct)

    var res4 = Infer4(t);
    res4.ToString(); // warning (incorrect); suppression has no effect
      
    var res5 = Infer4(t, null!);
    res5.ToString(); // no warning (correct); the call is equivalent to the previous one
  }

  public T Infer1<T>(T t1, T t2 = default) where T: class? => t1;
  public T Infer2<T>(T t1, T t2 = default!) where T: class? => t1;
  public T Infer3<T>(T t1, T t2 = null) where T: class? => t1;
  public T Infer4<T>(T t1, T t2 = null!) where T: class? => t1;
}

Expected Behavior:
Either one of the following:

• CS8602: Dereference of a possibly null reference. warnings for res1, res2, res3, res4
• warnings for res1, res3 only since other variables are initialized with calls with suppressed default parameter values

Actual Behavior:
CS8602: Dereference of a possibly null reference. warnings are reported for res1, res3, res4

Notes
Right now Roslyn takes default parameter value suppressions into account only for default and default(T) values but not for null values.
This leads to a situation where semantically equivalent code is processed differently (there's no difference between Infer2 and Infer4 both assign null value to the t2 parameter and both suppress the warning but infer different type arguments.
I believe they should behave the same.

As a side note I think that the current behavior where the default parameter values' suppressions affect type inference might not be desired since these suppressions are not expressed in any way in metadata. Therefore the actual type inference result will depend on whether the method you are using is declared in source code or in a referenced assembly. It looks like it'd be better not to take default value suppressions into account at all and report warnings for all variables but res5

Might also be related to #43536

[RikkiGibson]

It looks like the sample used res1 in a context where it was supposed to use res2. When that is fixed, the warnings are actually consistent between default and null. I expected that to be the case because when we synthesize a default argument at a call site, we only look at the ConstantValue associated with the parameter, not the actual expression used for the default argument. Because of this, at the call site a default or a null parameter default value ends up being treated the same, and the ! suppression is dropped/ignored.

The closest bug I am aware of to this bug is #50311. My suggestion to fix both of these scenarios is to make it so default arguments do not "contribute" to nullable type argument reinference. The assumption being that such default arguments could only have the effect of making the type argument "more nullable" than it would have been otherwise, and that it would only do so in cases where the non-suppressed parameter default value would produce a warning in the declaration.

[TessenR]

Ah, sorry!
I tested various combinations of suppressions and default values and didn't notice that I forgot to rename one of the usages. It works as expected indeed. Closing the issue