From d0fc6ec27bbd7507d5abd5fcfba917208706a9d3 Mon Sep 17 00:00:00 2001
From: Paul Berry <paulberry@google.com>
Date: Fri, 9 Dec 2022 17:20:38 +0000
Subject: [PATCH] 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>
---
 .../lib/src/flow_analysis/flow_analysis.dart  | 35 ++++++++++++++++---
 .../lib/src/type_inference/type_analyzer.dart |  2 ++
 .../flow_analysis/flow_analysis_test.dart     | 27 ++++++++++++++
 pkg/nnbd_migration/lib/src/edge_builder.dart  |  1 +
 4 files changed, 60 insertions(+), 5 deletions(-)

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 22267b8897e..a67485318d6 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
@@ -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() {}
 
diff --git a/pkg/_fe_analyzer_shared/lib/src/type_inference/type_analyzer.dart b/pkg/_fe_analyzer_shared/lib/src/type_inference/type_analyzer.dart
index 1b4650fb943..1684383db98 100644
--- a/pkg/_fe_analyzer_shared/lib/src/type_inference/type_analyzer.dart
+++ b/pkg/_fe_analyzer_shared/lib/src/type_inference/type_analyzer.dart
@@ -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)
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 c67f144499b..8b005a0eb87 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
@@ -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:', () {
diff --git a/pkg/nnbd_migration/lib/src/edge_builder.dart b/pkg/nnbd_migration/lib/src/edge_builder.dart
index 5432b2144de..e4388342797 100644
--- a/pkg/nnbd_migration/lib/src/edge_builder.dart
+++ b/pkg/nnbd_migration/lib/src/edge_builder.dart
@@ -1956,6 +1956,7 @@ class EdgeBuilder extends GeneralizingAstVisitor<DecoratedType>
           hasDefault = true;
         }
         _dispatchList(member.statements);
+        _flowAnalysis!.switchStatement_afterCase();
       });
     }
     _flowAnalysis!.switchStatement_end(hasDefault);