Skip to content

Commit 1b95cf1

Browse files
stereotype441Commit Queue
authored andcommitted
[flow analysis] Change representation of promotion chains to match spec.
Previously, flow analysis represented a promotion chain as `List<Type>?`, with `null` representing an empty promotion chain. This was an unnecessary optimization, and it was a source of confusion when comparing the implementation of flow analysis to the spec. This CL changes the representation of a promotion chain to a non-nullable `List<Type>`, and represents an empty promotion chain as an empty list. To reduce the risk of mistakes, I've tried to minimize the changes to flow analysis unit tests; for the most part, they still consider an empty promotion chain to be represented as `null`, and convert to the new representation at the last minute. In a follow-up CL, I'll modify the flow analysis unit tests to better follow the new representation. Change-Id: I72afd18f9d7729f3109e3e3b5c776c5a23860985 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/443540 Reviewed-by: Erik Ernst <[email protected]> Commit-Queue: Paul Berry <[email protected]>
1 parent 111e84b commit 1b95cf1

File tree

2 files changed

+78
-77
lines changed

2 files changed

+78
-77
lines changed

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

Lines changed: 54 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -2772,7 +2772,7 @@ class FlowModel<Type extends Object> {
27722772
// We can just use the "write captured" state from the `finally` block,
27732773
// because any write captures in the `try` block are conservatively
27742774
// considered to take effect in the `finally` block too.
2775-
List<Type>? newPromotedTypes;
2775+
List<Type> newPromotedTypes;
27762776
SsaNode<Type>? newSsaNode;
27772777
if (beforeFinallyModel == null ||
27782778
beforeFinallyModel.ssaNode == afterFinallyModel.ssaNode) {
@@ -3067,10 +3067,10 @@ class FlowModel<Type extends Object> {
30673067
// it's captured now.
30683068
bool newWriteCaptured =
30693069
thisModel.writeCaptured || baseModel.writeCaptured;
3070-
List<Type>? newPromotedTypes;
3070+
List<Type> newPromotedTypes;
30713071
if (newWriteCaptured) {
30723072
// Write captured variables can't be promoted.
3073-
newPromotedTypes = null;
3073+
newPromotedTypes = const [];
30743074
} else if (baseModel.ssaNode != thisModel.ssaNode) {
30753075
// The variable may have been written to since `thisModel`, so we can't
30763076
// use any of the promotions from `thisModel`.
@@ -3389,7 +3389,7 @@ class FlowModel<Type extends Object> {
33893389
newTested = PromotionModel._addTypeToUniqueList(info.tested, testedType);
33903390
}
33913391

3392-
List<Type>? newPromotedTypes = info.promotedTypes;
3392+
List<Type> newPromotedTypes = info.promotedTypes;
33933393
if (promotedType != null) {
33943394
newPromotedTypes = PromotionModel._addToPromotedTypes(
33953395
info.promotedTypes,
@@ -3760,7 +3760,7 @@ class PromotionModel<Type extends Object> {
37603760
/// Sequence of types that the variable or property has been promoted to,
37613761
/// where each element of the sequence is a subtype of the previous. Null if
37623762
/// the variable or property hasn't been promoted.
3763-
final List<Type>? promotedTypes;
3763+
final List<Type> promotedTypes;
37643764

37653765
/// List of types that the variable has been tested against in all code paths
37663766
/// leading to the given point in the source code. Not relevant for
@@ -3801,9 +3801,8 @@ class PromotionModel<Type extends Object> {
38013801
!(assigned && unassigned),
38023802
"Can't be both definitely assigned and unassigned",
38033803
);
3804-
assert(promotedTypes == null || promotedTypes!.isNotEmpty);
38053804
assert(
3806-
!writeCaptured || promotedTypes == null,
3805+
!writeCaptured || promotedTypes.isEmpty,
38073806
"Write-captured variables can't be promoted",
38083807
);
38093808
assert(
@@ -3817,7 +3816,7 @@ class PromotionModel<Type extends Object> {
38173816
/// Creates a [PromotionModel] representing a variable or property that's
38183817
/// never been seen before.
38193818
PromotionModel.fresh({this.assigned = false, required this.ssaNode})
3820-
: promotedTypes = null,
3819+
: promotedTypes = const [],
38213820
tested = const [],
38223821
unassigned = !assigned,
38233822
nonPromotionHistory = null;
@@ -3833,7 +3832,7 @@ class PromotionModel<Type extends Object> {
38333832
/// the top of loops whose bodies write to them.
38343833
PromotionModel<Type> discardPromotionsAndMarkNotUnassigned() {
38353834
return new PromotionModel<Type>(
3836-
promotedTypes: null,
3835+
promotedTypes: const [],
38373836
tested: tested,
38383837
assigned: assigned,
38393838
unassigned: false,
@@ -3844,7 +3843,7 @@ class PromotionModel<Type extends Object> {
38443843
@override
38453844
String toString() {
38463845
List<String> parts = [ssaNode.toString()];
3847-
if (promotedTypes != null) {
3846+
if (promotedTypes.isNotEmpty) {
38483847
parts.add('promotedTypes: $promotedTypes');
38493848
}
38503849
if (tested.isNotEmpty) {
@@ -3895,7 +3894,7 @@ class PromotionModel<Type extends Object> {
38953894
helper.typeOperations,
38963895
nonPromotionReason,
38973896
);
3898-
List<Type>? newPromotedTypes = demotionResult.promotedTypes;
3897+
List<Type> newPromotedTypes = demotionResult.promotedTypes;
38993898

39003899
if (promoteToTypeOfInterest) {
39013900
newPromotedTypes = _tryPromoteToTypeOfInterest(
@@ -3918,8 +3917,8 @@ class PromotionModel<Type extends Object> {
39183917
}
39193918

39203919
List<Type> newTested;
3921-
if (newPromotedTypes == null &&
3922-
promotedTypes != null &&
3920+
if (newPromotedTypes.isEmpty &&
3921+
promotedTypes.isNotEmpty &&
39233922
!helper.typeAnalyzerOptions.soundFlowAnalysisEnabled) {
39243923
// A full demotion used to clear types of interest. This behavior was
39253924
// removed as part of the sound-flow-analysis update (see
@@ -3943,7 +3942,7 @@ class PromotionModel<Type extends Object> {
39433942
/// been write-captured.
39443943
PromotionModel<Type> writeCapture() {
39453944
return new PromotionModel<Type>(
3946-
promotedTypes: null,
3945+
promotedTypes: const [],
39473946
tested: const [],
39483947
assigned: assigned,
39493948
unassigned: false,
@@ -3962,14 +3961,14 @@ class PromotionModel<Type extends Object> {
39623961
FlowAnalysisTypeOperations<Type> typeOperations,
39633962
NonPromotionReason? nonPromotionReason,
39643963
) {
3965-
List<Type>? promotedTypes = this.promotedTypes;
3966-
if (promotedTypes == null) {
3967-
return new _DemotionResult<Type>(null, nonPromotionHistory);
3964+
List<Type> promotedTypes = this.promotedTypes;
3965+
if (promotedTypes.isEmpty) {
3966+
return new _DemotionResult<Type>(const [], nonPromotionHistory);
39683967
}
39693968

39703969
int numElementsToKeep = promotedTypes.length;
39713970
NonPromotionHistory<Type>? newNonPromotionHistory = nonPromotionHistory;
3972-
List<Type>? newPromotedTypes;
3971+
List<Type> newPromotedTypes = const [];
39733972
for (; ; numElementsToKeep--) {
39743973
if (numElementsToKeep == 0) {
39753974
break;
@@ -4017,20 +4016,21 @@ class PromotionModel<Type extends Object> {
40174016
///
40184017
/// Note that since promotion chains are considered immutable, if promotion
40194018
/// is required, a new promotion chain will be created and returned.
4020-
List<Type>? _tryPromoteToTypeOfInterest(
4019+
List<Type> _tryPromoteToTypeOfInterest(
40214020
FlowModelHelper<Type> helper,
40224021
Type declaredType,
4023-
List<Type>? promotedTypes,
4022+
List<Type> promotedTypes,
40244023
Type writtenType,
40254024
) {
40264025
assert(!writeCaptured);
40274026

40284027
// Figure out if we have any promotion candidates (types that are a
40294028
// supertype of writtenType and a proper subtype of the currently-promoted
40304029
// type). If at any point we find an exact match, we take it immediately.
4031-
Type currentlyPromotedType = promotedTypes?.last ?? declaredType;
4030+
Type currentlyPromotedType =
4031+
promotedTypes.isNotEmpty ? promotedTypes.last : declaredType;
40324032

4033-
List<Type>? result;
4033+
List<Type>? result = null;
40344034
List<Type>? candidates = null;
40354035

40364036
void handleTypeOfInterest(Type type) {
@@ -4171,7 +4171,7 @@ class PromotionModel<Type extends Object> {
41714171
_PropertySsaNode<Type>? propertySsaNode,
41724172
}) {
41734173
FlowAnalysisTypeOperations<Type> typeOperations = helper.typeOperations;
4174-
List<Type>? newPromotedTypes = joinPromotedTypes(
4174+
List<Type> newPromotedTypes = joinPromotedTypes(
41754175
first.promotedTypes,
41764176
second.promotedTypes,
41774177
typeOperations,
@@ -4208,13 +4208,13 @@ class PromotionModel<Type extends Object> {
42084208
/// chains. Briefly, we intersect given chains. The chains are totally
42094209
/// ordered subsets of a global partial order. Their intersection is a
42104210
/// subset of each, and as such is also totally ordered.
4211-
static List<Type>? joinPromotedTypes<Type extends Object>(
4212-
List<Type>? chain1,
4213-
List<Type>? chain2,
4211+
static List<Type> joinPromotedTypes<Type extends Object>(
4212+
List<Type> chain1,
4213+
List<Type> chain2,
42144214
FlowAnalysisTypeOperations<Type> typeOperations,
42154215
) {
4216-
if (chain1 == null) return chain1;
4217-
if (chain2 == null) return chain2;
4216+
if (chain1.isEmpty) return chain1;
4217+
if (chain2.isEmpty) return chain2;
42184218

42194219
int index1 = 0;
42204220
int index2 = 0;
@@ -4244,7 +4244,7 @@ class PromotionModel<Type extends Object> {
42444244

42454245
if (index1 == chain1.length && !skipped1) return chain1;
42464246
if (index2 == chain2.length && !skipped2) return chain2;
4247-
return result;
4247+
return result ?? const [];
42484248
}
42494249

42504250
/// Performs the portion of the "join" algorithm that applies to promotion
@@ -4294,16 +4294,16 @@ class PromotionModel<Type extends Object> {
42944294
/// In degenerate cases, the returned chain will be identical to
42954295
/// [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-
required List<Type>? basePromotions,
4299-
required List<Type>? newPromotions,
4297+
static List<Type> rebasePromotedTypes<Type extends Object>({
4298+
required List<Type> basePromotions,
4299+
required List<Type> newPromotions,
43004300
required FlowModelHelper<Type> helper,
43014301
}) {
4302-
if (basePromotions == null) {
4302+
if (basePromotions.isEmpty) {
43034303
// The base promotion chain contributes nothing so we just use this
43044304
// promotion chain directly.
43054305
return newPromotions;
4306-
} else if (newPromotions == null) {
4306+
} else if (newPromotions.isEmpty) {
43074307
// This promotion chain contributes nothing so we just use the base
43084308
// promotion chain directly. Note: this is a performance optimization of
43094309
// the `else` block below; it is not required by the spec.
@@ -4335,10 +4335,10 @@ class PromotionModel<Type extends Object> {
43354335
}
43364336

43374337
static List<Type> _addToPromotedTypes<Type extends Object>(
4338-
List<Type>? promotedTypes,
4338+
List<Type> promotedTypes,
43394339
Type promoted,
43404340
) =>
4341-
promotedTypes == null
4341+
promotedTypes.isEmpty
43424342
? [promoted]
43434343
: (promotedTypes.toList()..add(promoted));
43444344

@@ -4355,7 +4355,7 @@ class PromotionModel<Type extends Object> {
43554355
static PromotionModel<Type> _identicalOrNew<Type extends Object>(
43564356
PromotionModel<Type> first,
43574357
PromotionModel<Type> second,
4358-
List<Type>? newPromotedTypes,
4358+
List<Type> newPromotedTypes,
43594359
List<Type> newTested,
43604360
bool newAssigned,
43614361
bool newUnassigned,
@@ -4740,7 +4740,7 @@ class SsaNode<Type extends Object> {
47404740
newFlowModel,
47414741
);
47424742
if (afterFinallyModel == null) continue;
4743-
List<Type>? afterFinallyPromotedTypes = afterFinallyModel.promotedTypes;
4743+
List<Type> afterFinallyPromotedTypes = afterFinallyModel.promotedTypes;
47444744
// The property was accessed in a promotion-relevant way in the `try`
47454745
// block, so we need to apply the promotions from the `finally` block to
47464746
// the flow model from the `try` block, and see what sticks.
@@ -4756,8 +4756,8 @@ class SsaNode<Type extends Object> {
47564756
newModel,
47574757
);
47584758
}
4759-
List<Type>? newPromotedTypes = newModel.promotedTypes;
4760-
List<Type>? rebasedPromotedTypes = PromotionModel.rebasePromotedTypes(
4759+
List<Type> newPromotedTypes = newModel.promotedTypes;
4760+
List<Type> rebasedPromotedTypes = PromotionModel.rebasePromotedTypes(
47614761
basePromotions: newPromotedTypes,
47624762
newPromotions: afterFinallyPromotedTypes,
47634763
helper: helper,
@@ -5090,7 +5090,7 @@ class _ConditionalContext<Type extends Object> extends _BranchContext<Type> {
50905090
/// to another.
50915091
class _DemotionResult<Type extends Object> {
50925092
/// The new set of promoted types.
5093-
final List<Type>? promotedTypes;
5093+
final List<Type> promotedTypes;
50945094

50955095
/// The new non-promotion history (including the types that the variable is
50965096
/// no longer promoted to).
@@ -6185,7 +6185,7 @@ class _FlowAnalysisImpl<
61856185
this,
61866186
promotionKey,
61876187
new PromotionModel(
6188-
promotedTypes: nonNullType == targetType ? null : [nonNullType],
6188+
promotedTypes: nonNullType == targetType ? const [] : [nonNullType],
61896189
tested: const [],
61906190
assigned: true,
61916191
unassigned: false,
@@ -6377,7 +6377,7 @@ class _FlowAnalysisImpl<
63776377
return _current.promotionInfo
63786378
?.get(this, promotionKeyStore.keyForVariable(variable))
63796379
?.promotedTypes
6380-
?.last;
6380+
.lastOrNull;
63816381
}
63826382

63836383
@override
@@ -6544,7 +6544,7 @@ class _FlowAnalysisImpl<
65446544
);
65456545
if (promotionInfo == null) return const [];
65466546
assert(promotionInfo.ssaNode == propertySsaNode);
6547-
return promotionInfo.promotedTypes ?? const [];
6547+
return promotionInfo.promotedTypes;
65486548
}
65496549

65506550
@override
@@ -6881,7 +6881,7 @@ class _FlowAnalysisImpl<
68816881
).addPreviousInfo(promotionModel.ssaNode?.expressionInfo, this, _current);
68826882
_storeExpressionReference(expression, expressionInfo);
68836883
_storeExpressionInfo(expression, expressionInfo);
6884-
return promotionModel.promotedTypes?.last;
6884+
return promotionModel.promotedTypes.lastOrNull;
68856885
}
68866886

68876887
@override
@@ -7109,7 +7109,7 @@ class _FlowAnalysisImpl<
71097109
return _current.promotionInfo
71107110
?.get(this, context._matchedValueInfo.promotionKey)
71117111
?.promotedTypes
7112-
?.last ??
7112+
.lastOrNull ??
71137113
context._matchedValueInfo._type;
71147114
}
71157115

@@ -7133,8 +7133,8 @@ class _FlowAnalysisImpl<
71337133
ssaNode.promotionKey,
71347134
ssaNode: ssaNode,
71357135
);
7136-
List<Type>? promotedTypes = previousPromotionInfo.promotedTypes;
7137-
if (promotedTypes != null) {
7136+
List<Type> promotedTypes = previousPromotionInfo.promotedTypes;
7137+
if (promotedTypes.isNotEmpty) {
71387138
(allPreviouslyPromotedTypes ??= []).add(promotedTypes);
71397139
}
71407140
ssaNode = ssaNode.previousSsaNode;
@@ -7171,8 +7171,8 @@ class _FlowAnalysisImpl<
71717171
reference.promotionKey,
71727172
);
71737173
if (variable == null) {
7174-
List<Type>? promotedTypes = currentPromotionInfo.promotedTypes;
7175-
if (promotedTypes != null) {
7174+
List<Type> promotedTypes = currentPromotionInfo.promotedTypes;
7175+
if (promotedTypes.isNotEmpty) {
71767176
return () {
71777177
Map<Type, NonPromotionReason> result = <Type, NonPromotionReason>{};
71787178
for (Type type in promotedTypes) {
@@ -7185,7 +7185,7 @@ class _FlowAnalysisImpl<
71857185
return () {
71867186
Map<Type, NonPromotionReason> result = <Type, NonPromotionReason>{};
71877187
Type currentType =
7188-
currentPromotionInfo.promotedTypes?.last ??
7188+
currentPromotionInfo.promotedTypes.lastOrNull ??
71897189
operations.variableType(variable);
71907190
NonPromotionHistory<Type>? nonPromotionHistory =
71917191
currentPromotionInfo.nonPromotionHistory;
@@ -7332,7 +7332,7 @@ class _FlowAnalysisImpl<
73327332
if (promotionInfo != null) {
73337333
assert(promotionInfo.ssaNode == propertySsaNode);
73347334
}
7335-
promotedType = promotionInfo?.promotedTypes?.last;
7335+
promotedType = promotionInfo?.promotedTypes.lastOrNull;
73367336
if (promotedType != null &&
73377337
!operations.isSubtypeOf(promotedType, unpromotedType)) {
73387338
promotedType = null;
@@ -7449,7 +7449,7 @@ class _FlowAnalysisImpl<
74497449
this,
74507450
promotionKey,
74517451
new PromotionModel(
7452-
promotedTypes: null,
7452+
promotedTypes: const [],
74537453
tested: const [],
74547454
assigned: true,
74557455
unassigned: false,
@@ -7625,7 +7625,7 @@ class _FlowAnalysisImpl<
76257625
return new TrivialVariableReference<Type>(
76267626
promotionKey: variableKey,
76277627
model: _current,
7628-
type: info.promotedTypes?.last ?? unpromotedType,
7628+
type: info.promotedTypes.lastOrNull ?? unpromotedType,
76297629
isThisOrSuper: false,
76307630
ssaNode: info.ssaNode ?? new SsaNode<Type>(null),
76317631
);

0 commit comments

Comments
 (0)