Skip to content

Commit d61816d

Browse files
committed
Flow analysis: store all variables in promoted map.
Previously we only stored promoted variables. Storing all variables paves the way for expanding the `promoted` map into a map from variable to variable state, which will be necessary to bring the implementation in line with dart-lang/language#473.
1 parent 8a8c62f commit d61816d

File tree

2 files changed

+60
-48
lines changed

2 files changed

+60
-48
lines changed

pkg/front_end/lib/src/fasta/flow_analysis/flow_analysis.dart

Lines changed: 51 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -301,8 +301,11 @@ class FlowAnalysis<Statement, Expression, Variable, Type> {
301301
_stack.add(_current);
302302

303303
Set<Variable> notPromoted = null;
304-
for (var variable in _current.promoted.keys) {
305-
if (functionBody.isPotentiallyMutatedInScope(variable)) {
304+
for (var entry in _current.promoted.entries) {
305+
var variable = entry.key;
306+
var promotedType = entry.value;
307+
if (promotedType != null &&
308+
functionBody.isPotentiallyMutatedInScope(variable)) {
306309
notPromoted ??= Set<Variable>.identity();
307310
notPromoted.add(variable);
308311
}
@@ -644,19 +647,18 @@ class FlowModel<Variable, Type> {
644647
/// the control flow.
645648
final _VariableSet<Variable> notAssigned;
646649

647-
/// For each variable whose type is promoted at this point in the control
648-
/// flow, the promoted type. Variables whose type is not promoted are not
649-
/// present in the map.
650+
/// For each variable being tracked by flow analysis, the variable's promoted
651+
/// type, or `null` if the variable's type is not promoted.
650652
///
651653
/// Flow analysis has no awareness of scope, so variables that are out of
652-
/// scope are retained in the map until such time as their promotions would
653-
/// have been lost, if their scope had extended to the entire function being
654-
/// analyzed. So, for example, if a variable is declared and then promoted
655-
/// inside the `then` branch of an `if` statement, and the `else` branch of
656-
/// the `if` statement ends in a `return` statement, then the promotion
657-
/// remains in the map after the `if` statement ends. This should not have
658-
/// any effect on analysis results for error-free code, because it is an error
659-
/// to refer to a variable that is no longer in scope.
654+
/// scope are retained in the map until such time as their declaration no
655+
/// longer dominates the control flow. So, for example, if a variable is
656+
/// declared and then promoted inside the `then` branch of an `if` statement,
657+
/// and the `else` branch of the `if` statement ends in a `return` statement,
658+
/// then the variable remains in the map after the `if` statement ends, even
659+
/// though the variable is not in scope anymore. This should not have any
660+
/// effect on analysis results for error-free code, because it is an error to
661+
/// refer to a variable that is no longer in scope.
660662
final Map<Variable, Type> promoted;
661663

662664
/// Creates a state object with the given [reachable] status. All variables
@@ -680,15 +682,13 @@ class FlowModel<Variable, Type> {
680682
/// the point of declaration.
681683
FlowModel<Variable, Type> add(Variable variable, {bool assigned: false}) {
682684
var newNotAssigned = assigned ? notAssigned : notAssigned.add(variable);
683-
684-
if (identical(newNotAssigned, notAssigned)) {
685-
return this;
686-
}
685+
var newPromoted = Map<Variable, Type>.from(promoted);
686+
newPromoted[variable] = null;
687687

688688
return FlowModel<Variable, Type>._(
689689
reachable,
690690
newNotAssigned,
691-
promoted,
691+
newPromoted,
692692
);
693693
}
694694

@@ -819,9 +819,9 @@ class FlowModel<Variable, Type> {
819819
var newPromoted = <Variable, Type>{};
820820
bool promotedMatchesThis = true;
821821
bool promotedMatchesOther = true;
822-
for (var variable in Set<Variable>.from(promoted.keys)
823-
..addAll(other.promoted.keys)) {
824-
var thisType = promoted[variable];
822+
for (var entry in promoted.entries) {
823+
var variable = entry.key;
824+
var thisType = entry.value;
825825
var otherType = other.promoted[variable];
826826
if (!unsafe.contains(variable)) {
827827
if (otherType != null &&
@@ -844,6 +844,7 @@ class FlowModel<Variable, Type> {
844844
promotedMatchesOther = false;
845845
}
846846
} else {
847+
newPromoted[variable] = null;
847848
if (promotedMatchesOther && otherType != null) {
848849
promotedMatchesOther = false;
849850
}
@@ -912,16 +913,10 @@ class FlowModel<Variable, Type> {
912913
/// immutable.
913914
Map<Variable, Type> _removePromoted(
914915
Map<Variable, Type> map, Variable variable) {
915-
if (map.isEmpty) return const {};
916-
917-
var result = <Variable, Type>{};
918-
for (var key in map.keys) {
919-
if (!identical(key, variable)) {
920-
result[key] = map[key];
921-
}
922-
}
916+
if (map[variable] == null) return map;
923917

924-
if (result.isEmpty) return const {};
918+
var result = Map<Variable, Type>.from(map);
919+
result[variable] = null;
925920
return result;
926921
}
927922

@@ -936,11 +931,14 @@ class FlowModel<Variable, Type> {
936931

937932
var result = <Variable, Type>{};
938933
var noChanges = true;
939-
for (var key in map.keys) {
940-
if (variables.contains(key)) {
934+
for (var entry in map.entries) {
935+
var variable = entry.key;
936+
var promotedType = entry.value;
937+
if (variables.contains(variable) && promotedType != null) {
938+
result[variable] = null;
941939
noChanges = false;
942940
} else {
943-
result[key] = map[key];
941+
result[variable] = promotedType;
944942
}
945943
}
946944

@@ -996,24 +994,32 @@ class FlowModel<Variable, Type> {
996994
var result = <Variable, Type>{};
997995
var alwaysFirst = true;
998996
var alwaysSecond = true;
999-
for (var variable in first.keys) {
1000-
var firstType = first[variable];
1001-
var secondType = second[variable];
1002-
if (secondType != null) {
997+
for (var entry in first.entries) {
998+
var variable = entry.key;
999+
if (!second.containsKey(variable)) {
1000+
alwaysFirst = false;
1001+
} else {
1002+
var firstType = entry.value;
1003+
var secondType = second[variable];
10031004
if (identical(firstType, secondType)) {
10041005
result[variable] = firstType;
1006+
} else if (firstType == null) {
1007+
result[variable] = null;
1008+
alwaysSecond = false;
1009+
} else if (secondType == null) {
1010+
result[variable] = null;
1011+
alwaysFirst = false;
10051012
} else if (typeOperations.isSubtypeOf(firstType, secondType)) {
10061013
result[variable] = secondType;
10071014
alwaysFirst = false;
10081015
} else if (typeOperations.isSubtypeOf(secondType, firstType)) {
10091016
result[variable] = firstType;
10101017
alwaysSecond = false;
10111018
} else {
1019+
result[variable] = null;
10121020
alwaysFirst = false;
10131021
alwaysSecond = false;
10141022
}
1015-
} else {
1016-
alwaysFirst = false;
10171023
}
10181024
}
10191025

@@ -1060,7 +1066,12 @@ class FlowModel<Variable, Type> {
10601066
for (var entry in p1.entries) {
10611067
var p1Value = entry.value;
10621068
var p2Value = p2[entry.key];
1063-
if (!typeOperations.isSameType(p1Value, p2Value)) return false;
1069+
if (p1Value == null) {
1070+
if (p2Value != null) return false;
1071+
} else {
1072+
if (p2Value == null) return false;
1073+
if (!typeOperations.isSameType(p1Value, p2Value)) return false;
1074+
}
10641075
}
10651076
return true;
10661077
}

pkg/front_end/test/fasta/flow_analysis/flow_analysis_test.dart

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -159,22 +159,22 @@ main() {
159159
var s2 = s1.add(intVar);
160160
expect(s2.notAssigned.contains(intVar), true);
161161
expect(s2.reachable, true);
162-
expect(s2.promoted, same(s1.promoted));
162+
expect(s2.promoted, {intVar: null});
163163
});
164164

165165
test('unassigned', () {
166166
var s1 = FlowModel<_Var, _Type>(true);
167167
var s2 = s1.add(intVar, assigned: false);
168168
expect(s2.notAssigned.contains(intVar), true);
169169
expect(s2.reachable, true);
170-
expect(s2.promoted, same(s1.promoted));
170+
expect(s2.promoted, {intVar: null});
171171
});
172172

173173
test('assigned', () {
174174
var s1 = FlowModel<_Var, _Type>(true);
175175
var s2 = s1.add(intVar, assigned: true);
176176
expect(s2.notAssigned.contains(intVar), false);
177-
expect(s2, same(s1));
177+
expect(s2.promoted, {intVar: null});
178178
});
179179
});
180180

@@ -279,7 +279,7 @@ main() {
279279
var s2 = s1.write(h, emptySet, objectQVar);
280280
expect(s2.reachable, true);
281281
expect(s2.notAssigned, same(s1.notAssigned));
282-
expect(s2.promoted, isEmpty);
282+
expect(s2.promoted, {objectQVar: null});
283283
});
284284
});
285285

@@ -345,7 +345,7 @@ main() {
345345
expect(s2.reachable, true);
346346
expect(s2.notAssigned, same(s1.notAssigned));
347347
_Type.allowComparisons(() {
348-
expect(s2.promoted, {objectQVar: _Type('int')});
348+
expect(s2.promoted, {objectQVar: _Type('int'), intQVar: null});
349349
});
350350
});
351351
});
@@ -392,7 +392,8 @@ main() {
392392
var result =
393393
s1.restrict(h, emptySet, s2, unsafe ? [x].toSet() : Set());
394394
if (expectedType == null) {
395-
expect(result.promoted, isNot(contains(x)));
395+
expect(result.promoted, contains(x));
396+
expect(result.promoted[x], isNull);
396397
} else {
397398
expect(result.promoted[x].type, expectedType);
398399
}
@@ -449,8 +450,8 @@ main() {
449450
var h = _Harness();
450451
var p1 = {x: intType};
451452
var p2 = {x: stringType};
452-
expect(FlowModel.joinPromoted(h, p1, p2), same(emptyMap));
453-
expect(FlowModel.joinPromoted(h, p2, p1), same(emptyMap));
453+
expect(FlowModel.joinPromoted(h, p1, p2), {x: null});
454+
expect(FlowModel.joinPromoted(h, p2, p1), {x: null});
454455
});
455456

456457
test('sub-map', () {

0 commit comments

Comments
 (0)