Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

??= fails to promote variable whose type is a nullable type parameter #3057

Open
stereotype441 opened this issue May 9, 2023 · 4 comments
Open
Labels
flow-analysis Discussions about possible future improvements to flow analysis

Comments

@stereotype441
Copy link
Member

The following code has a compile-time error at (2) ("A value of type 'T?' can't be returned from a function with return type 'T' because 'T?' is nullable and 'T' isn't."):

class C<T> {
  T f(T? t, T u) {
    t ??= u; // (1)
    return t; // (2)
  }
}

main() {}

It seems like the null-aware assignment at (1) should promote the type of t from T? to T, but apparently this isn't happening.

@eernstg
Copy link
Member

eernstg commented May 10, 2023

There was a substantial amount of discussion about a similar situation here: #3031. The two situations differ in that the expression v ??= e does not have a (non-trivial) context type here, whereas the context type was crucial in dart-lang/sdk#3031.

The compound assignment t ??= e where t can be various kinds of terms isn't specified in terms of desugaring, but let's consider the following desugaring which is compatible with the specified semantics (in particular, because evaluation of t does not have any side effects):

class C<T> {
  T f(T? t, T u) {
    t == null ? t = u : t;
    return t;
  }
}

main() {}

In the rewrite we still don't get the promotion, in spite of the fact that t = u should promote t to have type T, and t in the false branch of the conditional should have the type NonNull(T?) which should be T.

Hence, it's quite surprising that this version of the code also doesn't promote t (we still get the error at return t;): No matter which control flow path has been taken when return t; is reached, t should have type T.

Here is another variant where t is also not promoted to T:

class C<T> {
  T f(T? t, T u) {
    if (t == null) t = u;
    return t;
  }
}

main() {}

Is there something fishy about types of the form T? where T is a type variable, and potentially nullable, in a more general sense than just for ??=? And does this matter for dart-lang/sdk#3031 as well?

@lrhn
Copy link
Member

lrhn commented May 10, 2023

The promotion of t to NonNull(T?) = NonNull(T) should work.

So (I decided after checking that it wasn't my first guess) it's probably about unifying T and T & Object. The promotion chain of t is (T?, T & Object), so it cannot be demoted to T. When assigning T to it, it demotes all the way back to T?.

If I add T to the promotion chain, the problem goes away:

class C<T> {
  T f(T? t, T u) {
    t as T;
    if (t == null) t = u;
    return t;
  }
}

main() {}

@stereotype441
Copy link
Member Author

The promotion of t to NonNull(T?) = NonNull(T) should work.

So (I decided after checking that it wasn't my first guess) it's probably about unifying T and T & Object. The promotion chain of t is (T?, T & Object), so it cannot be demoted to T. When assigning T to it, it demotes all the way back to T?.

I dug into the implementation and confirmed that this is indeed what's happening.

I think we should consider, in future versions of Dart, extending the notion of "type of interest" so that for a variable of type T?, both T and T & Object will automatically be considered types of interest. That would have allowed t ??= u to automatically promote t from T? to T.

Since this would require a language change (which theoretically could break existing code), I'll move this issue to the language repo for further discussion.

@stereotype441 stereotype441 transferred this issue from dart-lang/sdk May 10, 2023
@stereotype441 stereotype441 added the flow-analysis Discussions about possible future improvements to flow analysis label May 10, 2023
@lrhn
Copy link
Member

lrhn commented May 10, 2023

Indeed. We already say that if T is a type-of-interest, so is, I guess, NonNull(T).

Which takes us from T?, which is definitely nullable, directly to T&Object, which is definitely non-nullable.

We don't cover potentially nullable, which is what T itself is. And which only type variables with nullable bounds can be.
(EDIT: And now extension types, which don't have an intersection form. If ET? doesn't extend Object, NonNull(ET?) is ET which isn't non-nullable.)

Or will probably just be a slight tweak to finding related types of interest when the type is a type variable, or intersection type of one. Seems doable, and not too dangerous.

Maybe we could say that if a union type is of interest, so is each part type. Then T? immediately makes T and Null be of interest, and then also add NonNull of those. (Well, maybe omit Null and Never, because they're never interesting.)
It would also work for FutureOr.

I'm not sure it helps here, unless we can demote to a related type of interest.
The promotion chain T? > T & Object doesn't contain T, so we can't demote back to it, and it's not as precise as T & Object, so we should promote to T & Object. It would be nice if we could demote back to just T instead of having to go back to T?.
But that suggests the promotion chain being seeded with potentially interesting types that we pass along the way, not just the one we promote from and to.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flow-analysis Discussions about possible future improvements to flow analysis
Projects
None yet
Development

No branches or pull requests

3 participants