Skip to content

Commit fd60a3e

Browse files
stereotype441commit-bot@chromium.org
stereotype441
authored and
[email protected]
committed
Flow analysis: generalize handling of ==.
This moves the logic for detecting the patterns `variable == null` and `null == variable` into flow analysis, so that they don't need to be replicated in each client. It also opens the door to potential future improvements (e.g. allowing `x == expr` to promote `x` to non-nullable if expr has a non-nullable type). Change-Id: I51fd9df822e3df1eb8bad5884f767c8b61c496ef Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/120934 Reviewed-by: Konstantin Shcheglov <[email protected]> Commit-Queue: Paul Berry <[email protected]>
1 parent 3dc60da commit fd60a3e

File tree

7 files changed

+193
-78
lines changed

7 files changed

+193
-78
lines changed

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

Lines changed: 1 addition & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ import 'package:analyzer/dart/element/type_system.dart';
1010
import 'package:analyzer/src/dart/element/type.dart';
1111
import 'package:analyzer/src/generated/variable_type_provider.dart';
1212
import 'package:front_end/src/fasta/flow_analysis/flow_analysis.dart';
13-
import 'package:meta/meta.dart';
1413

1514
class AnalyzerNodeOperations implements NodeOperations<Expression> {
1615
const AnalyzerNodeOperations();
@@ -77,28 +76,6 @@ class FlowAnalysisHelper {
7776
flow.write(localElement);
7877
}
7978

80-
void binaryExpression_equal(
81-
BinaryExpression node, Expression left, Expression right,
82-
{@required bool notEqual}) {
83-
if (flow == null) return;
84-
85-
if (right is NullLiteral) {
86-
if (left is SimpleIdentifier) {
87-
var element = left.staticElement;
88-
if (element is VariableElement) {
89-
flow.conditionEqNull(node, element, notEqual: notEqual);
90-
}
91-
}
92-
} else if (left is NullLiteral) {
93-
if (right is SimpleIdentifier) {
94-
var element = right.staticElement;
95-
if (element is VariableElement) {
96-
flow.conditionEqNull(node, element, notEqual: notEqual);
97-
}
98-
}
99-
}
100-
}
101-
10279
void breakStatement(BreakStatement node) {
10380
var target = getLabelTarget(node, node.label?.staticElement);
10481
flow.handleBreak(target);
@@ -513,7 +490,7 @@ class _LocalVariableTypeProvider implements LocalVariableTypeProvider {
513490
DartType getType(SimpleIdentifier node) {
514491
var variable = node.staticElement as VariableElement;
515492
if (variable is PromotableElement) {
516-
var promotedType = _manager.flow?.promotedType(variable);
493+
var promotedType = _manager.flow?.variableRead(node, variable);
517494
if (promotedType != null) return promotedType;
518495
}
519496
return variable.type;

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

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3476,9 +3476,10 @@ class ResolverVisitor extends ScopedVisitor {
34763476
node.accept(elementResolver);
34773477
} else if (operator == TokenType.BANG_EQ || operator == TokenType.EQ_EQ) {
34783478
left.accept(this);
3479+
_flowAnalysis?.flow?.equalityOp_rightBegin(left);
34793480
right.accept(this);
34803481
node.accept(elementResolver);
3481-
_flowAnalysis?.binaryExpression_equal(node, left, right,
3482+
_flowAnalysis?.flow?.equalityOp_end(node, right,
34823483
notEqual: operator == TokenType.BANG_EQ);
34833484
} else {
34843485
if (operator == TokenType.QUESTION_QUESTION) {
@@ -4334,6 +4335,12 @@ class ResolverVisitor extends ScopedVisitor {
43344335
node.accept(typeAnalyzer);
43354336
}
43364337

4338+
@override
4339+
void visitNullLiteral(NullLiteral node) {
4340+
_flowAnalysis?.flow?.nullLiteral(node);
4341+
super.visitNullLiteral(node);
4342+
}
4343+
43374344
@override
43384345
void visitParenthesizedExpression(ParenthesizedExpression node) {
43394346
InferenceContext.setTypeFromNode(node.expression, node);

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

Lines changed: 84 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -250,19 +250,6 @@ class FlowAnalysis<Statement, Expression, Variable, Type> {
250250
_current = conditionInfo._ifTrue;
251251
}
252252

253-
/// The [binaryExpression] checks that the [variable] is, or is not, equal to
254-
/// `null`.
255-
void conditionEqNull(Expression binaryExpression, Variable variable,
256-
{bool notEqual: false}) {
257-
FlowModel<Variable, Type> ifNotNull =
258-
_current.markNonNullable(typeOperations, variable);
259-
_storeExpressionInfo(
260-
binaryExpression,
261-
notEqual
262-
? new _ExpressionInfo(_current, ifNotNull, _current)
263-
: new _ExpressionInfo(_current, _current, ifNotNull));
264-
}
265-
266253
void doStatement_bodyBegin(Statement doStatement,
267254
Iterable<Variable> loopAssigned, Iterable<Variable> loopCaptured) {
268255
_BranchTargetContext<Variable, Type> context =
@@ -284,6 +271,39 @@ class FlowAnalysis<Statement, Expression, Variable, Type> {
284271
_current = _join(_expressionEnd(condition)._ifFalse, context._breakModel);
285272
}
286273

274+
/// Call this method just after visiting a binary `==` or `!=` expression.
275+
void equalityOp_end(Expression wholeExpression, Expression rightOperand,
276+
{bool notEqual = false}) {
277+
_BranchContext<Variable, Type> context =
278+
_stack.removeLast() as _BranchContext<Variable, Type>;
279+
_ExpressionInfo<Variable, Type> lhsInfo = context._conditionInfo;
280+
_ExpressionInfo<Variable, Type> rhsInfo = _getExpressionInfo(rightOperand);
281+
Variable variable;
282+
if (lhsInfo is _NullInfo<Variable, Type> &&
283+
rhsInfo is _VariableReadInfo<Variable, Type>) {
284+
variable = rhsInfo._variable;
285+
} else if (rhsInfo is _NullInfo<Variable, Type> &&
286+
lhsInfo is _VariableReadInfo<Variable, Type>) {
287+
variable = lhsInfo._variable;
288+
} else {
289+
return;
290+
}
291+
FlowModel<Variable, Type> ifNotNull =
292+
_current.markNonNullable(typeOperations, variable);
293+
_storeExpressionInfo(
294+
wholeExpression,
295+
notEqual
296+
? new _ExpressionInfo(_current, ifNotNull, _current)
297+
: new _ExpressionInfo(_current, _current, ifNotNull));
298+
}
299+
300+
/// Call this method just after visiting the left hand side of a binary `==`
301+
/// or `!=` expression.
302+
void equalityOp_rightBegin(Expression leftOperand) {
303+
_stack.add(
304+
new _BranchContext<Variable, Type>(_getExpressionInfo(leftOperand)));
305+
}
306+
287307
/// This method should be called at the conclusion of flow analysis for a top
288308
/// level function or method. Performs assertion checks.
289309
void finish() {
@@ -527,8 +547,16 @@ class FlowAnalysis<Statement, Expression, Variable, Type> {
527547
conditionInfo._ifTrue));
528548
}
529549

550+
/// Call this method when encountering an expression that is a `null` literal.
551+
void nullLiteral(Expression expression) {
552+
_storeExpressionInfo(expression, new _NullInfo(_current));
553+
}
554+
530555
/// Retrieves the type that the [variable] is promoted to, if the [variable]
531556
/// is currently promoted. Otherwise returns `null`.
557+
///
558+
/// For testing only. Please use [variableRead] instead.
559+
@visibleForTesting
532560
Type promotedType(Variable variable) {
533561
return _current.infoFor(variable).promotedType;
534562
}
@@ -641,6 +669,16 @@ class FlowAnalysis<Statement, Expression, Variable, Type> {
641669
context._previous.removePromotedAll(assignedInBody, capturedInBody));
642670
}
643671

672+
/// Call this method when encountering an expression that reads the value of
673+
/// a variable.
674+
///
675+
/// If the variable's type is currently promoted, the promoted type is
676+
/// returned. Otherwise `null` is returned.
677+
Type variableRead(Expression expression, Variable variable) {
678+
_storeExpressionInfo(expression, new _VariableReadInfo(_current, variable));
679+
return _current.infoFor(variable).promotedType;
680+
}
681+
644682
void whileStatement_bodyBegin(
645683
Statement whileStatement, Expression condition) {
646684
_ExpressionInfo<Variable, Type> conditionInfo = _expressionEnd(condition);
@@ -1265,6 +1303,20 @@ class _IfContext<Variable, Type> extends _BranchContext<Variable, Type> {
12651303
: super(conditionInfo);
12661304
}
12671305

1306+
/// [_ExpressionInfo] representing a `null` literal.
1307+
class _NullInfo<Variable, Type> implements _ExpressionInfo<Variable, Type> {
1308+
@override
1309+
final FlowModel<Variable, Type> _after;
1310+
1311+
_NullInfo(this._after);
1312+
1313+
@override
1314+
FlowModel<Variable, Type> get _ifFalse => _after;
1315+
1316+
@override
1317+
FlowModel<Variable, Type> get _ifTrue => _after;
1318+
}
1319+
12681320
/// [_FlowContext] representing a language construct for which flow analysis
12691321
/// must store a flow model state to be retrieved later, such as a `try`
12701322
/// statement, function expression, or "if-null" (`??`) expression.
@@ -1309,6 +1361,25 @@ class _TryContext<Variable, Type> extends _SimpleContext<Variable, Type> {
13091361
_TryContext(FlowModel<Variable, Type> previous) : super(previous);
13101362
}
13111363

1364+
/// [_ExpressionInfo] representing an expression that reads the value of a
1365+
/// variable.
1366+
class _VariableReadInfo<Variable, Type>
1367+
implements _ExpressionInfo<Variable, Type> {
1368+
@override
1369+
final FlowModel<Variable, Type> _after;
1370+
1371+
/// The variable that is being read.
1372+
final Variable _variable;
1373+
1374+
_VariableReadInfo(this._after, this._variable);
1375+
1376+
@override
1377+
FlowModel<Variable, Type> get _ifFalse => _after;
1378+
1379+
@override
1380+
FlowModel<Variable, Type> get _ifTrue => _after;
1381+
}
1382+
13121383
/// [_FlowContext] representing a `while` loop (or a C-style `for` loop, which
13131384
/// is functionally similar).
13141385
class _WhileContext<Variable, Type>

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

Lines changed: 66 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -105,13 +105,18 @@ main() {
105105
});
106106
});
107107

108-
test('conditionEqNull(notEqual: true) promotes true branch', () {
108+
test('equalityOp(x != null) promotes true branch', () {
109109
var h = _Harness();
110110
var x = h.addVar('x', 'int?');
111111
h.run((flow) {
112112
h.declare(x, initialized: true);
113+
var varExpr = _Expression();
114+
flow.variableRead(varExpr, x);
115+
flow.equalityOp_rightBegin(varExpr);
116+
var nullExpr = _Expression();
117+
flow.nullLiteral(nullExpr);
113118
var expr = _Expression();
114-
flow.conditionEqNull(expr, x, notEqual: true);
119+
flow.equalityOp_end(expr, nullExpr, notEqual: true);
115120
flow.ifStatement_thenBegin(expr);
116121
expect(flow.promotedType(x).type, 'int');
117122
flow.ifStatement_elseBegin();
@@ -120,13 +125,58 @@ main() {
120125
});
121126
});
122127

123-
test('conditionEqNull(notEqual: false) promotes false branch', () {
128+
test('equalityOp(x == null) promotes false branch', () {
124129
var h = _Harness();
125130
var x = h.addVar('x', 'int?');
126131
h.run((flow) {
127132
h.declare(x, initialized: true);
133+
var varExpr = _Expression();
134+
flow.variableRead(varExpr, x);
135+
flow.equalityOp_rightBegin(varExpr);
136+
var nullExpr = _Expression();
137+
flow.nullLiteral(nullExpr);
128138
var expr = _Expression();
129-
flow.conditionEqNull(expr, x, notEqual: false);
139+
flow.equalityOp_end(expr, nullExpr, notEqual: false);
140+
flow.ifStatement_thenBegin(expr);
141+
expect(flow.promotedType(x), isNull);
142+
flow.ifStatement_elseBegin();
143+
expect(flow.promotedType(x).type, 'int');
144+
flow.ifStatement_end(true);
145+
});
146+
});
147+
148+
test('equalityOp(null != x) promotes true branch', () {
149+
var h = _Harness();
150+
var x = h.addVar('x', 'int?');
151+
h.run((flow) {
152+
h.declare(x, initialized: true);
153+
var nullExpr = _Expression();
154+
flow.nullLiteral(nullExpr);
155+
flow.equalityOp_rightBegin(nullExpr);
156+
var varExpr = _Expression();
157+
flow.variableRead(varExpr, x);
158+
var expr = _Expression();
159+
flow.equalityOp_end(expr, varExpr, notEqual: true);
160+
flow.ifStatement_thenBegin(expr);
161+
expect(flow.promotedType(x).type, 'int');
162+
flow.ifStatement_elseBegin();
163+
expect(flow.promotedType(x), isNull);
164+
flow.ifStatement_end(true);
165+
});
166+
});
167+
168+
test('equalityOp(null == x) promotes false branch', () {
169+
var h = _Harness();
170+
var x = h.addVar('x', 'int?');
171+
h.run((flow) {
172+
h.declare(x, initialized: true);
173+
var nullExpr = _Expression();
174+
flow.nullLiteral(nullExpr);
175+
flow.equalityOp_rightBegin(nullExpr);
176+
var varExpr = _Expression();
177+
flow.variableRead(varExpr, x);
178+
var expr = _Expression();
179+
flow.equalityOp_end(expr, varExpr, notEqual: false);
130180
flow.ifStatement_thenBegin(expr);
131181
expect(flow.promotedType(x), isNull);
132182
flow.ifStatement_elseBegin();
@@ -1799,8 +1849,13 @@ class _Harness
17991849
/// [variable].
18001850
LazyExpression eqNull(_Var variable) {
18011851
return () {
1852+
var varExpr = _Expression();
1853+
_flow.variableRead(varExpr, variable);
1854+
_flow.equalityOp_rightBegin(varExpr);
1855+
var nullExpr = _Expression();
1856+
_flow.nullLiteral(nullExpr);
18021857
var expr = _Expression();
1803-
_flow.conditionEqNull(expr, variable, notEqual: false);
1858+
_flow.equalityOp_end(expr, nullExpr, notEqual: false);
18041859
return expr;
18051860
};
18061861
}
@@ -1877,8 +1932,13 @@ class _Harness
18771932
/// [variable].
18781933
LazyExpression notNull(_Var variable) {
18791934
return () {
1935+
var varExpr = _Expression();
1936+
_flow.variableRead(varExpr, variable);
1937+
_flow.equalityOp_rightBegin(varExpr);
1938+
var nullExpr = _Expression();
1939+
_flow.nullLiteral(nullExpr);
18801940
var expr = _Expression();
1881-
_flow.conditionEqNull(expr, variable, notEqual: true);
1941+
_flow.equalityOp_end(expr, nullExpr, notEqual: true);
18821942
return expr;
18831943
};
18841944
}

‎pkg/front_end/test/spell_checking_list_common.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -889,6 +889,7 @@ encodes
889889
encoding
890890
encounter
891891
encountered
892+
encountering
892893
encounters
893894
end
894895
ended

0 commit comments

Comments
 (0)