dart-lang / sdk

The Dart SDK, including the VM, JS and Wasm compilers, analysis, core libraries, and more.
https://dart.dev
BSD 3-Clause "New" or "Revised" License
10.2k stars 1.57k forks source link

[NNBD] Type promotion via flow analysis fails in nested loops #45360

Open nex3 opened 3 years ago

nex3 commented 3 years ago

The following code:

int foo(int? arg) {
  arg = 0;
  while (true) {
    while (true) {
      arg++;
    }
  }
}

produces this error:

  error • The method '+' can't be unconditionally invoked because the receiver can be 'null'. • test.dart:5:7 • unchecked_use_of_nullable_value

Despite the fact that arg is always provably non-null by the point the ++ happens.

stereotype441 commented 3 years ago

Yes, this is a known limitation of flow analysis. We deliberately decided to make the algorithm work in a single pass through the source code (with limited look-ahead to find writes inside loops) rather than iterate to a fixed point. So when flow analysis reaches the top of the first while loop, it sees that the loop contains a write to arg, but it doesn't yet know the type of the value written to arg. So it makes the conservative assumption that on any future iteration of the loop, arg might be null.

nex3 commented 3 years ago

For better or for worse, NNBD makes flow analysis a really substantial part of the developer experience of the language. In order to write correct code, developers now need to have a mental model of how the analyzer will track nullability. Roughly speaking, the intuitive model is:

Whenever the actual behavior breaks from this model, it causes developer frustration and a need to do elaborate workarounds. It makes the substantial value provided by NNBD feel like a chore rather than a boon. This is particularly true because TypeScript—by far the most popular language with a similar nullability model, and thus a source of user expectations—does handle cases like this as expected. As such, I strongly encourage you to rethink the simplicity of the flow analysis model to allow it to better match developers' intuitive understanding of how it "should" work.