[cfe] Handle break/continue in flow analysis

Change-Id: Ic02e60237b8eda3c98c0715e3fcc668d21e1bf62
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/126728
Reviewed-by: Dmitry Stefantsov <[email protected]>
Commit-Queue: Johnni Winther <[email protected]>

Author: johnniwinther

pkg/front_end/lib/src/fasta/kernel/body_builder.dart

@@ -2402,8 +2402,7 @@ class BodyBuilder extends ScopeListener<JumpTarget>
     exitLocalScope();
     push(block);
     if (blockKind == BlockKind.tryStatement) {
-      // This is matched by the calls to [deferNode] and [endNode] in
-      // [endTryStatement].
+      // This is matched by the call to [beginNode] in [beginBlock].
       typeInferrer?.assignedVariables?.endNode(block);
     }
   }
@@ -2624,23 +2623,30 @@ class BodyBuilder extends ScopeListener<JumpTarget>
     exitLocalScope();
     JumpTarget continueTarget = exitContinueTarget();
     JumpTarget breakTarget = exitBreakTarget();
+    List<BreakStatementImpl> continueStatements;
     if (continueTarget.hasUsers) {
       body = forest.createLabeledStatement(body);
-      continueTarget.resolveContinues(forest, body);
+      continueStatements = continueTarget.resolveContinues(forest, body);
     }
     Expression condition;
     if (conditionStatement is ExpressionStatement) {
       condition = conditionStatement.expression;
     } else {
       assert(conditionStatement is EmptyStatement);
     }
-    Statement result = forest.createForStatement(
+    Statement forStatement = forest.createForStatement(
         offsetForToken(forKeyword), variables, condition, updates, body);
     typeInferrer?.assignedVariables
-        ?.storeInfo(result, assignedVariablesNodeInfo);
+        ?.storeInfo(forStatement, assignedVariablesNodeInfo);
+    if (continueStatements != null) {
+      for (BreakStatementImpl continueStatement in continueStatements) {
+        continueStatement.targetStatement = forStatement;
+      }
+    }
+    Statement result = forStatement;
     if (breakTarget.hasUsers) {
       result = forest.createLabeledStatement(result);
-      breakTarget.resolveBreaks(forest, result);
+      breakTarget.resolveBreaks(forest, result, forStatement);
     }
     if (variableOrExpression is ParserRecovery) {
       problemInLoopOrSwitch ??= buildProblemStatement(
@@ -4397,17 +4403,24 @@ class BodyBuilder extends ScopeListener<JumpTarget>
     Statement body = popStatement();
     JumpTarget continueTarget = exitContinueTarget();
     JumpTarget breakTarget = exitBreakTarget();
+    List<BreakStatementImpl> continueStatements;
     if (continueTarget.hasUsers) {
       body = forest.createLabeledStatement(body);
-      continueTarget.resolveContinues(forest, body);
+      continueStatements = continueTarget.resolveContinues(forest, body);
     }
-    Statement result =
+    Statement doStatement =
         forest.createDoStatement(offsetForToken(doKeyword), body, condition);
     // This is matched by the [beginNode] call in [beginDoWhileStatement].
-    typeInferrer?.assignedVariables?.endNode(result);
+    typeInferrer?.assignedVariables?.endNode(doStatement);
+    if (continueStatements != null) {
+      for (BreakStatementImpl continueStatement in continueStatements) {
+        continueStatement.targetStatement = doStatement;
+      }
+    }
+    Statement result = doStatement;
     if (breakTarget.hasUsers) {
       result = forest.createLabeledStatement(result);
-      breakTarget.resolveBreaks(forest, result);
+      breakTarget.resolveBreaks(forest, result, doStatement);
     }
     exitLoopOrSwitch(result);
   }
@@ -4584,9 +4597,10 @@ class BodyBuilder extends ScopeListener<JumpTarget>
     exitLocalScope();
     JumpTarget continueTarget = exitContinueTarget();
     JumpTarget breakTarget = exitBreakTarget();
+    List<BreakStatementImpl> continueStatements;
     if (continueTarget.hasUsers) {
       body = forest.createLabeledStatement(body);
-      continueTarget.resolveContinues(forest, body);
+      continueStatements = continueTarget.resolveContinues(forest, body);
     }
     VariableDeclaration variable = buildForInVariable(forToken, lvalue);
     Expression problem = checkForInVariable(lvalue, variable, forToken);
@@ -4605,15 +4619,21 @@ class BodyBuilder extends ScopeListener<JumpTarget>
         body = combineStatements(prologue, body);
       }
     }
