Variance testing: language/variance/variance_downwards_inference_test needs updates

[eernstg]

Said test needs to have some null-safety migration changes applied: Line 13 and line 25, parameter type Object should be changed to Object?.

Also, the following declarations give rise to inference failures:

  B<int> b = new B(<num>[])..x=2.2;
  ...
  D<int> d = new D(3, (num x) {})..x=2.2;

This seems legitimate, because the preference given to the context type makes the instance creation new B<int>(...), where an argument of type List<num> is not accepted. Similarly for D. So the test needs to be updated in order to test the intended properties with code that has no errors. Perhaps:

  var b = new B(<num>[])..x=2.2;
  B<int> bAssignable = b;
  ...
  var d = new D(3, (num x) {})..x=2.2;
  D<int> dAssignable = d;

[munificent]

This seems legitimate, because the preference given to the context type makes the instance creation new B<int>(...), where an argument of type List<num> is not accepted.

When I first migrated the test, I assumed that the intent was specifically to test how inference behaved in that downwards context, given the name of the test.

I feel like I'm flying blind here. Is this the spec I should be using as a reference? Is there any documentation of how variance interacts with inference?

[leafpetersen]

@munificent we don't have a good declaration site variance only spec, I should split one out.

@eernstg The variance feature as implemented intentionally does not prefer the context type when doing inference for soundly variant classes (just as we do not prefer the context type when doing inference for function types).

[leafpetersen]

@munificent, @eernstg feel free to send any of these over to me to update if it's not clear what the intent of the test is.

[eernstg]

@leafpetersen wrote:

The variance feature as implemented intentionally does not prefer the context type

Right, I remember that we discussed that as an option, didn't know that the implementation actually did it.

we don't have a good declaration site variance only spec, I should split one out.

I could create a proposal based on removing the use-site mechanisms from dart-lang/language#557, file accepted/future-releases/variance/feature-specification.md.

[munificent]

The CL I have out is: https://dart-review.googlesource.com/c/sdk/+/162961. I do find it pretty hard to tell whether these tests are correct or not because the specification of variance, inference, and context types are kind of spread around. Maybe I'm not the best person to fix these tests?

[leafpetersen]

@munificent if you want to hand this off to me, that's fine, just let me know where things are and I'll pick up.

[munificent]

I landed this one and I'm working with Erik on the other one that needs test changes. I believe the remaining ones have actual implementation bugs but if they need some test work I'll let you know if I get stuck.