dart-lang / language

Design of the Dart language
Other
2.66k stars 204 forks source link

Nullable variable not consistently promoted #1307

Open lrhn opened 3 years ago

lrhn commented 3 years ago

Example:

foo<E>(E? value) {
  if (value is! E) {
    value = value as E;
  }
  // value is promoted to E here.
  ...
}

but

foo<E>(E? value) {
  if (value == null) {
    value = value as E;
    // or: value as E
  }
  // value is *not* promoted to E here.
  ...
}

As a coder I'd expect value == null and value is! E to be largely equivalent for this purpose. I know they differ if E ends up as a nullable type, but that both cases, assigning an E to value on the true branch should ensure the result is E.

Is this a bug, or a mis-feature, or a deliberate design?

leafpetersen commented 3 years ago

This is unfortunately WAI. See here for discussion and explanation. We could choose to try to fix this in beta, it does seem to come up now and then. cc @stereotype441

lrhn commented 3 years ago

Makes sense. The test value != null would promote value to NON_NULL(E?) which is E & Object and make that type, and its nullable dual, types of interest. The later assignment to E would only retain promotion if E is a type-of-interest, but it's not. I'd have to cast the value to E & Object instead of E, which is obviously impossible.

An alternative could be that if we want to make E&Object types of interest, we make E a type of interest too, the intersection-erased type which is what variables would have to be typed as anyway. (And maybe Object too? Not sure if that makes sense if we end up with E & Foo and want to consider Foo a type of interest.)

lrhn commented 3 years ago

Is this still working as intended? If I try:

void foo<E>(E? value, E sample) {
  if (value != null) {
    value = sample;
  } else {
    value = sample;
  }
  E x = value;
}

in Dartpad, I get an error that value can't be assigned to x because it's E?.

If I change the test from value != null to value is E, the code is accepted!

That means it's possible to unify promotions happening on both branches, but it only works for a value is E check.

Both assignments should promote because the type E is of interest for the variable value of type E?, whether I check it or not (and definitely whether I check it using is E or != null).

stereotype441 commented 3 years ago

@lrhn I think it's behaving as designed. What's happening is that the only type of interest for value is considered to be the result of promoting its declared type (E?) to non-nullable, and that type is E & Object. Since sample has type E, which is not a subtype of E & Object, the assignment value = sample demotes the type back to E?. If I change the declaration to foo<E extends Object> the error goes away, because it makes E and E & Object equivalent types.

The reason that it works if you change value != null to value is E is that testing value is E makes E a type of interest.

Not sure if this is a good design, though. Arguably, making E & Object a type of interest is not too useful to the user, because that's not even a type they can name. Perhaps we should change the definition of "types of interest" so that rather than promoting to non-nullable to get the type of interest, we strip away ?s.

lrhn commented 3 years ago

I definitely thought that for assigning to value, of type E?, E would be the type of interest, not E & Object. For a type variable, those two types are different, and it might be worth considering both to be of interest.

As a programmer, I don't think about intersection types unless forced to, it's not a type I can visualize, so I didn't even consider it relevant here. I'd expect most programmers to be oblivious about intersection types.

stereotype441 commented 3 years ago

@lrhn agreed, this is not a great user experience. I've added the "flow-analysis" label so that we remember to consider this next time we are thinking about flow analysis improvements.