Skip to content

Commit 9248bba

Browse files
stereotype441Commit Queue
authored andcommitted
[flow analysis] Update rebasePromotedTypes to match spec.
The following changes are made to `rebasePromotedTypes`, to make it more accurately reflect the specification changes in dart-lang/language#4427. - The order of arguments is reversed to match the order of arguments in the spec. - The arguments are renamed to match the argument names in the spec, and they are changed into named arguments so that the call sites are clearer. - Comments are updated to reflect which parts of the algorithm come from the spec and which parts are optimizations. There is no functional change. Change-Id: I0d45654ad14a6414bc1cba7c56cfc66b561c7441 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/443462 Commit-Queue: Paul Berry <[email protected]> Reviewed-by: Erik Ernst <[email protected]>
1 parent 21a9b00 commit 9248bba

File tree

1 file changed

+40
-35
lines changed

1 file changed

+40
-35
lines changed

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

Lines changed: 40 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -2795,14 +2795,14 @@ class FlowModel<Type extends Object> {
27952795
newPromotedTypes =
27962796
helper.typeAnalyzerOptions.soundFlowAnalysisEnabled
27972797
? PromotionModel.rebasePromotedTypes(
2798-
helper,
2799-
afterFinallyModel.promotedTypes,
2800-
thisModel.promotedTypes,
2798+
basePromotions: thisModel.promotedTypes,
2799+
newPromotions: afterFinallyModel.promotedTypes,
2800+
helper: helper,
28012801
)
28022802
: PromotionModel.rebasePromotedTypes(
2803-
helper,
2804-
thisModel.promotedTypes,
2805-
afterFinallyModel.promotedTypes,
2803+
basePromotions: afterFinallyModel.promotedTypes,
2804+
newPromotions: thisModel.promotedTypes,
2805+
helper: helper,
28062806
);
28072807
// And we can safely restore the SSA node from the end of the try block.
28082808
newSsaNode = thisModel.ssaNode;
@@ -3081,9 +3081,9 @@ class FlowModel<Type extends Object> {
30813081
// usual "promotion chain" invariant (each promoted type is a subtype of
30823082
// the previous).
30833083
newPromotedTypes = PromotionModel.rebasePromotedTypes(
3084-
helper,
3085-
thisModel.promotedTypes,
3086-
baseModel.promotedTypes,
3084+
basePromotions: baseModel.promotedTypes,
3085+
newPromotions: thisModel.promotedTypes,
3086+
helper: helper,
30873087
);
30883088
}
30893089
// Tests are kept regardless of whether they are in `this` model or the
@@ -4286,46 +4286,51 @@ class PromotionModel<Type extends Object> {
42864286
return types2;
42874287
}
42884288

4289-
/// Forms a promotion chain by starting with [basePromotedTypes] and applying
4290-
/// promotions from [thisPromotedTypes] to it, to the extent possible without
4289+
/// Forms a promotion chain by starting with [basePromotions] and applying
4290+
/// promotions from [newPromotions] to it, to the extent possible without
42914291
/// violating the usual ordering invariant (each promoted type must be a
42924292
/// subtype of the previous).
42934293
///
42944294
/// In degenerate cases, the returned chain will be identical to
4295-
/// [thisPromotedTypes] or [basePromotedTypes] (to make it easier for the
4295+
/// [newPromotions] or [basePromotions] (to make it easier for the
42964296
/// caller to detect when data structures may be re-used).
4297-
static List<Type>? rebasePromotedTypes<Type extends Object>(
4298-
FlowModelHelper<Type> helper,
4299-
List<Type>? thisPromotedTypes,
4300-
List<Type>? basePromotedTypes,
4301-
) {
4302-
if (basePromotedTypes == null) {
4297+
static List<Type>? rebasePromotedTypes<Type extends Object>({
4298+
required List<Type>? basePromotions,
4299+
required List<Type>? newPromotions,
4300+
required FlowModelHelper<Type> helper,
4301+
}) {
4302+
if (basePromotions == null) {
43034303
// The base promotion chain contributes nothing so we just use this
43044304
// promotion chain directly.
4305-
return thisPromotedTypes;
4306-
} else if (thisPromotedTypes == null) {
4305+
return newPromotions;
4306+
} else if (newPromotions == null) {
43074307
// This promotion chain contributes nothing so we just use the base
4308-
// promotion chain directly.
4309-
return basePromotedTypes;
4308+
// promotion chain directly. Note: this is a performance optimization of
4309+
// the `else` block below; it is not required by the spec.
4310+
return basePromotions;
43104311
} else {
43114312
// Start with basePromotedTypes and apply each of the promotions in
43124313
// thisPromotedTypes (discarding any that don't follow the ordering
43134314
// invariant)
4314-
List<Type> newPromotedTypes = basePromotedTypes;
4315-
Type otherPromotedType = basePromotedTypes.last;
4316-
for (int i = 0; i < thisPromotedTypes.length; i++) {
4317-
Type nextType = thisPromotedTypes[i];
4318-
if (helper.typeOperations.isSubtypeOf(nextType, otherPromotedType) &&
4315+
Type basePromotedType = basePromotions.last;
4316+
for (int i = 0; i < newPromotions.length; i++) {
4317+
Type nextType = newPromotions[i];
4318+
// Determine if `nextType` is safe to attach to `basePromotedTypes`.
4319+
if (helper.typeOperations.isSubtypeOf(nextType, basePromotedType) &&
43194320
helper.isValidPromotionStep(
4320-
previousType: otherPromotedType,
4321+
previousType: basePromotedType,
43214322
newType: nextType,
43224323
)) {
4323-
newPromotedTypes =
4324-
basePromotedTypes.toList()..addAll(thisPromotedTypes.skip(i));
4325-
break;
4324+
// Since `newPromotions` is a valid promotion chain, it follows that
4325+
// all the types that follow `nextType` are also safe to attach to the
4326+
// base promotion chain, so simply concatenate `basePromotions` with
4327+
// the remainder of `newPromotions`.
4328+
return basePromotions.toList()..addAll(newPromotions.skip(i));
43264329
}
43274330
}
4328-
return newPromotedTypes;
4331+
// No types from `newPromotions` were safe to attach to
4332+
// `basePromotedTypes`, so return `basePromotions` unchanged.
4333+
return basePromotions;
43294334
}
43304335
}
43314336

@@ -4753,9 +4758,9 @@ class SsaNode<Type extends Object> {
47534758
}
47544759
List<Type>? newPromotedTypes = newModel.promotedTypes;
47554760
List<Type>? rebasedPromotedTypes = PromotionModel.rebasePromotedTypes(
4756-
helper,
4757-
afterFinallyPromotedTypes,
4758-
newPromotedTypes,
4761+
basePromotions: newPromotedTypes,
4762+
newPromotions: afterFinallyPromotedTypes,
4763+
helper: helper,
47594764
);
47604765
if (!identical(newPromotedTypes, rebasedPromotedTypes)) {
47614766
newFlowModel = newFlowModel.updatePromotionInfo(

0 commit comments

Comments
 (0)