In analyzer, catch use of continue and break in inappropriate places

compiler/java/com/google/dart/compiler/parser/DartParser.java

Index: compiler/java/com/google/dart/compiler/parser/DartParser.java
diff --git a/compiler/java/com/google/dart/compiler/parser/DartParser.java b/compiler/java/com/google/dart/compiler/parser/DartParser.java
index f9cd10098f342f64d33ff83e7b40b21dba40514a..3469d04b9f0f668141da8367e2aa6b0242510b84 100644
--- a/compiler/java/com/google/dart/compiler/parser/DartParser.java
+++ b/compiler/java/com/google/dart/compiler/parser/DartParser.java
@@ -221,6 +221,12 @@ public class DartParser extends CompletionHooksParserBase {
    * {@link #setAllowFunctionExpression(boolean)}.
    */
   private boolean allowFunctionExpression = true;
+  
+  /**
+   * breaks and continues are not valid just anywhere, you need to be inside a loop
+   * or a case stmt.
+   */
+  private boolean inBreakableStatement = false;

[Brian Wilkerson, 2012/06/08 18:49:05]

Looking at the spec, it seems that there is a difference between a "breakable"
statement and a "continuable" statement. I think we need to maintain both pieces
of state.

Both flags need to be reset to false while processing a nested function.

[zundel, 2012/06/08 20:15:08]

I realized this halfway through the patch.  I had two different state variables
at first, but there is already handling for labels not matching up, so I'm
letting all breaks with labels on them fall through without comment and allow
the resolver to catch the problem.  This is better, I think, because a break
with no label is still not valid, so setting the 'isBreakable' flag when you see
a label will falsely ignore problems in some cases.

I did some renaming and added a comment above.

 
   /**
    * Set the {@link #allowFunctionExpression} flag indicating whether function expressions are
@@ -3181,6 +3187,9 @@ public class DartParser extends CompletionHooksParserBase {
     DartIdentifier label = null;
     if (match(Token.IDENTIFIER)) {
       label = parseIdentifier();
+    } else if (!inBreakableStatement) {
+      // The validation of matching of labels to break statements is done later.
+      reportErrorWithoutAdvancing(ParserErrorCode.BREAK_OUTSIDE_OF_LOOP);      
     }
     expectStatmentTerminator();
     return done(new DartBreakStatement(label));
@@ -3190,6 +3199,9 @@ public class DartParser extends CompletionHooksParserBase {
     beginContinueStatement();
     expect(Token.CONTINUE);
     DartIdentifier label = null;
+    if (!inBreakableStatement) {
+      reportErrorWithoutAdvancing(ParserErrorCode.CONTINUE_OUTSIDE_OF_LOOP);      
+    }
     if (peek(0) == Token.IDENTIFIER) {
       label = parseIdentifier();
     }
@@ -3653,7 +3665,7 @@ public class DartParser extends CompletionHooksParserBase {
     expect(Token.LPAREN);
     DartExpression condition = parseExpression();
     expectCloseParen();
-    DartStatement body = parseStatement();
+    DartStatement body = parseBreakableStatement();
     return done(new DartWhileStatement(condition, body));
   }
 
@@ -3669,7 +3681,7 @@ public class DartParser extends CompletionHooksParserBase {
   private DartDoWhileStatement parseDoWhileStatement() {
     beginDoStatement();
     expect(Token.DO);
-    DartStatement body = parseStatement();
+    DartStatement body = parseBreakableStatement();
     expect(Token.WHILE);
     expect(Token.LPAREN);
     DartExpression condition = parseExpression();
@@ -3679,6 +3691,20 @@ public class DartParser extends CompletionHooksParserBase {
   }
 
   /**
+   * Use this wrapper to parse the body of a loop or case statement.
+   * 
+   * Sets up flag variables to make sure continue and break are properly 
+   * marked as errors when in wrong context. 
+   */
+  private DartStatement parseBreakableStatement() {

[Brian Wilkerson, 2012/06/08 18:49:05]

This is fine for both "breakable" and "continuable" statement, but I think we
also need to change parseStatement() to mark a statement as "breakable" if there
is a label before the statement.

[zundel, 2012/06/08 20:15:08]

Renamed, this just records the fact that your in a loop or case stmt and it is
up to the break and continue parsers to react accordingly.

+    boolean oldInBreakable = inBreakableStatement;
+    inBreakableStatement = true;
+    DartStatement stmt = parseStatement();
+    inBreakableStatement = oldInBreakable;
+    return stmt;
+  }
+
+  /**
    * <pre>
    * iterationStatement
    *     : WHILE '(' expression ')' statement
@@ -3745,7 +3771,7 @@ public class DartParser extends CompletionHooksParserBase {
       DartExpression iterable = parseExpression();
       expectCloseParen();
 
-      DartStatement body = parseStatement();
+      DartStatement body = parseBreakableStatement();
       return done(new DartForInStatement(setup, iterable, body));
 
     } else if (optional(Token.SEMICOLON)) {
@@ -3764,7 +3790,7 @@ public class DartParser extends CompletionHooksParserBase {
       }
       expectCloseParen();
 
-      DartStatement body = parseStatement();
+      DartStatement body = parseBreakableStatement();
       return done(new DartForStatement(setup, condition, next, body));
     } else {
       reportUnexpectedToken(position(), null, peek(0));
@@ -3812,7 +3838,7 @@ public class DartParser extends CompletionHooksParserBase {
         case EOS:
           return statements;
         default:
-          if ((statement = parseStatement()) == null) {
+          if ((statement = parseBreakableStatement()) == null) {
             return statements;
           }
           statements.add(statement);