Flow analysis sometimes ignores destructuring assignment

[natebosch]

There is an unexpected error in the following:

void main() {
  late String s;
  () {
    (s,) = ('',);
  }();
  print(s);
  //    ^ The late local variable 's' is definitely unassigned at this point.
}

The equivalent without using records has no diagnostic:

void main() {
  late String s;
  () {
    s = '';
  }();
  print(s);
}

This error shows up from both the analyzer and when trying to compile.

[johnniwinther]

cc @stereotype441

[stereotype441]

Good find, @natebosch! I see the problem. The bug is in the first phase of flow analysis, where it "looks ahead" to see which variables are potentially assigned in each code block, so that it can properly demote variables and adjust its notion of definite assignment at the top of loops and in the presence of closures. Both the analyzer and the CFE are failing to call AssignedVariables.write when a variable is assigned inside a pattern assignment, so flow analysis is failing to see pattern assignments when "looking ahead".

In your example, correct code is rejected. It's also possible to create examples in which incorrect code is accepted, resulting in a soundness violation, e.g.:

int f(int? i) {
  if (i == null) return 0;
  int k = 0;
  // `i` is promoted to non-nullable `int` now
  for (int j = 0; j < 2; j++) {
    // `i` should be demoted at this point, because it's assigned later in the
    // loop, so this statement should be an error.
    k += i;
    // Now assign a nullable value to `i`.
    (i,) = (null,);
  }
  return k;
}

void main() {
  print(f(0));
}

I've prototyped a fix. I'll add test cases and send it out for review ASAP.

[stereotype441]

Fix is out for review (https://dart-review.googlesource.com/c/sdk/+/310502). Since this is a soundness issue, once the fix lands I'll follow up with a cherry-pick request.