languages/dart-sdk
languages/dart-sdk
1
0
Fork 0
mirror of https://github.com/dart-lang/sdk synced 2024-10-01 16:44:02 +00:00
Code Issues Packages Projects Releases Wiki Activity

Flow analysis: use a more precise split point for refutable patterns.

Browse source 
Previously, the flow control logic for patterns didn't use the
`FlowModel.split` or `FlowModel.unsplit` methods at all. This meant
that if a control flow join point occurred in pattern logic, flow
analysis would consider the split point to be whatever split point was
established by the enclosing expression or statement. In the case of
an if-case statement, it would consider the split point to be at the
beginning of the scrutinee expression.

Split points are used by flow analysis for the sole purpose of
ensuring that joins propagate type promotions the same way in dead
code as they do in live code (so that users introducing temporary
`throw` expressions or `return` statements into their code do not have
to deal with nuisance compile errors in the (now dead) code that
follows. The consequence of flow analysis considering the split point
to be at the beginning of the scrutinee expression is that if the
scrutinee expression is proven to always throw, then joins that arise
from the pattern or guard may not behave consistently with how they
would have behaved otherwise. For example:

    int getInt(Object o) => ...;
    void consumeInt(int i) { ... }
    test(int? i) {
      if (
          // (1)
          getInt('foo')
          case
              // (2)
              int()
          // (3)
          when i == null) {
      } else {
        // (4)
        consumeInt(i);
      }
    }

In the above code, there is a join point at (4), joining control flows
from (a) the situation where the pattern `int()` failed to match, and
(b) the situation where `i == null` evaluated to `false` (and hence
`i` is promoted to non-nullable `int`). Since the return type of
`getInt` is `int`, it's impossible for the pattern `int()` to fail, so
at the join point, control flow path (a) is considered
unreacable. Therefore the promotion from control flow path (b) is
kept, and so the call to `consumeInt` is valid.

In order to decide whether to preserve promotions from one of the
control flow paths leading up to a join, flow analysis only considers
reachability relative to the corresponding split point. Prior to this
change, the split point in question occurred at (1), so if the
expression `getInt('foo')` had been replaced with `getInt(throw
UnimplementedError())`, flow analysis would have considered both
control flow paths (a) and (b) to be unreachable relative to the split
point, so it would not have preserved the promotion from (b), and
there would have been a compile time error in the (now dead) call to
`consumeInt`.

This change moves the split point from (1) to (2), so that changing
`getInt('foo')` to `getInt(throw UnimplementedError())` no longer
causes any change in type promotion behavior.

The implementation of this change is to add calls to `FlowModel.split`
and `FlowModel.unsplit` around all top-level patterns. At first glance
this might appear to affect the behavior of all patterns, but actually
the only user-visible effect is on patterns in if-case statements,
because:

- In switch statements and switch expressions, there is already a
  split point before each case.

- In irrefutable patterns, there is no user-visible effect, because
  irrefutable patterns cannot fail to match, and therefore don't do
  any control flow joins.

This change allows the split points for patterns to be determined by a
simple syntactic rule, which will facilitate some refactoring of split
points that I am currently working on.

Change-Id: I55573ba5c28b2f2e6bba8731f9e3b02613b6beb2
Bug: https://github.com/dart-lang/sdk/issues/53167
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/319381
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
This commit is contained in:
Paul Berry 2023-08-11 17:09:49 +00:00 committed by Commit Queue
parent c9684e54e8
commit 907e705307
5 changed files with 166 additions and 1 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

14
CHANGELOG.md
View file

@ -2,6 +2,20 @@
### Language
- **Breaking Change** [#53167][]: Use a more precise split point for refutable
patterns. Previously, in an if-case statement, if flow analysis could prove
that the scrutinee expression was guaranteed to throw an exception, it would
sometimes fail to propagate type promotions implied by the pattern to the
(dead) code that follows. This change makes the type promotion behavior of
if-case statements consistent regardless of whether the scrutinee expression
throws an exception.
No live code is affected by this change, but there is a small chance that the
change in types will cause a compile-time error to appear in some dead code in
the user's project, where no compile-time error appeared previously.
[#53167]: https://github.com/dart-lang/sdk/issues/53167
### Libraries
#### `dart:js_interop`

4
pkg/_fe_analyzer_shared/lib/src/flow_analysis/flow_analysis.dart
View file

@ -5483,7 +5483,8 @@ class _FlowAnalysisImpl<Node extends Object, Statement extends Node,
_current = guardInfo.ifTrue;
unmatched = _join(unmatched, guardInfo.ifFalse);
}
return unmatched;
_current = _current.unsplit();
return unmatched.unsplit();
}
void _popScrutinee() {
@ -5496,6 +5497,7 @@ class _FlowAnalysisImpl<Node extends Object, Statement extends Node,
/// into a pattern or subpattern match. [matchedValueInfo] should be the
/// [EqualityInfo] representing the value being matched.
void _pushPattern(_Reference<Type> matchedValueInfo) {
_current = _current.split();
_stack.add(new _TopPatternContext<Type>(matchedValueInfo, _unmatched));
_unmatched = _current.setUnreachable();
}

91
pkg/_fe_analyzer_shared/test/flow_analysis/flow_analysis_test.dart
View file

@ -9863,6 +9863,97 @@ main() {
[]),
]);
});
group('Split points:', () {
test('Guarded', () {
// This test verifies that for a guarded pattern, the join of the two
// "unmatched" control flow paths corresponds to a split point at the
// beginning of the pattern.
var i = Var('i');
h.run([
declare(i, initializer: expr('int?')),
ifCase(
second(throw_(expr('Object')), expr('int')).checkType('int'),
objectPattern(requiredType: 'int', fields: [])
.when(i.eq(nullLiteral)),
[],
[
// There is a join point here, joining the flow control paths
// where (a) the pattern `int()` failed to match and (b) the
// guard `i == null` was not satisfied. Since the scrutinee has
// type `int`, and the pattern is `int()`, the pattern is
// guaranteed to match, so path (a) is unreachable. Path (b) is
// also unreachable due to the fact that the scrutinee throws,
// but since the split point is the beginning of the pattern,
// path (b) is reachable from the split point. So the promotion
// implied by (b) is preserved after the join.
checkPromoted(i, 'int'),
// Note that due to the `throw` in the scrutinee, this code is
// unreachable.
checkReachable(false),
]),
]);
});
test('Logical-or', () {
// This test verifies that for a logical-or pattern, the join of the two
// "matched" control flow paths corresponds to a split point at the
// beginning of the top level pattern.
var x = Var('x');
h.run([
ifCase(
expr('(Null, Null, int?)'),
recordPattern([
relationalPattern('!=', nullLiteral).recordField(),
wildcard().recordField(),
wildcard().recordField()
])
// At this point, control flow is unreachable due to the fact
// that the `!= null` pattern in the first field of the
// record pattern above can never match the type `Null`.
.and(recordPattern([
wildcard().recordField(),
relationalPattern('!=', nullLiteral).recordField(),
wildcard().recordField()
])
// At this point, control flow is unreachable for a
// second reason: because the `!= null` pattern in the
// second field of the record pattern above can never
// match the type `Null`.
.or(recordPattern([
wildcard().recordField(),
wildcard().recordField(),
wildcard().nullCheck.recordField()
])
// At this point, the third field of the scrutinee
// is promoted from `int?` to `int`, due to the
// null check pattern.
)
// At this point, there is a control flow join between the
// two branches of the logical-or pattern. Since the split
// point corresponding to the control flow join is at the
// beginning of the top level pattern, both branches are
// considered unreachable, so neither is favored in the
// join, and therefore, the promotion from the second
// branch is lost.
)
.and(
// The record pattern below matches `x` to the unpromoted
// type of the third field of the scrutinee, so we just
// have to verify that it has the expected type of `int?`.
recordPattern([
wildcard().recordField(),
wildcard().recordField(),
x.pattern(expectInferredType: 'int?').recordField()
])),
[
// As a sanity check, confirm that the overall pattern
// can't ever match.
checkReachable(false),
]),
]);
});
});
});
}

