Flow analysis: account for implicit break at the end of switch statement cases.

Bug: https://github.com/dart-lang/sdk/issues/50419
Change-Id: Ie9b0dd3319dd4e28f53082794aa18ed422b0894b
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/274605
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
This commit is contained in:
Paul Berry 2022-12-09 17:20:38 +00:00 committed by Commit Queue
parent 966aed6bd1
commit d0fc6ec27b
4 changed files with 60 additions and 5 deletions

View file

@ -600,6 +600,10 @@ abstract class FlowAnalysis<Node extends Object, Statement extends Node,
@visibleForTesting
SsaNode<Type>? ssaNodeForTesting(Variable variable);
/// Call this method just after visiting a `case` or `default` body. See
/// [switchStatement_expressionEnd] for details.
void switchStatement_afterCase();
/// Call this method just before visiting a `case` or `default` clause. See
/// [switchStatement_expressionEnd] for details.
void switchStatement_beginAlternative();
@ -650,6 +654,7 @@ abstract class FlowAnalysis<Node extends Object, Statement extends Node,
/// - Call [switchStatement_endAlternative].
/// - Call [switchStatement_endAlternatives].
/// - Visit the case body.
/// - Call [switchStatement_afterCase].
/// - Call [switchStatement_end].
///
/// [scrutinee] should be the expression appearing in parentheses after the
@ -1326,6 +1331,12 @@ class FlowAnalysisDebug<Node extends Object, Statement extends Node,
isQuery: true);
}
@override
void switchStatement_afterCase() {
_wrap('switchStatement_afterCase()',
() => _wrapped.switchStatement_afterCase());
}
@override
void switchStatement_beginAlternative() {
_wrap('switchStatement_beginAlternative()',
@ -3937,6 +3948,13 @@ class _FlowAnalysisImpl<Node extends Object, Statement extends Node,
SsaNode<Type>? ssaNodeForTesting(Variable variable) => _current
.variableInfo[promotionKeyStore.keyForVariable(variable)]?.ssaNode;
@override
void switchStatement_afterCase() {
_SwitchStatementContext<Type> context =
_stack.last as _SwitchStatementContext<Type>;
context._breakModel = _join(context._breakModel, _current);
}
@override
void switchStatement_beginAlternative() {
_SwitchAlternativesContext<Type> context =
@ -3959,13 +3977,17 @@ class _FlowAnalysisImpl<Node extends Object, Statement extends Node,
_stack.removeLast() as _SimpleStatementContext<Type>;
FlowModel<Type>? breakState = context._breakModel;
// It is allowed to "fall off" the end of a switch statement, so join the
// current state to any breaks that were found previously.
breakState = _join(breakState, _current);
// And, if there is an implicit fall-through default, join it to any breaks.
// If there is an implicit fall-through default, join it to any breaks.
if (!isExhaustive) breakState = _join(breakState, context._previous);
// If there were no breaks (neither implicit nor explicit), then
// `breakState` will be `null`. This means this is an empty switch
// statement and the type of the scrutinee is an exhaustive type. This
// could happen, for instance, if the scrutinee type is the empty record
// type. We need to consider the code after the switch statement reachable
// if this happens.
breakState ??= context._previous;
_current = breakState.unsplit();
}
@ -4850,6 +4872,9 @@ class _LegacyTypePromotion<Node extends Object, Statement extends Node,
throw new StateError('ssaNodeForTesting requires null-aware flow analysis');
}
@override
void switchStatement_afterCase() {}
@override
void switchStatement_beginAlternative() {}

View file

@ -1236,6 +1236,7 @@ mixin TypeAnalyzer<
flow?.switchStatement_endAlternatives(null, hasLabels: false);
// Stack: (Expression, i * ExpressionCase, CaseHead)
Type type = analyzeExpression(memberInfo.expression, context);
flow?.switchStatement_afterCase();
// Stack: (Expression, i * ExpressionCase, CaseHead, Expression)
if (lubType == null) {
lubType = type;
@ -1337,6 +1338,7 @@ mixin TypeAnalyzer<
!lastCaseTerminates) {
errors?.switchCaseCompletesNormally(node, caseIndex, 1);
}
flow?.switchStatement_afterCase();
handleMergedStatementCase(node,
caseIndex: caseIndex, isTerminating: lastCaseTerminates);
// Stack: (Expression, (numExecutionPaths + 1) * StatementCase)

View file

@ -6686,6 +6686,33 @@ main() {
isExhaustive: true),
]);
});
test('implicit break', () {
var x = Var('x');
h.run([
declare(x, type: 'Object'),
switch_(
expr('Object'),
[
switchStatementMember([
wildcard(type: 'int').switchCase
], [
x.expr.as_('int').stmt,
]),
switchStatementMember([default_], [return_()])
],
isExhaustive: true),
checkReachable(true),
checkPromoted(x, 'int'),
]);
});
test('empty exhaustive', () {
h.run([
switch_(expr('()'), [], isExhaustive: true),
checkReachable(true),
]);
});
});
group('Variable pattern:', () {

View file

@ -1956,6 +1956,7 @@ class EdgeBuilder extends GeneralizingAstVisitor<DecoratedType>
hasDefault = true;
}
_dispatchList(member.statements);
_flowAnalysis!.switchStatement_afterCase();
});
}
_flowAnalysis!.switchStatement_end(hasDefault);