Dart cannot be sure that the method argument is not null if the argument is used in a closure

[vadlit]

By some reason Dart cannot be sure that the method argument is not null if the argument is used in a closure. Property 'isEven' cannot be accessed on 'int?' because it is potentially null

void main() {
  foo(1);
}

void foo(int? nullable) {
  Future.delayed(Duration(seconds:0)).then((_)
  {
    if (nullable == null) { 
      nullable = 5;
    }
    if (nullable == null) {
      return;
    }
    print(nullable.isEven);
  });
}

In this case, as far as I undestand, there's no chance that the 'nullable' argument suddenly becomes nullable after the 1st check, because it's just a method argument, not class field.
The problem is not reproduced if remove the first check.

Dart 2.12.2
Windows

[eernstg]

Cf. #1247. It's possible that the flow analysis could give special treatment to the case where all references (read & write) to a given local variable occurs in the same function literal or local function, even though the variable is declared in an enclosing function.

[lrhn]

The general issue here is that the exists a function which assigns to the variable, and we assume it can be invoked at any time. Even inside that function, we can't be sure that the same function won't be invoked again.
This code is so exceedingly simple that we can actually be sure nothing happens between the checks (== null calls no other code) and assignments (assigning to a local variable calls no other code). Most code isn't that simple, so it's not something we optimize for.

However, there is something we can do better here. We have a situation where all assignments to the variable assign a non-nullable value. We know that there is no assignment floating around which can make the variable null after we have checked that it's non-null.
After the

  if (nullable == null) {
    nullable = 5;
  }

we do know that nullable is non-null because it was, and there is no assignment to nullable anywhere which can invalidate that. Even if we don't know when this function is called, what state we start in or which code can run before, after or during the invocation, we still know that after proving that nullable is non-null by either failing at nullable == null or assigning 5 to nullable, it will stay non-nullable.

@stereotype441

[stereotype441]

@lrhn Agreed, it would be really nice if we could see that all assignments to the variable assign non-nullable values. Unfortunately, doing that in the general case is difficult. Consider:

extension on int? {
  int bar() => this ?? 0;
}
extension on int {
  int? bar() => this == 0 ? null : this - 1;
}
void doStuff1(void callback()) { ... }
void doStuff2() { ... }
void foo(int? nullable) {
  doStuff(() {
    if (nullable == null) {
      nullable = 5;
    }
    doStuff2();
    nullable = nullable.bar();
    print(nullable.isEven); // Is this ok?
  });
}

Now, let's assume our rule is that the promotion succeeds if the right hand side of every assignment to nullable assigns a non-nullable value. To figure out whether promotion succeeds in this case, we have to figure out whether nullable.bar() has a nullable type. That depends on the type of nullable (if nullable has type int, then nullable.bar() has type int?, and vice versa). So to figure out whether nullable.bar() has a nullable type we first have to figure out whether nullable has a nullable type. But that in turn depends on whether or not the promotion succeeded. And we are caught in a loop.

Obviously this is a contrived example, but we have to have some sort of plan for how to break loops like this. When I was designing flow analysis, one of my goals was to be able to do the whole algorithm in two passes over the source code: one to locate assignments, and one to do the final type analysis. This eased the implementation by adhering to some assumptions in the analyzer and CFE that type analysis only visits each subexpression once. But it had the disadvantage that in cases like the one @vadlit mentioned, flow analysis has to decide whether it's safe to promote nullable based solely on whether/when it's assigned to, and not accounting for the types assigned to it. Also, it has to conservatively assume that closures might be invoked reentrantly (it doesn't know that Future.then only executes its argument once). The combination of those two factors means that it can't tell that it's safe to promote nullable.

If we dropped the "two passes" requirement, we could certainly do a better job of cases like this by iterating to a fixed point. It would require modification to the analyzer and CFE to allow subexpressions to be visited multiple times during type analysis. And it would require some care in the design to make sure that a fixed point would always be reached (it would be bad if we assigned a variable a provisional type of Object, then refined it to List<Object>, then to List<List<Object>>, and so on forever). But assuming we sorted all that out, then it would be totally doable to detect that all assignments to nullable assign non-null values, therefore the promotion is safe.