1
pkg/_fe_analyzer_shared/test/mini_ast.dart
View file

@ -2616,6 +2616,7 @@ class MiniAstOperations
'error <: int': Type('error'),
'error <: num': Type('error'),
'int <: dynamic': Type('int'),
'int <: int': Type('int'),
'int <: num': Type('int'),
'int <: Object?': Type('int'),
'List <: Iterable<int>': Type('List<int>'),

57
tests/language/patterns/flow_analysis/split_point_test.dart Normal file
View file

@ -0,0 +1,57 @@
// Copyright (c) 2023, 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.
// Verifies that when there is a control flow join implied by a pattern, the
// split point is the beginning of the top level pattern.
import '../../static_type_helper.dart';
void guarded(int Function(Object) f, int? i) {
if (f(throw '') case int() when i == null) {
} else {
// There is a join point here, joining the flow control paths where (a) the
// pattern `int()` failed to match and (b) the guard `i == null` was not
// satisfied. Since the scrutinee has type `int`, and the pattern is
// `int()`, the pattern is guaranteed to match, so path (a) is
// unreachable. Path (b) is also unreachable due to the fact that the
// scrutinee throws, but since the split point is the beginning of the
// pattern, path (b) is reachable from the split point. So the promotion
// implied by (b) is preserved after the join.
i.expectStaticType<Exactly<int>>();
}
}
void logicalOr((Null, Null, int?) x) {
if (x
case ((!= null, _, _)
// At this point, control flow is unreachable due to the fact that
// the `!= null` pattern in the first field of the record pattern
// above can never match the type `Null`.
&&
((_, != null, _)
// At this point, control flow is unreachable for a second
// reason: because the `!= null` pattern in the second field
// of the record pattern above can never match the type
// `Null`.
||
(_, _, _?)
// At this point, the third field of the scrutinee is promoted
// from `int?` to `int`, due to the null check pattern.
)
// At this point, there is a control flow join between the two
// branches of the logical-or pattern. Since the split point
// corresponding to the control flow join is at the beginning of the
// top level pattern, both branches are considered unreachable, so
// neither is favored in the join, and therefore, the promotion from
// the second branch is lost.
) &&
// The record pattern below matches `x` to the unpromoted type of the
// third field of the scrutinee, so we just have to verify that it has
// the expected type of `int?`.
(_, _, var y)) {
y.expectStaticType<Exactly<int?>>();
}
}
main() {}