Skip to content

Commit 9f0e522

Browse files
stereotype441Commit Queue
authored and
Commit Queue
committed
[flow analysis] Fix layering of type promotions in try/finally.
A tricky part of the implementation of flow analysis is the handling of try/finally statements. Although promotions are tracked separately in the `try` and `finally` blocks, promotions from both blocks need to be merged together at the conclusion of the finally block. This creates an ambiguity, because each type in a promotion chain is required to be a subtype of the previous, and hence multiple promotions of the same variable are inherently ordered. The ambiguity is: when the promotions from the `try` and `finally` block are merged, which promotions should be applied first? In discussion with the language team, we've decided that the promotions from the `try` block should be applied first, because that matches the order of code execution. This change makes the behavior of flow analysis more uniform, which should make it easier to reason about and maintain. In practice, the difference in behavior is quite subtle, and I don't expect users to notice. However, to be on the safe side, the change in behavior is conditioned on the `sound-flow-analysis` flag, so it will only take effect when the user deliberately upgrades to language version 3.9, and it will not affect already-published packages. A test in google3 showed that no internal code would be broken by force-enabling this change. Fixes dart-lang/language#4382. Change-Id: I0e9f6db808a964e0b4325d3020654a9f2be273a2 Bug: dart-lang/language#4382 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/432001 Reviewed-by: Konstantin Shcheglov <[email protected]> Commit-Queue: Paul Berry <[email protected]>
1 parent 5f9c5da commit 9f0e522

File tree

6 files changed

+753
-40
lines changed

6 files changed

+753
-40
lines changed

pkg/_fe_analyzer_shared/lib/src/flow_analysis/flow_analysis.dart

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2668,11 +2668,18 @@ class FlowModel<Type extends Object> {
26682668
//
26692669
// In all of these cases, the correct thing to do is to keep all
26702670
// promotions that were done in both the `try` and `finally` blocks.
2671-
newPromotedTypes = PromotionModel.rebasePromotedTypes(
2672-
helper,
2673-
thisModel.promotedTypes,
2674-
afterFinallyModel.promotedTypes,
2675-
);
2671+
newPromotedTypes =
2672+
helper.typeAnalyzerOptions.soundFlowAnalysisEnabled
2673+
? PromotionModel.rebasePromotedTypes(
2674+
helper,
2675+
afterFinallyModel.promotedTypes,
2676+
thisModel.promotedTypes,
2677+
)
2678+
: PromotionModel.rebasePromotedTypes(
2679+
helper,
2680+
thisModel.promotedTypes,
2681+
afterFinallyModel.promotedTypes,
2682+
);
26762683
// And we can safely restore the SSA node from the end of the try block.
26772684
newSsaNode = thisModel.ssaNode;
26782685
if (newSsaNode != afterFinallyModel.ssaNode) {
@@ -3382,6 +3389,9 @@ mixin FlowModelHelper<Type extends Object> {
33823389
@visibleForTesting
33833390
PromotionKeyStore<Object> get promotionKeyStore;
33843391

3392+
/// Language features enables affecting the behavior of flow analysis.
3393+
TypeAnalyzerOptions get typeAnalyzerOptions;
3394+
33853395
/// The [FlowAnalysisTypeOperations], used to access types and check
33863396
/// subtyping.
33873397
@visibleForTesting
@@ -4988,7 +4998,7 @@ class _FlowAnalysisImpl<
49884998
implements
49894999
FlowAnalysis<Node, Statement, Expression, Variable, Type>,
49905000
_PropertyTargetHelper<Expression, Type> {
4991-
/// Language features enables affecting the behavior of flow analysis.
5001+
@override
49925002
final TypeAnalyzerOptions typeAnalyzerOptions;
49935003

49945004
/// The [FlowAnalysisOperations], used to access types, check subtyping, and

pkg/_fe_analyzer_shared/test/flow_analysis/flow_analysis_mini_ast.dart

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import 'package:_fe_analyzer_shared/src/flow_analysis/flow_analysis.dart';
66
import 'package:_fe_analyzer_shared/src/flow_analysis/flow_analysis_operations.dart';
77
import 'package:_fe_analyzer_shared/src/type_inference/promotion_key_store.dart';
88
import 'package:_fe_analyzer_shared/src/type_inference/type_analysis_result.dart';
9+
import 'package:_fe_analyzer_shared/src/type_inference/type_analyzer.dart';
910
import 'package:_fe_analyzer_shared/src/types/shared_type.dart';
1011

1112
import '../mini_ast.dart';
@@ -38,6 +39,9 @@ class FlowAnalysisTestHarness extends Harness
3839
@override
3940
final SharedTypeView boolType = SharedTypeView(Type('bool'));
4041

42+
@override
43+
TypeAnalyzerOptions get typeAnalyzerOptions => computeTypeAnalyzerOptions();
44+
4145
@override
4246
FlowAnalysisOperations<Var, SharedTypeView> get typeOperations =>
4347
typeAnalyzer.operations;

0 commit comments

Comments
 (0)