Skip to content

Commit c2e78e8

Browse files
stereotype441commit-bot@chromium.org
stereotype441
authored and
[email protected]
committed
Flow analysis: fix and unit test joinPromoted.
I will add further unit tests of flow analysis in follow-up CLs. Note that flow_analysis_unit_test.dart does not make use of the test_reflective_loader package because the intent is to move these tests into the front end, and front end tests are written using vanilla package:test conventions. Change-Id: I10ba27212a1986f0c1fd960d33105f281712b506 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/109760 Reviewed-by: Konstantin Shcheglov <[email protected]> Commit-Queue: Paul Berry <[email protected]>
1 parent 7976bea commit c2e78e8

File tree

3 files changed

+218
-37
lines changed

3 files changed

+218
-37
lines changed

‎pkg/analyzer/lib/src/dart/resolver/flow_analysis.dart‎

Lines changed: 41 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22
// for details. All rights reserved. Use of this source code is governed by a
33
// BSD-style license that can be found in the LICENSE file.
44

5+
import 'package:meta/meta.dart';
6+
57
/// Sets of local variables that are potentially assigned in a statement.
68
///
79
/// These statements are loops, `switch`, and `try` statements.
@@ -387,6 +389,44 @@ class FlowAnalysis<Statement, Expression, Variable, Type> {
387389
return !_current.notNullable.contains(variable);
388390
}
389391

