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
https://github.com/dart-lang/language/issues/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: https://github.com/dart-lang/language/issues/2980
Change-Id: Ie9f8564cde66a5a2c41114033ca3ff0e1a0f139a
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/293860
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
This commit is contained in:
Paul Berry
2023-04-05 23:32:28 +00:00
committed by Commit Queue
parent 9e1997971e
commit 2ec3b513db
4 changed files with 788 additions and 1 deletions
@@ -4592,7 +4592,7 @@ class _FlowAnalysisImpl<Node extends Object, Statement extends Node,
FlowModel<Type>? breakState = context._breakModel;
// If there is an implicit fall-through default, join it to any breaks.
if (!isExhaustive) breakState = _join(breakState, context._previous);
if (!isExhaustive) breakState = _join(breakState, context._unmatched);
// If there were no breaks (neither implicit nor explicit), then
// `breakState` will be `null`. This means this is an empty switch
@@ -9131,6 +9131,106 @@ main() {
]);
});
});
group('Trivial exhaustiveness:', () {
// Although flow analysis doesn't attempt to do full exhaustiveness
// checking on switch statements, it understands that if any single case
// fully covers the matched value type, the switch statement is
// exhaustive. (Such a switch is called "trivially exhaustive").
//
// Note that we don't test all possible patterns, because the flow
// analysis logic for detecting trivial exhaustiveness builds on the
// logic for tracking the "unmatched" state, which is tested elsewhere.
test('exhaustive', () {
h.run([
switch_(expr('Object'), [
wildcard().switchCase.then([
return_(),
]),
]),
checkReachable(false),
]);
});
test('exhaustive but a reachable switch case completes', () {
// In this case, even though the switch is trivially exhaustive, the
// code after the switch is reachable because one of the reachable
// switch cases completes normally.
h.run([
switch_(expr('Object'), [
wildcard(type: 'int').switchCase.then([
checkReachable(true),
]),
wildcard().switchCase.then([
return_(),
]),
]),
checkReachable(true),
]);
});
test('exhaustive but an unreachable switch case completes', () {
// In this case, even though the `int` case completes normally, that
// case is unreachable, so the code after the switch is unreachable.
h.run([
switch_(expr('Object'), [
wildcard().switchCase.then([
return_(),
]),
wildcard(type: 'int').switchCase.then([
checkReachable(false),
]),
]),
checkReachable(false),
]);
});
test('exhaustive but a reachable switch case breaks', () {
// In this case, even though the switch is trivially exhaustive, the
// code after the switch is reachable because one of the reachable
// switch cases ends in a break.
h.run([
switch_(expr('Object'), [
wildcard(type: 'int').switchCase.then([
checkReachable(true),
break_(),
]),
wildcard().switchCase.then([
return_(),
]),
]),
checkReachable(true),
]);
});
test('exhaustive but an unreachable switch case breaks', () {
// In this case, even though the `int` case breaks, that case is
// unreachable, so the code after the switch is unreachable.
h.run([
switch_(expr('Object'), [
wildcard().switchCase.then([
return_(),
]),
wildcard(type: 'int').switchCase.then([
checkReachable(false),
break_(),
]),
]),
checkReachable(false),
]);
});
test('not exhaustive', () {
h.run([
switch_(expr('Object'), [
wildcard(type: 'int').switchCase.then([
return_(),
]),
]),
checkReachable(true),
]);
});
});
});
group('Variable pattern:', () {
@@ -1960,6 +1960,8 @@ class EdgeBuilder extends GeneralizingAstVisitor<DecoratedType>
var hasLabel = member.labels.isNotEmpty;
_flowAnalysis!.switchStatement_beginAlternatives();
_flowAnalysis!.switchStatement_beginAlternative();
_flowAnalysis!.constantPattern_end(node.expression, scrutineeType,
patternsEnabled: false);
_flowAnalysis!.switchStatement_endAlternative(null, {});
_flowAnalysis!
.switchStatement_endAlternatives(node, hasLabels: hasLabel);
@@ -0,0 +1,685 @@
// Copyright (c) 2023, 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.
// SharedOptions=--enable-experiment=patterns,records
// Flow analysis doesn't do full exhaustiveness analysis of switch statements,
// but it detects if a switch statement is "trivially exhaustive". A switch
// statement is trivially exhaustive if it has at least one case that fully
// covers the matched value type.
//
// Also, flow analysis understands that after a case that fully covers the
// matched value type, any further cases are unreachable.
//
// We detect whether flow analysis considers the switch exhaustive by assigning
// to a nullable variable in all cases (this promotes the variable to
// non-nullable), and seeing whether the promotion lasts after the switch.
import '../static_type_helper.dart';
void testTwoCasesSecondExhaustive(Object x) {
// Trivially exhaustive because the second case fully covers the matched type
bool? y;
switch (x) {
case int _:
y = true;
case _:
y = true;
}
y.expectStaticType<Exactly<bool>>();
}
void testTwoCasesNotExhaustive(Object x) {
// Not exhaustive because neither case fully covers the matched type
bool? y;
switch (x) {
case int _:
y = true;
case String _:
y = true;
}
y.expectStaticType<Exactly<bool?>>();
}
void testUnreachableCase(Object x) {
// Not only is this switch trivially exhaustive, but also the second case is
// unreachable, and hence `y` remains promoted after the switch.
bool? y;
switch (x) {
case _:
y = true;
case int _:
// ^^^^
// [analyzer] HINT.UNREACHABLE_SWITCH_CASE
y = null;
}
y.expectStaticType<Exactly<bool>>();
}
void testCastWhereSubpatternAlwaysMatches(Object x) {
// Trivially exhaustive
bool? y;
switch (x) {
case _ as int:
y = true;
}
y.expectStaticType<Exactly<bool>>();
}
void testCastWhereSubpatternMatchesCastType(Object x) {
// Trivially exhaustive
bool? y;
switch (x) {
case bool() as bool:
y = true;
}
y.expectStaticType<Exactly<bool>>();
}
void testCastWhereSubpatternMayFailToMatch(Object x) {
// Not exhaustive
bool? y;
switch (x) {
case (== 0) as int:
y = true;
}
y.expectStaticType<Exactly<bool?>>();
}
void testListEmpty(List<Object> x) {
// Not exhaustive
bool? y;
switch (x) {
case []:
y = true;
}
y.expectStaticType<Exactly<bool?>>();
}
void testListContainingNonRestPattern(List<Object> x) {
// Not exhaustive
bool? y;
switch (x) {
case [_]:
y = true;
}
y.expectStaticType<Exactly<bool?>>();
}
void testListContainingNonRestPatternAndRestPattern(List<Object> x) {
// Not exhaustive
bool? y;
switch (x) {
case [_, ...]:
y = true;
}
y.expectStaticType<Exactly<bool?>>();
}
void testListContainingRestPatternAndNonRestPattern(List<Object> x) {
// Not exhaustive
bool? y;
switch (x) {
case [..., _]:
y = true;
}
y.expectStaticType<Exactly<bool?>>();
}
void testListContainingOnlyRestPattern(List<Object> x) {
// TODO(paulberry): this should be trivially exhaustive
bool? y;
switch (x) {
case [...]:
y = true;
}
y.expectStaticType<Exactly<bool?>>();
}
void testListContainingOnlyRestPatternWithSubpatternWildcard(List<Object> x) {
// TODO(paulberry): this should be trivially exhaustive
bool? y;
switch (x) {
case [..._]:
y = true;
}
y.expectStaticType<Exactly<bool?>>();
}
void testListContainingOnlyRestPatternWithSubpatternAnyList(List<Object> x) {
// TODO(paulberry): this should be trivially exhaustive
bool? y;
switch (x) {
case [...[...]]:
y = true;
}
y.expectStaticType<Exactly<bool?>>();
}
void testListContainingOnlyRestPatternWithSubpatternObjectPattern(
List<Object> x) {
// TODO(paulberry): this should be trivially exhaustive
bool? y;
switch (x) {
case [...List()]:
y = true;
}
y.expectStaticType<Exactly<bool?>>();
}
void testListContainingOnlyRestPatternWithSubpatternOther(List<Object> x) {
// Not exhaustive
bool? y;
switch (x) {
case [...List(length: 1)]:
y = true;
}
y.expectStaticType<Exactly<bool?>>();
}
void testListSupertype(List<Object> x) {
// TODO(paulberry): this should be trivially exhaustive
bool? y;
switch (x) {
case <Object?>[...]:
y = true;
}
y.expectStaticType<Exactly<bool?>>();
}
void testListSubtype(List<Object> x) {
// Not exhaustive
bool? y;
switch (x) {
case <int>[...]:
y = true;
}
y.expectStaticType<Exactly<bool?>>();
}
void testListSubtypeObject(Object x) {
// Not exhaustive
bool? y;
switch (x) {
case [...]:
y = true;
}
y.expectStaticType<Exactly<bool?>>();
}
void testListUnrelatedType(List<Object> x) {
// Not exhaustive
bool? y;
switch (x) {
case <int?>[...]:
y = true;
}
y.expectStaticType<Exactly<bool?>>();
}
void testMap(Map<Object, Object> x) {
// Not exhaustive
bool? y;
switch (x) {
case {0: _}:
y = true;
}
y.expectStaticType<Exactly<bool?>>();
}
void testLogicalAndBothMatch(Object x) {
// Trivially exhaustive because both subpatterns always match
bool? y;
switch (x) {
case _ && _:
y = true;
}
y.expectStaticType<Exactly<bool>>();
}
void testLogicalAndLhsMatches(Object x) {
// Not exhaustive because only the LHS always matches
bool? y;
switch (x) {
case _ && int _:
y = true;
}
y.expectStaticType<Exactly<bool?>>();
}
void testLogicalAndRhsMatches(Object x) {
// Not exhaustive because only the RHS always matches
bool? y;
switch (x) {
case _ && int _:
y = true;
}
y.expectStaticType<Exactly<bool?>>();
}
void testLogicalAndNeitherMatches(Object x) {
// Not exhaustive because neither side always matches
bool? y;
switch (x) {
case int _ && String _:
y = true;
}
y.expectStaticType<Exactly<bool?>>();
}
void testLogicalOrBothMatch(Object x) {
// Trivially exhaustive because both subpatterns always match
bool? y;
switch (x) {
case _ || _:
y = true;
}
y.expectStaticType<Exactly<bool>>();
}
void testLogicalOrLhsMatches(Object x) {
// Trivially exhaustive because the LHS always matches
bool? y;
switch (x) {
case _ || int _:
y = true;
}
y.expectStaticType<Exactly<bool>>();
}
void testLogicalOrRhsMatches(Object x) {
// Trivially exhaustive because the RHS always matches
bool? y;
switch (x) {
case _ || int _:
y = true;
}
y.expectStaticType<Exactly<bool>>();
}
void testLogicalOrNeitherMatches(Object x) {
// Not exhaustive because neither side always matches
bool? y;
switch (x) {
case int _ || String _:
y = true;
}
y.expectStaticType<Exactly<bool?>>();
}
void testNullCheckAlwaysMatches(Object x) {
// TODO(paulberry): should be trivially exhaustive because the matched value
// type is non-nullable and the subpattern always matches
bool? y;
switch (x) {
case _?:
// ^
// [analyzer] STATIC_WARNING.UNNECESSARY_NULL_CHECK_PATTERN
// [cfe] The null-check pattern will have no effect because the matched type isn't nullable.
y = true;
}
y.expectStaticType<Exactly<bool?>>();
}
void testNullCheckNullableMatchedValueType(Object? x) {
// Not exhaustive because the matched value type is nullable
bool? y;
switch (x) {
case _?:
y = true;
}
y.expectStaticType<Exactly<bool?>>();
}
void testNullCheckSubpatternMayFailToMatch(Object x) {
// Not exhaustive because the subpattern may fail to match
bool? y;
switch (x) {
case int _?:
// ^
// [analyzer] STATIC_WARNING.UNNECESSARY_NULL_CHECK_PATTERN
// [cfe] The null-check pattern will have no effect because the matched type isn't nullable.
y = true;
}
y.expectStaticType<Exactly<bool?>>();
}
void testNullAssertSubpatternAlwaysMatches(Object? x) {
// Trivially exhaustive because the subpattern always matches
bool? y;
switch (x) {
case _!:
y = true;
}
y.expectStaticType<Exactly<bool>>();
}
void testNullAssertSubpatternAlwaysMatchesObjectPattern(bool? x) {
// Trivially exhaustive because the subpattern always matches
bool? y;
switch (x) {
case bool()!:
y = true;
}
y.expectStaticType<Exactly<bool>>();
}
void testNullAssertSubpatternMayFailToMatch(Object x) {
// Not exhaustive because the subpattern may fail to match
bool? y;
switch (x) {
case int _!:
// ^
// [analyzer] STATIC_WARNING.UNNECESSARY_NULL_ASSERT_PATTERN
// [cfe] The null-assert pattern will have no effect because the matched type isn't nullable.
y = true;
}
y.expectStaticType<Exactly<bool?>>();
}
void testObjectSubtype(Object x) {
// Not exhaustive
bool? y;
switch (x) {
case int():
y = true;
}
y.expectStaticType<Exactly<bool?>>();
}
void testObjectSupertype(Object x) {
// Trivially exhaustive
bool? y;
switch (x) {
case dynamic():
y = true;
}
y.expectStaticType<Exactly<bool>>();
}
void testObjectUnrelatedType(List<String> x) {
// Not exhaustive
bool? y;
switch (x) {
case List<int>():
y = true;
}
y.expectStaticType<Exactly<bool?>>();
}
void testObjectSubpatternAlwaysMatches(Object x) {
// Trivially exhaustive because the hashCode always matches
bool? y;
switch (x) {
case Object(hashCode: _):
y = true;
}
y.expectStaticType<Exactly<bool>>();
}
void testObjectSubpatternMayFailToMatch(Object x) {
// Not exhaustive because the hashCode may fail to match
bool? y;
switch (x) {
case Object(hashCode: == 0):
y = true;
}
y.expectStaticType<Exactly<bool?>>();
}
void testObjectTwoSubpatternsBothMatch(Object x) {
// Trivially exhaustive because both subpatterns always match
bool? y;
switch (x) {
case Object(hashCode: _, runtimeType: _):
y = true;
}
y.expectStaticType<Exactly<bool>>();
}
void testObjectTwoSubpatternsFirstMatches(Object x) {
// Not exhaustive because the runtimeType may not match
bool? y;
switch (x) {
case Object(hashCode: _, runtimeType: == int):
y = true;
}
y.expectStaticType<Exactly<bool?>>();
}
void testObjectTwoSubpatternsSecondMatches(Object x) {
// Not exhaustive because the hashCode may not match
bool? y;
switch (x) {
case Object(hashCode: == 0, runtimeType: _):
y = true;
}
y.expectStaticType<Exactly<bool?>>();
}
void testObjectTwoSubpatternsNeitherMatches(Object x) {
// Not exhaustive because neither subpattern always matches
bool? y;
switch (x) {
case Object(hashCode: == 0, runtimeType: == int):
y = true;
}
y.expectStaticType<Exactly<bool?>>();
}
void testRecordSubtype(Object x) {
// Not exhaustive
bool? y;
switch (x) {
case (_, _):
y = true;
}
y.expectStaticType<Exactly<bool?>>();
}
void testRecordMatchingType((Object, Object) x) {
// Trivially exhaustive
bool? y;
switch (x) {
case (_, _):
y = true;
}
y.expectStaticType<Exactly<bool>>();
}
void testRecordUnrelatedType(List<String> x) {
// Not exhaustive
bool? y;
switch (x) {
case (_, _):
y = true;
}
y.expectStaticType<Exactly<bool?>>();
}
void testRecordSubpatternAlwaysMatches((Object,) x) {
// Trivially exhaustive because the subpattern always matches
bool? y;
switch (x) {
case (_,):
y = true;
}
y.expectStaticType<Exactly<bool>>();
}
void testRecordSubpatternMayFailToMatch((Object,) x) {
// Not exhaustive because the hashCode may fail to match
bool? y;
switch (x) {
case (int _,):
y = true;
}
y.expectStaticType<Exactly<bool?>>();
}
void testRecordTwoSubpatternsBothMatch((Object, Object) x) {
// Trivially exhaustive because both subpatterns always match
bool? y;
switch (x) {
case (_, _):
y = true;
}
y.expectStaticType<Exactly<bool>>();
}
void testRecordTwoSubpatternsFirstMatches((Object, Object) x) {
// Not exhaustive because the second subpattern may not match
bool? y;
switch (x) {
case (_, int _):
y = true;
}
y.expectStaticType<Exactly<bool?>>();
}
void testRecordTwoSubpatternsSecondMatches((Object, Object) x) {
// Not exhaustive because the first subpattern may not match
bool? y;
switch (x) {
case (int _, _):
y = true;
}
y.expectStaticType<Exactly<bool?>>();
}
void testRecordTwoSubpatternsNeitherMatches((Object, Object) x) {
// Not exhaustive because neither subpattern always matches
bool? y;
switch (x) {
case (int _, int _):
y = true;
}
y.expectStaticType<Exactly<bool?>>();
}
void testVariableSubtype(Object x) {
// Not exhaustive because Object !<: int
bool? y;
switch (x) {
case int v:
y = true;
}
y.expectStaticType<Exactly<bool?>>();
}
void testVariableSupertype(Object x) {
// Trivially exhaustive because Object <: Object?
bool? y;
switch (x) {
case Object? v:
y = true;
}
y.expectStaticType<Exactly<bool>>();
}
void testVariableUnrelatedType(List<String> x) {
// Not exhaustive because List<String> !<: List<int>
bool? y;
switch (x) {
case List<int> v:
y = true;
}
y.expectStaticType<Exactly<bool?>>();
}
void testVariableUntyped(Object x) {
// Trivially exhaustive because an untyped variable always matches
bool? y;
switch (x) {
case var v:
y = true;
}
y.expectStaticType<Exactly<bool>>();
}
void testWildcardSubtype(Object x) {
// Not exhaustive because Object !<: int
bool? y;
switch (x) {
case int _:
y = true;
}
y.expectStaticType<Exactly<bool?>>();
}
void testWildcardSupertype(Object x) {
// Trivially exhaustive because Object <: Object?
bool? y;
switch (x) {
case Object? _:
y = true;
}
y.expectStaticType<Exactly<bool>>();
}
void testWildcardUnrelatedType(List<String> x) {
// Not exhaustive because List<String> !<: List<int>
bool? y;
switch (x) {
case List<int> _:
y = true;
}
y.expectStaticType<Exactly<bool?>>();
}
void testWildcardUntyped(Object x) {
// Trivially exhaustive because an untyped wildcard always matches
bool? y;
switch (x) {
case _:
y = true;
}
y.expectStaticType<Exactly<bool>>();
}
void testRelationalNotEqualsNullWithNonNullableScrutinee(Object x) {
// TODO(paulberry): this should be trivially exhaustive
bool? y;
switch (x) {
case != null:
y = true;
}
y.expectStaticType<Exactly<bool?>>();
}
void testRelationalNotEqualsNullWithNullableScrutinee(Object? x) {
// Not exhaustive
bool? y;
switch (x) {
case != null:
y = true;
}
y.expectStaticType<Exactly<bool?>>();
}
void testRelationalEqualsNullWithNullScrutinee(Null x) {
// Trivially exhaustive
bool? y;
switch (x) {
//^^^^^^
// [analyzer] COMPILE_TIME_ERROR.NON_EXHAUSTIVE_SWITCH_STATEMENT
// ^
// [cfe] The type 'Null' is not exhaustively matched by the switch cases since it doesn't match 'null'.
case == null:
y = true;
}
y.expectStaticType<Exactly<bool>>();
}
void testRelationalEqualsNullWithOtherScrutinee(Object x) {
// Not exhaustive
bool? y;
switch (x) {
case == null:
y = true;
}
y.expectStaticType<Exactly<bool?>>();
}
main() {}