sdk icon indicating copy to clipboard operation
sdk copied to clipboard

Inconsistent switch promotion.

Open lrhn opened this issue 8 months ago • 1 comments

Example:

import 'dart:async';

void main() {
  var f = Future<int>.value(1);
  var o = f as FutureOr<int>?;
  for (var b in [true, false]) {
    if (b) {
      print(switch (o) {
        null => "nope",
        int _ => "neither",
        var _ => "${o.staticType}",  // No variable name.
      });
    } else {
      print(switch (o) {
        null => "nope",
        int _ => "neither",
        var x => "${o.staticType}", // Variable name, not referenced.
      });
    }
  }
}

extension<T> on T {
  Type get staticType => T;
}

When run, this prints Future<int>? and Future<int>. The promotion in the switch depends on whether the last catch-all clause has a variable name or not, even if the variable name is never referenced. That's probably not intended.

The second result is the best result, after having rejected null and int, a FutureOr<int>? can be concluded to be a Future<int>.

If I switch the first two cases, the result changes to FutureOr<int> and Future<int>, so it's like the promotion from failing the first case gets lost when going from case 2 to case 3.

Probably some optimization going wrong, maybe overly optimistically thinking that a pattern that doesn't reference o can't affect its type. (But that should be the matched value type before the pattern is applied, so probably something more than that.)

@stereotype441

lrhn avatar Apr 08 '25 13:04 lrhn

I dug through the implementation a bit, and this is definitely a bug.

When analyzing a switch statement, flow analysis potentially has to keep track of promotions to two different values: the switch scrutinee, and the variable the switch scrutinee refers to. The reason these might be different is because a when clause might contain an assignment to the variable. For example:

bool f(_) => false;
Object? g() => null;
void h(Object _) {}
test(Object? x) {
  switch (x) {
    case _ when f(x = g()): // `x` and the scrutinee no longer point to the same value
      break;
    case null: // promotes the scrutinee to `Object`, but not `x`
      break;
    case var y: // Inferred type of `y` is `Object`
      h(y); // ok; `y` is `Object`
      h(x); // ERROR: `x` might be `null`
  }
}

This example works today. But the flow analysis logic to keep track of the two potential promotions is a little wonky. It's been on the back burner for me to clean it up, but it was hard to justify because I wasn't aware of any bugs it was causing... until now!

Here's what goes wrong in @lrhn's example:

  • In the code path where null => "nope" fails to match, flow analysis promotes both o and the switch scrutinee to non-nullable FutureOr<int>.
  • In the code path where int _ => "neither" fails to match:
    • flow analysis promotes the switch scrutinee from its previously promoted type (FutureOr<int>) to Future<int> (because factor(FutureOr<int>, int) = Future<int>; see the definition of factor)
    • but it fails to remember that the previously promoted type of o is FutureOr<int>; it still thinks it's FutureOr<int>?. Therefore, it promotes o to Future<int>? (because factor(FutureOr<int>?, int) = Future<int>?).[^1]
  • In the first switch, when var _ is encountered, no further promotions are done, so o remains promoted to Future<int>?.
  • In the second switch, when var x is encountered, flow analysis infers a type for x based on the promoted type of the switch scrutinee (which is Future<int>). Then, it treats var x as equivalent to Future<int> x; this causes o to be promoted to Future<int>.

I've added this issue to my list of issues to potentially fix as part of my work on the sound-flow-analysis feature.

[^1]: Note that this violates the property that each entry in promotedTypes is supposed to be a subtype of the previous.

stereotype441 avatar Apr 08 '25 18:04 stereotype441