Change flow analysis to assume sound null safety once unsound null safety support is fully removed

[stereotype441]

Currently, flow analysis conservatively assumes that a variable with a non-nullable type might still be null because the program might be running with unsound null safety, and we don't want null-related unsoundness to be able to be escalated into full unsoundness (see #1143 for details).

Although we don't officially support running with unsound null safety anymore, support has not yet been fully removed. Once we remove it, we should remove this conservative assumption from flow analysis.

Note that this is a significant enough change that it probably should be tied to a language version.

[lrhn]

I'm actually worried that we assume some syntactically allowed control flow cannot happen because of type analysis.
We went back on that because of unsound null safety, but I also think it was the right idea in general.

If code contains a branch, I think we should assume that all branches can be taken, if at all possible.
We currently don't in some cases, predicting that a particular branch will never get executed.
That matters for things like definite assignment, where a variable assigned on every reachable branch is considered definitely assigned after the branches has been joined.

The problem with that is that an unreachable branch is dead code. Dead code is likely a mistake, or at least a temporary thing. At some point that code will either be deleted, or the branch will be enabled.
If type inference and definite assignment changes when you remove a temporary && false added to an if condition, then you might have to go back and do a larger rewrite.

If type analysis and definite assignment has always considered the branch to be executable, then twiddling with the conditions won't suddenly make your code have type errors.

The one issue I see with such branches, is that it's not always clear which types promotable variables have on entry, since the reason the branch is unreachable might be a type test that rules out everything.

Say (probably not a correct example, but maybe there are valid examples):

// ignore_for_file: prefer_void_to_null
import "dart:async";

void main() {
  FutureOr<Null> x = null as dynamic;
  int y;
  int? z = 0;
  if (x is Future<Null>) {
    // x promoted to `Future<Nul>`
    y = 0;
  } else {
    // x promoted to `Null`
    if (x != null) {
      // x promoted to `Never` ? (Actually not, it's still `Null`)
      z = null;
    } else {
      y = 2;
    }
  }
  print(y);
  z.toRadixString(10); // Allowed because `z=null` doesn't demote.
}

Here y is considered definitely assigned, and z is not demoted to nullable,
because the type inference/definite assignment decides that the x != null cannot be true.

We do analyze the "unreacable" branch (can be seen by introducing type errors there), we just don't merge it back into the result afterwards

That's "very clever" from an analysis which cannot determine that const f= false; if (f) { reachable? } is not reachable.

Which brings me back to the argument that unreachable code should probably not be considered to be deliberately unreachable, because nobody needs unreachable code in their final production code.
If you don't need the code right now, you can comment it out, or delete it.

If the code is still there, we should, IMO, assume that it's intended to be reachable, and analyze the program as if it was.
So that when it does become reachable, by flipping a single false to true, we don't get sudden non-local changes in analysis results.

It should even be easier, just assume that all branches in the code, and all merges after them, are (intended to be) possible.

[stereotype441]

Now that dart-lang/sdk@0060b0f has removed support for running in unsound null safety mode, I'm going to try prototyping this to see how breaking of a change it would be. Once I have that information, I'll report my findings and we can decide whether we want to go ahead with this change or not.

[eernstg]

At the meeting Wednesday Mar 5 2025 the language team decided to proceed with this proposal.

[eernstg]

@stereotype441, I created dart-lang/sdk#60438 in order to make it possible to see the existence and status of the ongoing implementation effort as usual. However, the old approach (one sub-issue for the analyzer and another one for the CFE, and at times lots of other issues for other things) doesn't fit the reality any more. Seems like it would suffice to just have dart-lang/sdk#60438?

[stereotype441]

@eernstg

@stereotype441, I created dart-lang/sdk#60438 in order to make it possible to see the existence and status of the ongoing implementation effort as usual. However, the old approach (one sub-issue for the analyzer and another one for the CFE, and at times lots of other issues for other things) doesn't fit the reality any more. Seems like it would suffice to just have dart-lang/sdk#60438?

Thanks, I'll attach all of my CLs to dart-lang/sdk#60438. I think a single implementation issue is sufficient.