ExitDetector: Examine continue when determining a 'do'

pkg/analyzer/lib/src/generated/resolver.dart

Index: pkg/analyzer/lib/src/generated/resolver.dart
diff --git a/pkg/analyzer/lib/src/generated/resolver.dart b/pkg/analyzer/lib/src/generated/resolver.dart
index 33dc56e933bfaa1926d071f466bb3efd263c306d..b5c87c87890ecbef6835bf9e062d11c3af3e8b23 100644
--- a/pkg/analyzer/lib/src/generated/resolver.dart
+++ b/pkg/analyzer/lib/src/generated/resolver.dart
@@ -3499,6 +3499,12 @@ class ExitDetector extends GeneralizingAstVisitor<bool> {
   bool _enclosingBlockContainsBreak = false;
 
   /**
+   * Set to `true` when a `continue` is encountered, and reset to `false` when a
+   * `do`, `while`, `for` or `switch` block is entered.
+   */
+  bool _enclosingBlockContainsContinue = false;
+
+  /**
    * Add node when a labelled `break` is encountered.
    */
   Set<AstNode> _enclosingBlockBreaksLabel = new Set<AstNode>();
@@ -3606,14 +3612,24 @@ class ExitDetector extends GeneralizingAstVisitor<bool> {
   }
 
   @override
-  bool visitContinueStatement(ContinueStatement node) => false;
+  bool visitContinueStatement(ContinueStatement node) {
+    _enclosingBlockContainsContinue = true;
+    return false;
+  }
 
   @override
   bool visitDoStatement(DoStatement node) {
     bool outerBreakValue = _enclosingBlockContainsBreak;
+    bool outerContinueValue = _enclosingBlockContainsContinue;
     _enclosingBlockContainsBreak = false;
+    _enclosingBlockContainsContinue = false;
     try {
-      if (_nodeExits(node.body) && !_enclosingBlockContainsBreak) {
+      bool bodyExits = _nodeExits(node.body);
+      bool containsBreakOrContinue =
+          _enclosingBlockContainsBreak || _enclosingBlockContainsContinue;
+      // Even if we determine that the body "exits", there might be break or
+      // continue statements that actually mean it _doesn't_ always exit.
+      if (bodyExits && !containsBreakOrContinue) {
         return true;
       }
       Expression conditionExpression = node.condition;
@@ -3630,6 +3646,7 @@ class ExitDetector extends GeneralizingAstVisitor<bool> {
       return false;
     } finally {
       _enclosingBlockContainsBreak = outerBreakValue;
+      _enclosingBlockContainsContinue = outerContinueValue;
     }
   }
 
@@ -3643,18 +3660,23 @@ class ExitDetector extends GeneralizingAstVisitor<bool> {
   @override
   bool visitForEachStatement(ForEachStatement node) {
     bool outerBreakValue = _enclosingBlockContainsBreak;
+    bool outerContinueValue = _enclosingBlockContainsContinue;
     _enclosingBlockContainsBreak = false;
+    _enclosingBlockContainsContinue = false;
     try {
       return _nodeExits(node.iterable);
     } finally {
       _enclosingBlockContainsBreak = outerBreakValue;
+      _enclosingBlockContainsContinue = outerContinueValue;
     }
   }
 
   @override
   bool visitForStatement(ForStatement node) {
     bool outerBreakValue = _enclosingBlockContainsBreak;
+    bool outerContinueValue = _enclosingBlockContainsContinue;
     _enclosingBlockContainsBreak = false;
+    _enclosingBlockContainsContinue = false;
     try {
       if (node.variables != null &&
           _visitVariableDeclarations(node.variables.variables)) {
@@ -3684,6 +3706,7 @@ class ExitDetector extends GeneralizingAstVisitor<bool> {
       return false;
     } finally {
       _enclosingBlockContainsBreak = outerBreakValue;
+      _enclosingBlockContainsContinue = outerContinueValue;
     }
   }
 
@@ -3824,7 +3847,9 @@ class ExitDetector extends GeneralizingAstVisitor<bool> {
   @override
   bool visitSwitchStatement(SwitchStatement node) {
     bool outerBreakValue = _enclosingBlockContainsBreak;
+    bool outerContinueValue = _enclosingBlockContainsContinue;

[Paul Berry, 2016/06/28 19:17:41]

I don't think this method should be modified.  Switch statements only affect the
target of "break" statements, not "continue" statements.  So for instance I
think with your code as written, we would incorrectly classify the following
code as exiting:

do {
  switch (x) {
    case 0:
      continue;
  }
  throw 'foo';
} while (false);

(Assuming I'm right about this, we should add a test case to verify that the
above code is handled correctly).

[srawlins, 2016/06/28 20:00:55]

Done. With testcase.

     _enclosingBlockContainsBreak = false;
+    _enclosingBlockContainsContinue = false;
     try {
       bool hasDefault = false;
       bool hasNonExitingCase = false;
@@ -3854,6 +3879,7 @@ class ExitDetector extends GeneralizingAstVisitor<bool> {
       return hasDefault;
     } finally {
       _enclosingBlockContainsBreak = outerBreakValue;
+      _enclosingBlockContainsContinue = outerContinueValue;
     }
   }
 
@@ -3909,7 +3935,9 @@ class ExitDetector extends GeneralizingAstVisitor<bool> {
   @override
   bool visitWhileStatement(WhileStatement node) {
     bool outerBreakValue = _enclosingBlockContainsBreak;
+    bool outerContinueValue = _enclosingBlockContainsContinue;
     _enclosingBlockContainsBreak = false;
+    _enclosingBlockContainsContinue = false;
     try {
       Expression conditionExpression = node.condition;
       if (conditionExpression.accept(this)) {
@@ -3936,6 +3964,7 @@ class ExitDetector extends GeneralizingAstVisitor<bool> {
       return false;
     } finally {
       _enclosingBlockContainsBreak = outerBreakValue;
+      _enclosingBlockContainsContinue = outerContinueValue;
     }
   }