Skip to content

Commit 2ec3b51

Browse files
stereotype441Commit Queue
authored and
Commit Queue
committed
Patterns flow analysis: recognize trivially exhaustive switches.
This fixes a minor bug in flow analysis which was preventing it from recognizing when a switch statement was trivially exhaustive, meaning one of its reachable cases was guaranteed to always match. This mostly addresses dart-lang/language#2980, but flow analysis still fails to recognize that: - A list pattern containing a just a single rest pattern always matches (unless the rest pattern has a subpattern that may fail to match). - A null check pattern always matches if its subpattern always matches and the matched value type is non-nullable. - The relational pattern `!= null` always matches if its subpattern always matches and the matched value type is non-nullable. Fortunately, these drawbacks are small and don't lead to unsoundness. I'll try to address them in follow up CLs. Bug: dart-lang/language#2980 Change-Id: Ie9f8564cde66a5a2c41114033ca3ff0e1a0f139a Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/293860 Reviewed-by: Konstantin Shcheglov <[email protected]> Commit-Queue: Paul Berry <[email protected]> Reviewed-by: Johnni Winther <[email protected]>
1 parent 9e19979 commit 2ec3b51

File tree

4 files changed

+788
-1
lines changed

4 files changed

+788
-1
lines changed

pkg/_fe_analyzer_shared/lib/src/flow_analysis/flow_analysis.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4592,7 +4592,7 @@ class _FlowAnalysisImpl<Node extends Object, Statement extends Node,
45924592
FlowModel<Type>? breakState = context._breakModel;
45934593

45944594
// If there is an implicit fall-through default, join it to any breaks.
4595-
if (!isExhaustive) breakState = _join(breakState, context._previous);
4595+
if (!isExhaustive) breakState = _join(breakState, context._unmatched);
45964596

45974597
// If there were no breaks (neither implicit nor explicit), then
45984598
// `breakState` will be `null`. This means this is an empty switch

pkg/_fe_analyzer_shared/test/flow_analysis/flow_analysis_test.dart

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9131,6 +9131,106 @@ main() {
91319131
]);
91329132
});
91339133
});
9134+
9135+
group('Trivial exhaustiveness:', () {
9136+
// Although flow analysis doesn't attempt to do full exhaustiveness
9137+
// checking on switch statements, it understands that if any single case
9138+
// fully covers the matched value type, the switch statement is
9139+
// exhaustive. (Such a switch is called "trivially exhaustive").
9140+
//
9141+
// Note that we don't test all possible patterns, because the flow
9142+
// analysis logic for detecting trivial exhaustiveness builds on the
9143+
// logic for tracking the "unmatched" state, which is tested elsewhere.
9144+
test('exhaustive', () {
9145+
h.run([
9146+
switch_(expr('Object'), [
9147+
wildcard().switchCase.then([
9148+
return_(),
9149+
]),
9150+
]),
9151+
checkReachable(false),
9152+
]);
9153+
});
9154+
9155+
test('exhaustive but a reachable switch case completes', () {
9156+
// In this case, even though the switch is trivially exhaustive, the
9157+
// code after the switch is reachable because one of the reachable
9158+
// switch cases completes normally.
9159+
h.run([
9160+
switch_(expr('Object'), [
9161+
wildcard(type: 'int').switchCase.then([
9162+
checkReachable(true),
9163+
]),
9164+
wildcard().switchCase.then([
9165+
return_(),
9166+
]),
9167+
]),
9168+
checkReachable(true),
9169+
]);
9170+
});
9171+
9172+
test('exhaustive but an unreachable switch case completes', () {
9173+
// In this case, even though the `int` case completes normally, that
9174+
// case is unreachable, so the code after the switch is unreachable.
9175+
h.run([
9176+
switch_(expr('Object'), [
9177+
wildcard().switchCase.then([
9178+
return_(),
9179+
]),
9180+
wildcard(type: 'int').switchCase.then([
9181+
checkReachable(false),
9182+
]),
9183+
]),
9184+
checkReachable(false),
9185+
]);
9186+
});
9187+
9188+
test('exhaustive but a reachable switch case breaks', () {
9189+
// In this case, even though the switch is trivially exhaustive, the
9190+
// code after the switch is reachable because one of the reachable
9191+
// switch cases ends in a break.
9192+
h.run([
9193+
switch_(expr('Object'), [
9194+
wildcard(type: 'int').switchCase.then([
9195+
checkReachable(true),
9196+
break_(),
9197+
]),
9198+
wildcard().switchCase.then([
9199+
return_(),
9200+
]),
9201+
]),
9202+
checkReachable(true),
9203+
]);
9204+
});
9205+
9206+
test('exhaustive but an unreachable switch case breaks', () {
9207+
// In this case, even though the `int` case breaks, that case is
9208+
// unreachable, so the code after the switch is unreachable.
9209+
h.run([
9210+
switch_(expr('Object'), [
9211+
wildcard().switchCase.then([
9212+
return_(),
9213+
]),
9214+
wildcard(type: 'int').switchCase.then([
9215+
checkReachable(false),
9216+
break_(),
9217+
]),
9218+
]),
9219+
checkReachable(false),
9220+
]);
9221+
});
9222+
9223+
test('not exhaustive', () {
9224+
h.run([
9225+
switch_(expr('Object'), [
9226+
wildcard(type: 'int').switchCase.then([
9227+
return_(),
9228+
]),
9229+
]),
9230+
checkReachable(true),
9231+
]);
9232+
});
9233+
});
91349234
});
91359235

91369236
group('Variable pattern:', () {

pkg/nnbd_migration/lib/src/edge_builder.dart

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1960,6 +1960,8 @@ class EdgeBuilder extends GeneralizingAstVisitor<DecoratedType>
19601960
var hasLabel = member.labels.isNotEmpty;
19611961
_flowAnalysis!.switchStatement_beginAlternatives();
19621962
_flowAnalysis!.switchStatement_beginAlternative();
1963+
_flowAnalysis!.constantPattern_end(node.expression, scrutineeType,
1964+
patternsEnabled: false);
19631965
_flowAnalysis!.switchStatement_endAlternative(null, {});
19641966
_flowAnalysis!
19651967
.switchStatement_endAlternatives(node, hasLabels: hasLabel);

0 commit comments

Comments
 (0)