Ensure that `if` statements promote properly in unreachable code
Bug: https://github.com/dart-lang/sdk/issues/40009
Change-Id: I04a5af558bb70b861d92b5379a8fb84489d5c9f4
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/165402
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
diff --git a/pkg/_fe_analyzer_shared/lib/src/flow_analysis/flow_analysis.dart b/pkg/_fe_analyzer_shared/lib/src/flow_analysis/flow_analysis.dart
index 1a1c1df..637e387 100644
--- a/pkg/_fe_analyzer_shared/lib/src/flow_analysis/flow_analysis.dart
+++ b/pkg/_fe_analyzer_shared/lib/src/flow_analysis/flow_analysis.dart
@@ -536,6 +536,25 @@
bool ifNullExpression_rightBegin(
Expression leftHandSide, Type leftHandSideType);
+ /// Call this method before visiting the condition part of an if statement.
+ ///
+ /// The order of visiting an if statement with no "else" part should be:
+ /// - Call [ifStatement_conditionBegin]
+ /// - Visit the condition
+ /// - Call [ifStatement_thenBegin]
+ /// - Visit the "then" statement
+ /// - Call [ifStatement_end], passing `false` for `hasElse`.
+ ///
+ /// The order of visiting an if statement with an "else" part should be:
+ /// - Call [ifStatement_conditionBegin]
+ /// - Visit the condition
+ /// - Call [ifStatement_thenBegin]
+ /// - Visit the "then" statement
+ /// - Call [ifStatement_elseBegin]
+ /// - Visit the "else" statement
+ /// - Call [ifStatement_end], passing `true` for `hasElse`.
+ void ifStatement_conditionBegin();
+
/// Call this method after visiting the "then" part of an if statement, and
/// before visiting the "else" part.
void ifStatement_elseBegin();
@@ -545,20 +564,6 @@
/// Call this method after visiting the condition part of an if statement.
/// [condition] should be the if statement's condition.
- ///
- /// The order of visiting an if statement with no "else" part should be:
- /// - Visit the condition
- /// - Call [ifStatement_thenBegin]
- /// - Visit the "then" statement
- /// - Call [ifStatement_end], passing `false` for `hasElse`.
- ///
- /// The order of visiting an if statement with an "else" part should be:
- /// - Visit the condition
- /// - Call [ifStatement_thenBegin]
- /// - Visit the "then" statement
- /// - Call [ifStatement_elseBegin]
- /// - Visit the "else" statement
- /// - Call [ifStatement_end], passing `true` for `hasElse`.
void ifStatement_thenBegin(Expression condition);
/// Return whether the [variable] is definitely assigned in the current state.
@@ -1015,6 +1020,12 @@
}
@override
+ void ifStatement_conditionBegin() {
+ return _wrap('ifStatement_conditionBegin()',
+ () => _wrapped.ifStatement_conditionBegin());
+ }
+
+ @override
void ifStatement_elseBegin() {
return _wrap(
'ifStatement_elseBegin()', () => _wrapped.ifStatement_elseBegin());
@@ -1658,6 +1669,17 @@
FlowModel<Variable, Type> unsplit() =>
new FlowModel<Variable, Type>.withInfo(reachable.unsplit(), variableInfo);
+ /// Removes control flow splits until a [FlowModel] is obtained whose
+ /// reachability has the given [parent].
+ FlowModel<Variable, Type> unsplitTo(Reachability parent) {
+ if (identical(this.reachable.parent, parent)) return this;
+ Reachability reachable = this.reachable.unsplit();
+ while (!identical(reachable.parent, parent)) {
+ reachable = reachable.unsplit();
+ }
+ return new FlowModel<Variable, Type>.withInfo(reachable, variableInfo);
+ }
+
/// Updates the state to indicate that an assignment was made to the given
/// [variable]. The variable is marked as definitely assigned, and any
/// previous type promotion is removed.
@@ -1808,6 +1830,37 @@
return result;
}
+ /// Models the result of joining the flow models [first] and [second] at the
+ /// merge of two control flow paths.
+ static FlowModel<Variable, Type> merge<Variable, Type>(
+ TypeOperations<Variable, Type> typeOperations,
+ FlowModel<Variable, Type> first,
+ FlowModel<Variable, Type> second,
+ Map<Variable, VariableModel<Variable, Type>> emptyVariableMap,
+ ) {
+ if (first == null) return second.unsplit();
+ if (second == null) return first.unsplit();
+
+ assert(identical(first.reachable.parent, second.reachable.parent));
+ if (first.reachable.locallyReachable &&
+ !second.reachable.locallyReachable) {
+ return first.unsplit();
+ }
+ if (!first.reachable.locallyReachable &&
+ second.reachable.locallyReachable) {
+ return second.unsplit();
+ }
+
+ Reachability newReachable =
+ Reachability.join(first.reachable, second.reachable).unsplit();
+ Map<Variable, VariableModel<Variable, Type>> newVariableInfo =
+ FlowModel.joinVariableInfo(typeOperations, first.variableInfo,
+ second.variableInfo, emptyVariableMap);
+
+ return FlowModel._identicalOrNew(
+ first, second, newReachable, newVariableInfo);
+ }
+
/// Creates a new [FlowModel] object, unless it is equivalent to either
/// [first] or [second], in which case one of those objects is re-used.
static FlowModel<Variable, Type> _identicalOrNew<Variable, Type>(
@@ -2554,9 +2607,17 @@
/// `null` if no `continue` statements have been seen yet.
FlowModel<Variable, Type> _continueModel;
+ /// The reachability checkpoint associated with this loop or switch statement.
+ /// When analyzing deeply nested `break` and `continue` statements, their flow
+ /// models need to be unsplit to this point before joining them to the control
+ /// flow paths for the loop or switch.
+ final Reachability _checkpoint;
+
+ _BranchTargetContext(this._checkpoint);
+
@override
String toString() => '_BranchTargetContext(breakModel: $_breakModel, '
- 'continueModel: $_continueModel)';
+ 'continueModel: $_continueModel, checkpoint: $_checkpoint)';
}
/// [_FlowContext] representing a conditional expression.
@@ -2710,7 +2771,7 @@
AssignedVariablesNodeInfo<Variable> info =
_assignedVariables._getInfoForNode(doStatement);
_BranchTargetContext<Variable, Type> context =
- new _BranchTargetContext<Variable, Type>();
+ new _BranchTargetContext<Variable, Type>(_current.reachable.parent);
_stack.add(context);
_current = _current.conservativeJoin(info._written, info._captured);
_statementToContext[doStatement] = context;
@@ -2794,8 +2855,8 @@
ExpressionInfo<Variable, Type> conditionInfo = condition == null
? new ExpressionInfo(_current, _current, _current.setUnreachable())
: _expressionEnd(condition);
- _WhileContext<Variable, Type> context =
- new _WhileContext<Variable, Type>(conditionInfo);
+ _WhileContext<Variable, Type> context = new _WhileContext<Variable, Type>(
+ _current.reachable.parent, conditionInfo);
_stack.add(context);
if (node != null) {
_statementToContext[node] = context;
@@ -2835,7 +2896,8 @@
_assignedVariables._getInfoForNode(node);
_current = _current.conservativeJoin(info._written, info._captured);
_SimpleStatementContext<Variable, Type> context =
- new _SimpleStatementContext<Variable, Type>(_current);
+ new _SimpleStatementContext<Variable, Type>(
+ _current.reachable.parent, _current);
_stack.add(context);
if (loopVariable != null) {
_current = _current.write(loopVariable, writtenType, typeOperations);
@@ -2880,7 +2942,8 @@
void handleBreak(Statement target) {
_BranchTargetContext<Variable, Type> context = _statementToContext[target];
if (context != null) {
- context._breakModel = _join(context._breakModel, _current);
+ context._breakModel =
+ _join(context._breakModel, _current.unsplitTo(context._checkpoint));
}
_current = _current.setUnreachable();
}
@@ -2889,7 +2952,8 @@
void handleContinue(Statement target) {
_BranchTargetContext<Variable, Type> context = _statementToContext[target];
if (context != null) {
- context._continueModel = _join(context._continueModel, _current);
+ context._continueModel = _join(
+ context._continueModel, _current.unsplitTo(context._checkpoint));
}
_current = _current.setUnreachable();
}
@@ -2924,6 +2988,11 @@
}
@override
+ void ifStatement_conditionBegin() {
+ _current = _current.split();
+ }
+
+ @override
void ifStatement_elseBegin() {
_IfContext<Variable, Type> context =
_stack.last as _IfContext<Variable, Type>;
@@ -2944,7 +3013,7 @@
afterThen = _current; // no `else`, so `then` is still current
afterElse = context._conditionInfo.ifFalse;
}
- _current = _join(afterThen, afterElse);
+ _current = _merge(afterThen, afterElse);
}
@override
@@ -2985,7 +3054,7 @@
@override
void labeledStatement_begin(Node node) {
_BranchTargetContext<Variable, Type> context =
- new _BranchTargetContext<Variable, Type>();
+ new _BranchTargetContext<Variable, Type>(_current.reachable.parent);
_stack.add(context);
_statementToContext[node] = context;
}
@@ -3140,7 +3209,8 @@
@override
void switchStatement_expressionEnd(Statement switchStatement) {
_SimpleStatementContext<Variable, Type> context =
- new _SimpleStatementContext<Variable, Type>(_current);
+ new _SimpleStatementContext<Variable, Type>(
+ _current.reachable.parent, _current);
_stack.add(context);
_statementToContext[switchStatement] = context;
}
@@ -3232,8 +3302,8 @@
void whileStatement_bodyBegin(
Statement whileStatement, Expression condition) {
ExpressionInfo<Variable, Type> conditionInfo = _expressionEnd(condition);
- _WhileContext<Variable, Type> context =
- new _WhileContext<Variable, Type>(conditionInfo);
+ _WhileContext<Variable, Type> context = new _WhileContext<Variable, Type>(
+ _current.reachable.parent, conditionInfo);
_stack.add(context);
_statementToContext[whileStatement] = context;
_current = conditionInfo.ifTrue;
@@ -3302,6 +3372,11 @@
FlowModel<Variable, Type> first, FlowModel<Variable, Type> second) =>
FlowModel.join(typeOperations, first, second, _current._emptyVariableMap);
+ FlowModel<Variable, Type> _merge(
+ FlowModel<Variable, Type> first, FlowModel<Variable, Type> second) =>
+ FlowModel.merge(
+ typeOperations, first, second, _current._emptyVariableMap);
+
/// Associates [expression], which should be the most recently visited
/// expression, with the given [expressionInfo] object, and updates the
/// current flow model state to correspond to it.
@@ -3373,11 +3448,13 @@
/// after evaluation of the switch expression.
final FlowModel<Variable, Type> _previous;
- _SimpleStatementContext(this._previous);
+ _SimpleStatementContext(Reachability checkpoint, this._previous)
+ : super(checkpoint);
@override
String toString() => '_SimpleStatementContext(breakModel: $_breakModel, '
- 'continueModel: $_continueModel, previous: $_previous)';
+ 'continueModel: $_continueModel, previous: $_previous, '
+ 'checkpoint: $_checkpoint)';
}
/// [_FlowContext] representing a try statement.
@@ -3430,9 +3507,11 @@
/// Flow models associated with the loop condition.
final ExpressionInfo<Variable, Type> _conditionInfo;
- _WhileContext(this._conditionInfo);
+ _WhileContext(Reachability checkpoint, this._conditionInfo)
+ : super(checkpoint);
@override
String toString() => '_WhileContext(breakModel: $_breakModel, '
- 'continueModel: $_continueModel, conditionInfo: $_conditionInfo)';
+ 'continueModel: $_continueModel, conditionInfo: $_conditionInfo, '
+ 'checkpoint: $_checkpoint)';
}
diff --git a/pkg/_fe_analyzer_shared/test/flow_analysis/flow_analysis_test.dart b/pkg/_fe_analyzer_shared/test/flow_analysis/flow_analysis_test.dart
index 16ae7ff..0af6a2f 100644
--- a/pkg/_fe_analyzer_shared/test/flow_analysis/flow_analysis_test.dart
+++ b/pkg/_fe_analyzer_shared/test/flow_analysis/flow_analysis_test.dart
@@ -179,6 +179,7 @@
var x = h.addVar('x', 'int?');
h.run((flow) {
h.declare(x, initialized: true);
+ flow.ifStatement_conditionBegin();
var varExpr = _Expression();
flow.variableRead(varExpr, x);
flow.equalityOp_rightBegin(varExpr, _Type('int?'));
@@ -203,6 +204,7 @@
var x = h.addVar('x', 'int');
h.run((flow) {
h.declare(x, initialized: true);
+ flow.ifStatement_conditionBegin();
var varExpr = _Expression();
flow.variableRead(varExpr, x);
flow.equalityOp_rightBegin(varExpr, _Type('int'));
@@ -227,6 +229,7 @@
test('equalityOp(<expr> == <expr>) has no special effect', () {
var h = _Harness();
h.run((flow) {
+ flow.ifStatement_conditionBegin();
flow.equalityOp_rightBegin(_Expression(), _Type('int?'));
var expr = _Expression();
var successIsReachable = flow.equalityOp_end(
@@ -244,6 +247,7 @@
test('equalityOp(<expr> != <expr>) has no special effect', () {
var h = _Harness();
h.run((flow) {
+ flow.ifStatement_conditionBegin();
flow.equalityOp_rightBegin(_Expression(), _Type('int?'));
var expr = _Expression();
var successIsReachable = flow
@@ -262,6 +266,7 @@
var x = h.addVar('x', 'int?');
h.run((flow) {
h.declare(x, initialized: true);
+ flow.ifStatement_conditionBegin();
var varExpr = _Expression();
flow.variableRead(varExpr, x);
flow.equalityOp_rightBegin(varExpr, _Type('int?'));
@@ -281,6 +286,7 @@
var x = h.addVar('x', 'int?');
h.run((flow) {
h.declare(x, initialized: true);
+ flow.ifStatement_conditionBegin();
var varExpr = _Expression();
flow.variableRead(varExpr, x);
flow.equalityOp_rightBegin(varExpr, _Type('int?'));
@@ -305,6 +311,7 @@
var x = h.addVar('x', 'int');
h.run((flow) {
h.declare(x, initialized: true);
+ flow.ifStatement_conditionBegin();
var varExpr = _Expression();
flow.variableRead(varExpr, x);
flow.equalityOp_rightBegin(varExpr, _Type('int'));
@@ -331,6 +338,7 @@
var x = h.addVar('x', 'int?');
h.run((flow) {
h.declare(x, initialized: true);
+ flow.ifStatement_conditionBegin();
var nullExpr = _Expression();
flow.nullLiteral(nullExpr);
flow.equalityOp_rightBegin(nullExpr, _Type('Null'));
@@ -351,6 +359,7 @@
var x = h.addVar('x', 'int?');
h.run((flow) {
h.declare(x, initialized: true);
+ flow.ifStatement_conditionBegin();
var nullExpr = _Expression();
flow.equalityOp_rightBegin(nullExpr, _Type('Null'));
var varExpr = _Expression();
@@ -370,6 +379,7 @@
var x = h.addVar('x', 'int?');
h.run((flow) {
h.declare(x, initialized: true);
+ flow.ifStatement_conditionBegin();
var nullExpr = _Expression();
flow.nullLiteral(nullExpr);
flow.equalityOp_rightBegin(nullExpr, _Type('Null'));
@@ -388,6 +398,7 @@
test('equalityOp(null == null) equivalent to true', () {
var h = _Harness();
h.run((flow) {
+ flow.ifStatement_conditionBegin();
var null1 = _Expression();
flow.equalityOp_rightBegin(null1, _Type('Null'));
var null2 = _Expression();
@@ -406,6 +417,7 @@
test('equalityOp(null != null) equivalent to false', () {
var h = _Harness();
h.run((flow) {
+ flow.ifStatement_conditionBegin();
var null1 = _Expression();
flow.equalityOp_rightBegin(null1, _Type('Null'));
var null2 = _Expression();
@@ -424,6 +436,7 @@
test('equalityOp(null == non-null) is not equivalent to false', () {
var h = _Harness();
h.run((flow) {
+ flow.ifStatement_conditionBegin();
var null1 = _Expression();
flow.equalityOp_rightBegin(null1, _Type('Null'));
var null2 = _Expression();
@@ -443,6 +456,7 @@
test('equalityOp(null != non-null) is not equivalent to true', () {
var h = _Harness();
h.run((flow) {
+ flow.ifStatement_conditionBegin();
var null1 = _Expression();
flow.equalityOp_rightBegin(null1, _Type('Null'));
var null2 = _Expression();
@@ -463,6 +477,7 @@
test('equalityOp(non-null == null) is not equivalent to false', () {
var h = _Harness();
h.run((flow) {
+ flow.ifStatement_conditionBegin();
var null1 = _Expression();
flow.equalityOp_rightBegin(null1, _Type('int'));
var null2 = _Expression();
@@ -483,6 +498,7 @@
test('equalityOp(non-null != null) is not equivalent to true', () {
var h = _Harness();
h.run((flow) {
+ flow.ifStatement_conditionBegin();
var null1 = _Expression();
flow.equalityOp_rightBegin(null1, _Type('int'));
var null2 = _Expression();
@@ -599,6 +615,7 @@
var h = _Harness();
var expr = _Expression();
var flow = h.createFlow();
+ flow.ifStatement_conditionBegin();
flow.ifStatement_thenBegin(expr);
expect(() => flow.finish(), _asserts);
});
@@ -1040,6 +1057,94 @@
});
});
+ test('handleBreak handles deep nesting', () {
+ var h = _Harness();
+ var whileStatement = _Statement();
+ h.assignedVariables((vars) {
+ vars.nest(whileStatement, () {});
+ });
+ h.run((flow) {
+ flow.whileStatement_conditionBegin(whileStatement);
+ flow.whileStatement_bodyBegin(whileStatement, h.booleanLiteral(true)());
+ h.if_(h.expr, () {
+ h.if_(h.expr, () {
+ flow.handleBreak(whileStatement);
+ });
+ });
+ flow.handleExit();
+ expect(flow.isReachable, false);
+ flow.whileStatement_end();
+ expect(flow.isReachable, true);
+ });
+ });
+
+ test('handleBreak handles mixed nesting', () {
+ var h = _Harness();
+ var whileStatement = _Statement();
+ h.assignedVariables((vars) {
+ vars.nest(whileStatement, () {});
+ });
+ h.run((flow) {
+ flow.whileStatement_conditionBegin(whileStatement);
+ flow.whileStatement_bodyBegin(whileStatement, h.booleanLiteral(true)());
+ h.if_(h.expr, () {
+ h.if_(h.expr, () {
+ flow.handleBreak(whileStatement);
+ });
+ flow.handleBreak(whileStatement);
+ });
+ flow.handleBreak(whileStatement);
+ expect(flow.isReachable, false);
+ flow.whileStatement_end();
+ expect(flow.isReachable, true);
+ });
+ });
+
+ test('handleContinue handles deep nesting', () {
+ var h = _Harness();
+ var doStatement = _Statement();
+ h.assignedVariables((vars) {
+ vars.nest(doStatement, () {});
+ });
+ h.run((flow) {
+ flow.doStatement_bodyBegin(doStatement);
+ h.if_(h.expr, () {
+ h.if_(h.expr, () {
+ flow.handleContinue(doStatement);
+ });
+ });
+ flow.handleExit();
+ expect(flow.isReachable, false);
+ flow.doStatement_conditionBegin();
+ expect(flow.isReachable, true);
+ flow.doStatement_end(h.booleanLiteral(true)());
+ expect(flow.isReachable, false);
+ });
+ });
+
+ test('handleContinue handles mixed nesting', () {
+ var h = _Harness();
+ var doStatement = _Statement();
+ h.assignedVariables((vars) {
+ vars.nest(doStatement, () {});
+ });
+ h.run((flow) {
+ flow.doStatement_bodyBegin(doStatement);
+ h.if_(h.expr, () {
+ h.if_(h.expr, () {
+ flow.handleContinue(doStatement);
+ });
+ flow.handleContinue(doStatement);
+ });
+ flow.handleContinue(doStatement);
+ expect(flow.isReachable, false);
+ flow.doStatement_conditionBegin();
+ expect(flow.isReachable, true);
+ flow.doStatement_end(h.booleanLiteral(true)());
+ expect(flow.isReachable, false);
+ });
+ });
+
test('ifNullExpression allows ensure guarding', () {
var h = _Harness();
var x = h.addVar('x', 'int?');
@@ -1119,11 +1224,28 @@
});
});
+ test('ifStatement with early exit promotes in unreachable code', () {
+ var h = _Harness();
+ var x = h.addVar('x', 'int?');
+ h.run((flow) {
+ h.declare(x, initialized: true);
+ flow.handleExit();
+ expect(flow.isReachable, false);
+ flow.ifStatement_conditionBegin();
+ flow.ifStatement_thenBegin(h.eqNull(x, _Type('int?'))());
+ flow.handleExit();
+ flow.ifStatement_end(false);
+ expect(flow.isReachable, false);
+ expect(flow.promotedType(x).type, 'int');
+ });
+ });
+
test('ifStatement_end(false) keeps else branch if then branch exits', () {
var h = _Harness();
var x = h.addVar('x', 'int?');
h.run((flow) {
h.declare(x, initialized: true);
+ flow.ifStatement_conditionBegin();
flow.ifStatement_thenBegin(h.eqNull(x, _Type('int?'))());
flow.handleExit();
flow.ifStatement_end(false);
@@ -1138,6 +1260,7 @@
var x = h.addVar('x', declaredType);
h.run((flow) {
h.declare(x, initialized: true);
+ flow.ifStatement_conditionBegin();
var read = _Expression();
flow.variableRead(read, x);
var expr = _Expression();
@@ -1190,6 +1313,7 @@
test('isExpression_end does nothing if applied to a non-variable', () {
var h = _Harness();
h.run((flow) {
+ flow.ifStatement_conditionBegin();
var subExpr = _Expression();
var expr = _Expression();
var failureReachable =
@@ -1207,6 +1331,7 @@
() {
var h = _Harness();
h.run((flow) {
+ flow.ifStatement_conditionBegin();
var subExpr = _Expression();
var expr = _Expression();
var failureReachable =
@@ -1305,6 +1430,7 @@
var x = h.addVar('x', 'int?');
h.run((flow) {
h.declare(x, initialized: true);
+ flow.ifStatement_conditionBegin();
flow.logicalBinaryOp_rightBegin(_Expression(), isAnd: true);
var wholeExpr = _Expression();
flow.logicalBinaryOp_end(wholeExpr, h.notNull(x, _Type('int?'))(),
@@ -1321,6 +1447,7 @@
var x = h.addVar('x', 'int?');
h.run((flow) {
h.declare(x, initialized: true);
+ flow.ifStatement_conditionBegin();
flow.logicalBinaryOp_rightBegin(_Expression(), isAnd: false);
var wholeExpr = _Expression();
flow.logicalBinaryOp_end(wholeExpr, h.eqNull(x, _Type('int?'))(),
@@ -2163,6 +2290,7 @@
h.promote(x, 'int');
expect(flow.promotedType(x).type, 'int');
// if (false) {
+ flow.ifStatement_conditionBegin();
var falseExpression = _Expression();
flow.booleanLiteral(falseExpression, false);
flow.ifStatement_thenBegin(falseExpression);
@@ -2328,6 +2456,55 @@
expect(s2.reachable, same(Reachability.initial));
});
+ group('unsplitTo', () {
+ test('no change', () {
+ var s1 = FlowModel<_Var, _Type>(Reachability.initial.split());
+ var result = s1.unsplitTo(s1.reachable.parent);
+ expect(result, same(s1));
+ });
+
+ test('unsplit once, reachable', () {
+ var s1 = FlowModel<_Var, _Type>(Reachability.initial.split());
+ var s2 = s1.split();
+ var result = s2.unsplitTo(s1.reachable.parent);
+ expect(result.reachable, same(s1.reachable));
+ });
+
+ test('unsplit once, unreachable', () {
+ var s1 = FlowModel<_Var, _Type>(Reachability.initial.split());
+ var s2 = s1.split().setUnreachable();
+ var result = s2.unsplitTo(s1.reachable.parent);
+ expect(result.reachable.locallyReachable, false);
+ expect(result.reachable.parent, same(s1.reachable.parent));
+ });
+
+ test('unsplit twice, reachable', () {
+ var s1 = FlowModel<_Var, _Type>(Reachability.initial.split());
+ var s2 = s1.split();
+ var s3 = s2.split();
+ var result = s3.unsplitTo(s1.reachable.parent);
+ expect(result.reachable, same(s1.reachable));
+ });
+
+ test('unsplit twice, top unreachable', () {
+ var s1 = FlowModel<_Var, _Type>(Reachability.initial.split());
+ var s2 = s1.split();
+ var s3 = s2.split().setUnreachable();
+ var result = s3.unsplitTo(s1.reachable.parent);
+ expect(result.reachable.locallyReachable, false);
+ expect(result.reachable.parent, same(s1.reachable.parent));
+ });
+
+ test('unsplit twice, previous unreachable', () {
+ var s1 = FlowModel<_Var, _Type>(Reachability.initial.split());
+ var s2 = s1.split().setUnreachable();
+ var s3 = s2.split();
+ var result = s3.unsplitTo(s1.reachable.parent);
+ expect(result.reachable.locallyReachable, false);
+ expect(result.reachable.parent, same(s1.reachable.parent));
+ });
+ });
+
group('tryPromoteForTypeCheck', () {
test('unpromoted -> unchanged (same)', () {
var h = _Harness();
@@ -3581,6 +3758,101 @@
});
});
});
+
+ group('merge', () {
+ var x = _Var('x', _Type('Object?'));
+ var intType = _Type('int');
+ var stringType = _Type('String');
+ const emptyMap = const <_Var, VariableModel<_Var, _Type>>{};
+
+ VariableModel<_Var, _Type> varModel(List<_Type> promotionChain,
+ {bool assigned = false}) =>
+ VariableModel<_Var, _Type>(
+ promotionChain,
+ promotionChain ?? [],
+ assigned,
+ !assigned,
+ false,
+ );
+
+ test('first is null', () {
+ var h = _Harness();
+ var s1 = FlowModel.withInfo(Reachability.initial.split(), {});
+ var result = FlowModel.merge(h, null, s1, emptyMap);
+ expect(result.reachable, same(Reachability.initial));
+ });
+
+ test('second is null', () {
+ var h = _Harness();
+ var splitPoint = Reachability.initial.split();
+ var afterSplit = splitPoint.split();
+ var s1 = FlowModel.withInfo(afterSplit, {});
+ var result = FlowModel.merge(h, s1, null, emptyMap);
+ expect(result.reachable, same(splitPoint));
+ });
+
+ test('both are reachable', () {
+ var h = _Harness();
+ var splitPoint = Reachability.initial.split();
+ var afterSplit = splitPoint.split();
+ var s1 = FlowModel.withInfo(afterSplit, {
+ x: varModel([intType])
+ });
+ var s2 = FlowModel.withInfo(afterSplit, {
+ x: varModel([stringType])
+ });
+ var result = FlowModel.merge(h, s1, s2, emptyMap);
+ expect(result.reachable, same(splitPoint));
+ expect(result.variableInfo[x].promotedTypes, isNull);
+ });
+
+ test('first is unreachable', () {
+ var h = _Harness();
+ var splitPoint = Reachability.initial.split();
+ var afterSplit = splitPoint.split();
+ var s1 = FlowModel.withInfo(afterSplit.setUnreachable(), {
+ x: varModel([intType])
+ });
+ var s2 = FlowModel.withInfo(afterSplit, {
+ x: varModel([stringType])
+ });
+ var result = FlowModel.merge(h, s1, s2, emptyMap);
+ expect(result.reachable, same(splitPoint));
+ expect(result.variableInfo, same(s2.variableInfo));
+ });
+
+ test('second is unreachable', () {
+ var h = _Harness();
+ var splitPoint = Reachability.initial.split();
+ var afterSplit = splitPoint.split();
+ var s1 = FlowModel.withInfo(afterSplit, {
+ x: varModel([intType])
+ });
+ var s2 = FlowModel.withInfo(afterSplit.setUnreachable(), {
+ x: varModel([stringType])
+ });
+ var result = FlowModel.merge(h, s1, s2, emptyMap);
+ expect(result.reachable, same(splitPoint));
+ expect(result.variableInfo, same(s1.variableInfo));
+ });
+
+ test('both are unreachable', () {
+ var h = _Harness();
+ var splitPoint = Reachability.initial.split();
+ var afterSplit = splitPoint.split();
+ var s1 = FlowModel.withInfo(afterSplit.setUnreachable(), {
+ x: varModel([intType])
+ });
+ var s2 = FlowModel.withInfo(afterSplit.setUnreachable(), {
+ x: varModel([stringType])
+ });
+ var result = FlowModel.merge(h, s1, s2, emptyMap);
+ expect(result.reachable.locallyReachable, false);
+ expect(result.reachable.parent, same(splitPoint.parent));
+ expect(result.variableInfo[x].promotedTypes, isNull);
+ });
+ });
+
group('inheritTested', () {
var x = _Var('x', _Type('Object?'));
var intType = _Type('int');
@@ -3881,6 +4153,12 @@
callback(_AssignedVariablesHarness(_assignedVariables));
}
+ LazyExpression booleanLiteral(bool value) => () {
+ var expr = _Expression();
+ _flow.booleanLiteral(expr, value);
+ return expr;
+ };
+
@override
TypeClassification classifyType(_Type type) {
if (isSubtypeOf(type, _Type('Object'))) {
@@ -3943,6 +4221,7 @@
/// Invokes flow analysis of an `if` statement with no `else` part.
void if_(LazyExpression cond, void ifTrue()) {
+ _flow.ifStatement_conditionBegin();
_flow.ifStatement_thenBegin(cond());
ifTrue();
_flow.ifStatement_end(false);
@@ -3950,6 +4229,7 @@
/// Invokes flow analysis of an `if` statement with an `else` part.
void ifElse(LazyExpression cond, void ifTrue(), void ifFalse()) {
+ _flow.ifStatement_conditionBegin();
_flow.ifStatement_thenBegin(cond());
ifTrue();
_flow.ifStatement_elseBegin();
diff --git a/pkg/_fe_analyzer_shared/test/flow_analysis/type_promotion/data/promotion_in_dead_code.dart b/pkg/_fe_analyzer_shared/test/flow_analysis/type_promotion/data/promotion_in_dead_code.dart
new file mode 100644
index 0000000..4a3c9a5
--- /dev/null
+++ b/pkg/_fe_analyzer_shared/test/flow_analysis/type_promotion/data/promotion_in_dead_code.dart
@@ -0,0 +1,31 @@
+// Copyright (c) 2020, the Dart project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+// These tests verify that the kinds of constructs we expect to cause type
+// promotion continue to function properly even when used inside unreachable
+// code.
+
+ifIsNot(Object o) {
+ return;
+ if (o is! int) return;
+ /*int*/ o;
+}
+
+ifIsNot_listElement(Object o) {
+ return;
+ [if (o is! int) throw 'x'];
+ /*int*/ o;
+}
+
+ifIsNot_setElement(Object o) {
+ return;
+ ({if (o is! int) throw 'x'});
+ /*int*/ o;
+}
+
+ifIsNot_mapElement(Object o) {
+ return;
+ ({if (o is! int) 0: throw 'x'});
+ /*int*/ o;
+}
diff --git a/pkg/analyzer/lib/src/generated/resolver.dart b/pkg/analyzer/lib/src/generated/resolver.dart
index bc2bbad..0d46f4a 100644
--- a/pkg/analyzer/lib/src/generated/resolver.dart
+++ b/pkg/analyzer/lib/src/generated/resolver.dart
@@ -1498,6 +1498,7 @@
@override
void visitIfElement(IfElement node) {
+ _flowAnalysis?.flow?.ifStatement_conditionBegin();
Expression condition = node.condition;
InferenceContext.setType(condition, typeProvider.boolType);
// TODO(scheglov) Do we need these checks for null?
@@ -1535,6 +1536,7 @@
@override
void visitIfStatement(IfStatement node) {
checkUnreachableNode(node);
+ _flowAnalysis?.flow?.ifStatement_conditionBegin();
Expression condition = node.condition;
diff --git a/pkg/front_end/lib/src/fasta/kernel/inference_visitor.dart b/pkg/front_end/lib/src/fasta/kernel/inference_visitor.dart
index 3f0e79b..7288dd8 100644
--- a/pkg/front_end/lib/src/fasta/kernel/inference_visitor.dart
+++ b/pkg/front_end/lib/src/fasta/kernel/inference_visitor.dart
@@ -1184,6 +1184,7 @@
@override
StatementInferenceResult visitIfStatement(IfStatement node) {
+ inferrer.flowAnalysis.ifStatement_conditionBegin();
InterfaceType expectedType =
inferrer.coreTypes.boolRawType(inferrer.library.nonNullable);
ExpressionInferenceResult conditionResult = inferrer.inferExpression(
@@ -1411,6 +1412,7 @@
element.elementType = spreadElementType ?? const DynamicType();
return new ExpressionInferenceResult(element.elementType, replacement);
} else if (element is IfElement) {
+ inferrer.flowAnalysis.ifStatement_conditionBegin();
DartType boolType =
inferrer.coreTypes.boolRawType(inferrer.library.nonNullable);
ExpressionInferenceResult conditionResult = inferrer.inferExpression(
@@ -1914,6 +1916,7 @@
return replacement;
} else if (entry is IfMapEntry) {
+ inferrer.flowAnalysis.ifStatement_conditionBegin();
DartType boolType =
inferrer.coreTypes.boolRawType(inferrer.library.nonNullable);
ExpressionInferenceResult conditionResult = inferrer.inferExpression(
diff --git a/pkg/front_end/test/spell_checking_list_common.txt b/pkg/front_end/test/spell_checking_list_common.txt
index b370368..47cfd38 100644
--- a/pkg/front_end/test/spell_checking_list_common.txt
+++ b/pkg/front_end/test/spell_checking_list_common.txt
@@ -2783,6 +2783,7 @@
speed
speedup
split
+splits
splitter
spread
spreadable
diff --git a/pkg/nnbd_migration/lib/src/edge_builder.dart b/pkg/nnbd_migration/lib/src/edge_builder.dart
index 7363e60..7d4a32e 100644
--- a/pkg/nnbd_migration/lib/src/edge_builder.dart
+++ b/pkg/nnbd_migration/lib/src/edge_builder.dart
@@ -947,6 +947,7 @@
@override
DecoratedType visitIfStatement(IfStatement node) {
+ _flowAnalysis.ifStatement_conditionBegin();
_checkExpressionNotNull(node.condition);
NullabilityNode trueGuard;
NullabilityNode falseGuard;