Skip to content

Inconsistent switch promotion. #60496

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
lrhn opened this issue Apr 8, 2025 · 1 comment
Open

Inconsistent switch promotion. #60496

lrhn opened this issue Apr 8, 2025 · 1 comment
Labels
area-dart-model For issues related to conformance to the language spec in the parser, compilers or the CLI analyzer. model-flow Implementation of flow analysis in analyzer/cfe

Comments

@lrhn
Copy link
Member

lrhn commented Apr 8, 2025

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 lrhn added the area-dart-model For issues related to conformance to the language spec in the parser, compilers or the CLI analyzer. label Apr 8, 2025
@stereotype441
Copy link
Member

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.

Footnotes

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

@johnniwinther johnniwinther added the model-flow Implementation of flow analysis in analyzer/cfe label Apr 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-dart-model For issues related to conformance to the language spec in the parser, compilers or the CLI analyzer. model-flow Implementation of flow analysis in analyzer/cfe
Projects
None yet
Development

No branches or pull requests

3 participants