Patterns flow analysis: fix handling of empty exhaustive switch statements.

I previously thought that the only possible situation where a switch
statement over a valid type could be exhaustive without containing any
cases was if the scrutinee type was the empty record type (`()`).  But
that isn't true at all:

- The empty record type is inhabited by empty record objects, so such
  a switch statement would not be exhaustive after all.

- It is possible for the user to create a type that can be
  exhaustively switched over with zero cases: by creating an abstract
  sealed class without any subclasses.

In the latter case, I think it makes the most sense to treat the code
after the switch as unreachable, because that's the normal behaviour
of a switch over an exhaustive type with no `break`s.  It is sound to
do so because the type is uninhabited, therefore the body of the
switch statement itself will never be reached.

Bug: https://github.com/dart-lang/sdk/issues/50419
Change-Id: Ibc0a21226f25ae155db994343874354d5b8e4f7d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/274621
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
This commit is contained in:
Paul Berry 2022-12-13 14:36:57 +00:00 committed by Commit Queue
parent 9169abc8f4
commit c430a3b4aa
2 changed files with 20 additions and 6 deletions

View file

@ -3987,10 +3987,13 @@ class _FlowAnalysisImpl<Node extends Object, Statement extends Node,
// 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;
// could happen, for instance, if the scrutinee type is an abstract sealed
// class that has no subclasses. It makes the most sense to treat the code
// after the switch as unreachable, because that's the normal behavior of a
// switch over an exhaustive type with no `break`s. It is sound to do so
// because the type is uninhabited, therefore the body of the switch
// statement itself will never be reached.
breakState ??= context._previous.setUnreachable();
_current = breakState.unsplit();
}

View file

@ -6736,9 +6736,20 @@ main() {
});
test('empty exhaustive', () {
// This can happen if a class is marked `sealed` but has no subclasses.
// Note that exhaustiveness checking of "always exhaustive" types is
// deferred until a later analysis stage (so that it can take constant
// evaluation into account), so flow analysis simply assumes that the
// switch is exhaustive without checking, and sets the
// `requiresExhaustivenessValidation` flag to let the client know that
// exhaustiveness checking must be performed later. Had this been a
// real compilation (and not just a unit test), exhaustiveness checking
// would later confirm that the class `C` has no subclasses, or report
// a compile-time error.
h.addExhaustiveness('C', true);
h.run([
switch_(expr('()'), []),
checkReachable(true),
switch_(expr('C'), [], expectRequiresExhaustivenessValidation: true),
checkReachable(false),
]);
});
});