-    Statement result = new ForInStatement(variable, expression, body,
+    Statement forInStatement = new ForInStatement(variable, expression, body,
         isAsync: awaitToken != null)
       ..fileOffset = awaitToken?.charOffset ?? forToken.charOffset
       ..bodyOffset = body.fileOffset; // TODO(ahe): Isn't this redundant?
     typeInferrer?.assignedVariables
-        ?.storeInfo(result, assignedVariablesNodeInfo);
+        ?.storeInfo(forInStatement, assignedVariablesNodeInfo);
+    if (continueStatements != null) {
+      for (BreakStatementImpl continueStatement in continueStatements) {
+        continueStatement.targetStatement = forInStatement;
+      }
+    }
+    Statement result = forInStatement;
     if (breakTarget.hasUsers) {
       result = forest.createLabeledStatement(result);
-      breakTarget.resolveBreaks(forest, result);
+      breakTarget.resolveBreaks(forest, result, forInStatement);
     }
     if (problem != null) {
       result = combineStatements(
@@ -4661,8 +4681,14 @@ class BodyBuilder extends ScopeListener<JumpTarget>
       if (statement is! LabeledStatement) {
         statement = forest.createLabeledStatement(statement);
       }
-      target.breakTarget.resolveBreaks(forest, statement);
-      target.continueTarget.resolveContinues(forest, statement);
+      target.breakTarget.resolveBreaks(forest, statement, statement);
+      List<BreakStatementImpl> continueStatements =
+          target.continueTarget.resolveContinues(forest, statement);
+      if (continueStatements != null) {
+        for (BreakStatementImpl continueStatement in continueStatements) {
+          continueStatement.targetStatement = statement;
+        }
+      }
     }
     push(statement);
   }
@@ -4700,16 +4726,22 @@ class BodyBuilder extends ScopeListener<JumpTarget>
     Expression condition = popForValue();
     JumpTarget continueTarget = exitContinueTarget();
     JumpTarget breakTarget = exitBreakTarget();
+    List<BreakStatementImpl> continueStatements;
     if (continueTarget.hasUsers) {
       body = forest.createLabeledStatement(body);
-      continueTarget.resolveContinues(forest, body);
+      continueStatements = continueTarget.resolveContinues(forest, body);
     }
     Statement whileStatement = forest.createWhileStatement(
         offsetForToken(whileKeyword), condition, body);
+    if (continueStatements != null) {
+      for (BreakStatementImpl continueStatement in continueStatements) {
+        continueStatement.targetStatement = whileStatement;
+      }
+    }
     Statement result = whileStatement;
     if (breakTarget.hasUsers) {
       result = forest.createLabeledStatement(result);
-      breakTarget.resolveBreaks(forest, result);
+      breakTarget.resolveBreaks(forest, result, whileStatement);
     }
     exitLoopOrSwitch(result);
     // This is matched by the [beginNode] call in [beginWhileStatement].
@@ -4899,7 +4931,7 @@ class BodyBuilder extends ScopeListener<JumpTarget>
     Statement result = switchStatement;
     if (target.hasUsers) {
       result = forest.createLabeledStatement(result);
-      target.resolveBreaks(forest, result);
+      target.resolveBreaks(forest, result, switchStatement);
     }
     exitLoopOrSwitch(result);
     // This is matched by the [beginNode] call in [beginSwitchBlock].
@@ -5730,20 +5762,25 @@ class JumpTarget extends BuilderImpl {
     users.add(statement);
   }
 
-  void resolveBreaks(Forest forest, Statement target) {
+  void resolveBreaks(
+      Forest forest, Statement target, Statement targetStatement) {
     assert(isBreakTarget);
-    for (BreakStatement user in users) {
+    for (BreakStatementImpl user in users) {
       user.target = target;
+      user.targetStatement = targetStatement;
     }
     users.clear();
   }
 
-  void resolveContinues(Forest forest, Statement target) {
+  List<BreakStatementImpl> resolveContinues(Forest forest, Statement target) {
     assert(isContinueTarget);
-    for (BreakStatement user in users) {
+    List<BreakStatementImpl> statements = <BreakStatementImpl>[];
+    for (BreakStatementImpl user in users) {
       user.target = target;
+      statements.add(user);
     }
     users.clear();
+    return statements;
   }
 
   void resolveGotos(Forest forest, SwitchCase target) {
@@ -5804,12 +5841,13 @@ class LabelTarget extends BuilderImpl implements JumpTarget {
     unsupported("addGoto", charOffset, fileUri);
   }
 
-  void resolveBreaks(Forest forest, Statement target) {
-    breakTarget.resolveBreaks(forest, target);
+  void resolveBreaks(
+      Forest forest, Statement target, Statement targetStatement) {
+    breakTarget.resolveBreaks(forest, target, targetStatement);
   }
 
-  void resolveContinues(Forest forest, Statement target) {
-    continueTarget.resolveContinues(forest, target);
+  List<BreakStatementImpl> resolveContinues(Forest forest, Statement target) {
+    return continueTarget.resolveContinues(forest, target);
   }
 
   void resolveGotos(Forest forest, SwitchCase target) {

pkg/front_end/lib/src/fasta/kernel/forest.dart

@@ -323,7 +323,7 @@ class Forest {
   Statement createBreakStatement(int fileOffset, Object label) {
     assert(fileOffset != null);
     // TODO(johnniwinther): Use [label]?
-    return new BreakStatement(null)..fileOffset = fileOffset;
+    return new BreakStatementImpl(isContinue: false)..fileOffset = fileOffset;
   }
 
   /// Return a representation of a catch clause.
@@ -355,7 +355,7 @@ class Forest {
   Statement createContinueStatement(int fileOffset, Object label) {
     assert(fileOffset != null);
     // TODO(johnniwinther): Use [label]?
-    return new BreakStatement(null)..fileOffset = fileOffset;
+    return new BreakStatementImpl(isContinue: true)..fileOffset = fileOffset;
   }
 
   /// Return a representation of a do statement.

pkg/front_end/lib/src/fasta/kernel/inference_visitor.dart

@@ -339,9 +339,15 @@ class InferenceVisitor
   }
 
   @override
-  StatementInferenceResult visitBreakStatement(BreakStatement node) {
+  StatementInferenceResult visitBreakStatement(
+      covariant BreakStatementImpl node) {
     // TODO(johnniwinther): Refactor break/continue encoding.
-    inferrer.flowAnalysis.handleBreak(node.target);
+    assert(node.targetStatement != null);
+    if (node.isContinue) {
+      inferrer.flowAnalysis.handleContinue(node.targetStatement);
+    } else {
+      inferrer.flowAnalysis.handleBreak(node.targetStatement);
+    }
     return const StatementInferenceResult();
   }
 
@@ -4969,7 +4975,10 @@ class InferenceVisitor
     if (bodyResult.hasChanged) {
       node.body = bodyResult.statement..parent = node;
     }
-    inferrer.flowAnalysis.tryFinallyStatement_finallyBegin(node.finalizer);
+    // TODO(johnniwinther): Use one internal statement for try-catch-finally.
+    TreeNode body = node.body;
+    inferrer.flowAnalysis
+        .tryFinallyStatement_finallyBegin(body is TryCatch ? body.body : body);
     StatementInferenceResult finalizerResult =
         inferrer.inferStatement(node.finalizer);
     if (finalizerResult.hasChanged) {
@@ -5161,7 +5170,10 @@ class InferenceVisitor
     DartType promotedType;
     DartType declaredOrInferredType = variable.type;
     if (inferrer.isNonNullableByDefault) {
-      promotedType = inferrer.flowAnalysis.variableRead(node, variable);
+      if (!variable.isLocalFunction) {
+        // Don't promote local functions.
+        promotedType = inferrer.flowAnalysis.variableRead(node, variable);
+      }
     } else {
       bool mutatedInClosure = variable.mutatedInClosure;
       promotedType = inferrer.typePromoter
@@ -5194,6 +5206,7 @@ class InferenceVisitor
 
   @override
   StatementInferenceResult visitWhileStatement(WhileStatement node) {
+    inferrer.flowAnalysis.whileStatement_conditionBegin(node);
     InterfaceType expectedType =
         inferrer.coreTypes.boolRawType(inferrer.library.nonNullable);
     ExpressionInferenceResult conditionResult = inferrer.inferExpression(
@@ -5202,10 +5215,12 @@ class InferenceVisitor
     Expression condition =
         inferrer.ensureAssignableResult(expectedType, conditionResult);
     node.condition = condition..parent = node;
+    inferrer.flowAnalysis.whileStatement_bodyBegin(node, node.condition);
     StatementInferenceResult bodyResult = inferrer.inferStatement(node.body);
     if (bodyResult.hasChanged) {
       node.body = bodyResult.statement..parent = node;
     }
+    inferrer.flowAnalysis.whileStatement_end();
     return const StatementInferenceResult();
   }
 

pkg/front_end/lib/src/fasta/kernel/internal_ast.dart

@@ -21,7 +21,7 @@
 import 'dart:core' hide MapEntry;
 
 import 'package:kernel/ast.dart';
-
+import 'package:kernel/text/ast_to_text.dart';
 import 'package:kernel/core_types.dart';
 
 import '../fasta_codes.dart'
@@ -195,6 +195,15 @@ class ClassInferenceInfo {
   ClassInferenceInfo(this.builder);
 }
 
+class BreakStatementImpl extends BreakStatement {
+  Statement targetStatement;
+  final bool isContinue;
+
+  BreakStatementImpl({this.isContinue})
+      : assert(isContinue != null),
+        super(null);
+}
+
 enum InternalExpressionKind {
   Binary,
   Cascade,
@@ -237,8 +246,13 @@ abstract class InternalExpression extends Expression {
   InternalExpressionKind get kind;
 
   @override
-  R accept<R>(ExpressionVisitor<R> visitor) =>
-      unsupported("${runtimeType}.accept", -1, null);
+  R accept<R>(ExpressionVisitor<R> visitor) {
+    if (visitor is Printer || visitor is Precedence) {
+      // Allow visitors needed for toString.
+      return visitor.defaultExpression(this);
+    }
+    return unsupported("${runtimeType}.accept", -1, null);
+  }
 
   @override
   R accept1<R, A>(ExpressionVisitor1<R, A> visitor, A arg) =>

pkg/front_end/test/id_tests/definite_assignment_test.dart

@@ -25,12 +25,9 @@ main(List<String> args) async {
       runTest: runTestFor(
           const DefiniteAssignmentDataComputer(), [cfeNonNullableOnlyConfig]),
       skipList: [
-        // TODO(johnniwinther): Update break/continue handling to support these:
-        'do.dart',
+        // TODO(johnniwinther): Change for loop encoding to support these:
         'for.dart',
         'for_each.dart',
-        'switch.dart',
-        'while.dart',
       ]);
 }
 

pkg/front_end/test/id_tests/nullability_test.dart

@@ -19,12 +19,7 @@ main(List<String> args) async {
       createUriForFileName: createUriForFileName,
       onFailure: onFailure,
       runTest: runTestFor(
-          const NullabilityDataComputer(), [cfeNonNullableOnlyConfig]),
-      skipList: [
-        // TODO(johnniwinther): Run all nullability tests.
-        'try_finally.dart',
-        'while.dart',
-      ]);
+          const NullabilityDataComputer(), [cfeNonNullableOnlyConfig]));
 }
 
 class NullabilityDataComputer extends DataComputer<String> {

pkg/front_end/test/id_tests/type_promotion_test.dart

@@ -27,11 +27,8 @@ main(List<String> args) async {
         'bug39178.dart',
         'constructor_initializer.dart',
         'for.dart',
-        'not_promoted.dart',
         'switch.dart',
-        'try_finally.dart',
         'type_parameter.dart',
-        'while.dart',
         'write_capture.dart',
       ]);
 }