Skip to content

Commit

Permalink
Flow analysis: fix handling of list pattern type promotion.
Browse files Browse the repository at this point in the history
When analyzing the type test implied by a pattern, flow analysis uses
three variables to control promotion behavior:

- `matchFailsIfWrongType`, which indicates whether flow analysis needs
  to account for the possible control flow path resulting from the
  type test failing. (This is `false` for cast patterns, because in
  the case where a cast pattern fails, an exception is thrown).

- `matchMayFailEvenIfCorrectType`, which indicates whether flow
  analysis needs to account for the possible control flow path
  resulting from the type test succeeding, but some other check
  causing the match to fail. (This is `true` for most list patterns,
  because the list pattern will fail to match if the list has the
  wrong length).

  (Note that `matchMayFailEvenIfCorrectType` doesn't account for the
  fact that a pattern match might fail due to failure in a subpattern
  match; this is automatically handled by the fact that flow analysis
  walks through the complete pattern in the order in which it
  executes.)

- `coversMatchedType`, which indicates whether the type test is
  guaranteed to succeed due to a subtype relationship between the
  matched value type and the type being tested (e.g. a `num x` pattern
  is guaranteed to succeed if the matched value type is `int`).

In the case where `matchFailsIfWrongType` is `true`,
`matchMayFailEvenIfCorrectType` is `true`, and `coversMatchedType` is
`false`, flow analysis must account for the fact that there are two
ways that the pattern match might fail: the type test might fail, or
the type test might succeed but then the pattern match might fail for
some other reason.

Before this change, this was done incorrectly, and flow analysis only
accounted for the possibility of the type test failing.

Fixes #55543.

Bug: #55543
Change-Id: I86603ec5f940402313f32177212b7960878db97f
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/364942
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
  • Loading branch information
stereotype441 authored and Commit Queue committed May 1, 2024
1 parent cd6ad63 commit b026068
Show file tree
Hide file tree
Showing 3 changed files with 65 additions and 11 deletions.
15 changes: 12 additions & 3 deletions pkg/_fe_analyzer_shared/lib/src/flow_analysis/flow_analysis.dart
Expand Up @@ -5200,9 +5200,18 @@ class _FlowAnalysisImpl<Node extends Object, Statement extends Node,
.ifFalse;
}
_current = ifTrue;
if (matchMayFailEvenIfCorrectType ||
(matchFailsIfWrongType && !coversMatchedType)) {
_unmatched = _join(_unmatched!, coversMatchedType ? ifTrue : ifFalse);
if (matchFailsIfWrongType && !coversMatchedType) {
// There's a reachable control flow path where the match might fail due to
// a type mismatch. Therefore, we must update the `_unmatched` flow state
// based on the state of flow analysis assuming the type check failed.
_unmatched = _join(_unmatched!, ifFalse);
}
if (matchMayFailEvenIfCorrectType) {
// There's a reachable control flow path where the type might match, but
// the match might nonetheless fail for some other reason. Therefore, we
// must update the `_unmatched` flow state based on the state of flow
// analysis assuming the type check succeeded.
_unmatched = _join(_unmatched!, ifTrue);
}
return coversMatchedType;
}
Expand Down
35 changes: 27 additions & 8 deletions pkg/_fe_analyzer_shared/test/flow_analysis/flow_analysis_test.dart
Expand Up @@ -8299,15 +8299,34 @@ main() {

group('List pattern:', () {
group('Not guaranteed to match:', () {
test('Empty list', () {
h.run([
switch_(expr('List<Object>'), [
listPattern([]).then([break_()]),
default_.then([
checkReachable(true),
group('Empty list:', () {
test('matched value type is non-nullable list', () {
h.run([
switch_(expr('List<Object>'), [
listPattern([]).then([break_()]),
default_.then([
checkReachable(true),
]),
]),
]),
]);
]);
});

test('matched value type is nullable list', () {
var x = Var('x');
h.run([
declare(x, initializer: expr('List<Object?>?')),
switch_(x, [
listPattern([]).then([
checkReachable(true),
checkPromoted(x, 'List<Object?>'),
]),
default_.then([
checkReachable(true),
checkNotPromoted(x),
]),
]),
]);
});
});

test('Single non-rest element', () {
Expand Down
@@ -0,0 +1,26 @@
// Copyright (c) 2024, the Dart project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.

// This test exercises a corner case of list pattern handling where the type of
// the scrutinee is a nullable list. Prior to fixing
// https://github.com/dart-lang/sdk/issues/55543, flow analysis would fail to
// account for the fact that a list pattern might fail to match due to a list
// length mismatch, so it would incorrectly conclude that in the "match failure"
// case, the scrutinee was `null`.

import "../../static_type_helper.dart";

test(List<Object?>? x) {
switch (x) {
case []:
x.expectStaticType<Exactly<List<Object?>>>();
default:
x.expectStaticType<Exactly<List<Object?>?>>();
}
}

main() {
test(null);
test([]);
}

0 comments on commit b026068

Please sign in to comment.