392+
@visibleForTesting
393+
Map<Variable, Type> joinPromoted(
394+
Map<Variable, Type> first,
395+
Map<Variable, Type> second,
396+
) {
397+
if (identical(first, second)) return first;
398+
if (first.isEmpty || second.isEmpty) return const {};
399+
400+
var result = <Variable, Type>{};
401+
var alwaysFirst = true;
402+
var alwaysSecond = true;
403+
for (var variable in first.keys) {
404+
var firstType = first[variable];
405+
var secondType = second[variable];
406+
if (secondType != null) {
407+
if (identical(firstType, secondType)) {
408+
result[variable] = firstType;
409+
} else if (typeOperations.isSubtypeOf(firstType, secondType)) {
410+
result[variable] = secondType;
411+
alwaysFirst = false;
412+
} else if (typeOperations.isSubtypeOf(secondType, firstType)) {
413+
result[variable] = firstType;
414+
alwaysSecond = false;
415+
} else {
416+
alwaysFirst = false;
417+
alwaysSecond = false;
418+
}
419+
} else {
420+
alwaysFirst = false;
421+
}
422+
}
423+
424+
if (alwaysFirst) return first;
425+
if (alwaysSecond && result.length == second.length) return second;
426+
if (result.isEmpty) return const {};
427+
return result;
428+
}
429+
390430
void logicalAnd_end(Expression andExpression, Expression rightOperand) {
391431
_conditionalEnd(rightOperand);
392432
// Tail of the stack: falseLeft, trueLeft, falseRight, trueRight
@@ -604,7 +644,7 @@ class FlowAnalysis<Statement, Expression, Variable, Type> {
604644
var newNotAssigned = first.notAssigned.union(second.notAssigned);
605645
var newNotNullable = first.notNullable.union(second.notNullable);
606646
var newNotNonNullable = first.notNonNullable.union(second.notNonNullable);
607-
var newPromoted = _joinPromoted(first.promoted, second.promoted);
647+
var newPromoted = joinPromoted(first.promoted, second.promoted);
608648

609649
return _State._identicalOrNew(
610650
first,
@@ -616,42 +656,6 @@ class FlowAnalysis<Statement, Expression, Variable, Type> {
616656
newPromoted,
617657
);
618658
}
619-
620-
Map<Variable, Type> _joinPromoted(
621-
Map<Variable, Type> first,
622-
Map<Variable, Type> second,
623-
) {
624-
if (identical(first, second)) return first;
625-
if (first.isEmpty || second.isEmpty) return const {};
626-
627-
var result = <Variable, Type>{};
628-
var alwaysFirst = true;
629-
var alwaysSecond = true;
630-
for (var variable in first.keys) {
631-
var firstType = first[variable];
632-
var secondType = second[variable];
633-
if (firstType != null && secondType != null) {
634-
if (typeOperations.isSubtypeOf(firstType, secondType)) {
635-
result[variable] = secondType;
636-
alwaysFirst = false;
637-
} else if (typeOperations.isSubtypeOf(secondType, firstType)) {
638-
result[variable] = firstType;
639-
alwaysSecond = false;
640-
} else {
641-
alwaysFirst = false;
642-
alwaysSecond = false;
643-
}
644-
} else {
645-
alwaysFirst = false;
646-
alwaysSecond = false;
647-
}
648-
}
649-
650-
if (alwaysFirst) return first;
651-
if (alwaysSecond) return second;
652-
if (result.isEmpty) return const {};
653-
return result;
654-
}
655659
}
656660

657661
/// Accessor for function body information.

‎pkg/analyzer/test/src/dart/resolution/flow_analysis_unit_test.dart‎

Lines changed: 175 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,175 @@
1+
// Copyright (c) 2019, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
import 'package:analyzer/src/dart/resolver/flow_analysis.dart';
6+
import 'package:test/test.dart';
7+
8+
main() {
9+
group('join', () {
10+
group('should re-use an input if possible', () {
11+
var x = _Var('x', null);
12+
var y = _Var('y', null);
13+
var intType = _Type('int');
14+
var intQType = _Type('int?');
15+
var stringType = _Type('String');
16+
const emptyMap = <Null, Null>{};
17+
18+
test('identical inputs', () {
19+
var flow = _Harness().flow;
20+
var p = {x: intType, y: stringType};
21+
expect(flow.joinPromoted(p, p), same(p));
22+
});
23+
24+
test('one input empty', () {
25+
var flow = _Harness().flow;
26+
var p1 = {x: intType, y: stringType};
27+
var p2 = <_Var, _Type>{};
28+
expect(flow.joinPromoted(p1, p2), same(emptyMap));
29+
expect(flow.joinPromoted(p2, p1), same(emptyMap));
30+
});
31+
32+
test('related types', () {
33+
var flow = _Harness().flow;
34+
var p1 = {x: intType};
35+
var p2 = {x: intQType};
36+
expect(flow.joinPromoted(p1, p2), same(p2));
37+
expect(flow.joinPromoted(p2, p1), same(p2));
38+
});
39+
40+
test('unrelated types', () {
41+
var flow = _Harness().flow;
42+
var p1 = {x: intType};
43+
var p2 = {x: stringType};
44+
expect(flow.joinPromoted(p1, p2), same(emptyMap));
45+
expect(flow.joinPromoted(p2, p1), same(emptyMap));
46+
});
47+
48+
test('sub-map', () {
49+
var flow = _Harness().flow;
50+
var p1 = {x: intType, y: stringType};
51+
var p2 = {x: intType};
52+
expect(flow.joinPromoted(p1, p2), same(p2));
53+
expect(flow.joinPromoted(p2, p1), same(p2));
54+
});
55+
56+
test('sub-map with matched subtype', () {
57+
var flow = _Harness().flow;
58+
var p1 = {x: intType, y: stringType};
59+
var p2 = {x: intQType};
60+
expect(flow.joinPromoted(p1, p2), same(p2));
61+
expect(flow.joinPromoted(p2, p1), same(p2));
62+
});
63+
64+
test('sub-map with mismatched subtype', () {
65+
var flow = _Harness().flow;
66+
var p1 = {x: intQType, y: stringType};
67+
var p2 = {x: intType};
68+
var join12 = flow.joinPromoted(p1, p2);
69+
_Type.allowComparisons(() => expect(join12, {x: intQType}));
70+
var join21 = flow.joinPromoted(p2, p1);
71+
_Type.allowComparisons(() => expect(join21, {x: intQType}));
72+
});
73+
});
74+
});
75+
}
76+
77+
class _Expression {}
78+
79+
class _Harness
80+
implements
81+
NodeOperations<_Expression>,
82+
TypeOperations<_Var, _Type>,
83+
FunctionBodyAccess<_Var> {
84+
FlowAnalysis<_Statement, _Expression, _Var, _Type> flow;
85+
86+
_Harness() {
87+
flow = FlowAnalysis<_Statement, _Expression, _Var, _Type>(this, this, this);
88+
}
89+
90+
@override
91+
bool isLocalVariable(_Var variable) {
92+
throw UnimplementedError('TODO(paulberry)');
93+
}
94+
95+
@override
96+
bool isPotentiallyMutatedInClosure(_Var variable) {
97+
// TODO(paulberry): make tests where this returns true
98+
return false;
99+
}
100+
101+
@override
102+
bool isPotentiallyMutatedInScope(_Var variable) {
103+
throw UnimplementedError('TODO(paulberry)');
104+
}
105+
106+
@override
107+
bool isSubtypeOf(_Type leftType, _Type rightType) {
108+
const Map<String, bool> _subtypes = const {
109+
'int <: int?': true,
110+
'int <: String': false,
111+
'int? <: int': false,
112+
'String <: int': false,
113+
};
114+
115+
if (leftType.type == rightType.type) return true;
116+
var query = '$leftType <: $rightType';
117+
return _subtypes[query] ?? fail('Unknown subtype query: $query');
118+
}
119+
120+
@override
121+
_Expression unwrapParenthesized(_Expression node) {
122+
// TODO(paulberry): test cases where this matters
123+
return node;
124+
}
125+
126+
@override
127+
_Type variableType(_Var variable) {
128+
return variable.type;
129+
}
130+
}
131+
132+
class _Statement {}
133+
134+
class _Type {
135+
static bool _allowingTypeComparisons = false;
136+
137+
final String type;
138+
139+
_Type(this.type);
140+
141+
@override
142+
bool operator ==(Object other) {
143+
if (_allowingTypeComparisons) {
144+
return other is _Type && other.type == this.type;
145+
} else {
146+
// The flow analysis engine should not compare types using operator==. It
147+
// should compare them using TypeOperations.
148+
fail('Unexpected use of operator== on types');
149+
}
150+
}
151+
152+
@override
153+
String toString() => type;
154+
155+
static T allowComparisons<T>(T callback()) {
156+
var oldAllowingTypeComparisons = _allowingTypeComparisons;
157+
_allowingTypeComparisons = true;
158+
try {
159+
return callback();
160+
} finally {
161+
_allowingTypeComparisons = oldAllowingTypeComparisons;
162+
}
163+
}
164+
}
165+
166+
class _Var {
167+
final String name;
168+
169+
final _Type type;
170+
171+
_Var(this.name, this.type);
172+
173+
@override
174+
String toString() => '$type $name';
175+
}

‎pkg/analyzer/test/src/dart/resolution/test_all.dart‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import 'export_test.dart' as export_;
1616
import 'extension_method_test.dart' as extension_method;
1717
import 'extension_override_test.dart' as extension_override;
1818
import 'flow_analysis_test.dart' as flow_analysis;
19+
import 'flow_analysis_unit_test.dart' as flow_analysis_unit;
1920
import 'for_element_test.dart' as for_element;
2021
import 'for_in_test.dart' as for_in;
2122
import 'function_expression_invocation_test.dart'
@@ -55,6 +56,7 @@ main() {
5556
extension_method.main();
5657
extension_override.main();
5758
flow_analysis.main();
59+
flow_analysis_unit.main();
5860
for_element.main();
5961
for_in.main();
6062
function_expression_invocation.main();

0 commit comments

Comments
 (0)