commit a3c25a6ce14156bad75f9fd971ffc58320d28c9f
author Laurent Le Brun <laurentlb@google.com> Wed Oct 26 10:59:09 2016 +0000
committer John Cater <jcater@google.com> Wed Oct 26 11:11:22 2016 +0000
tree e040081d01f4b501218ae19030e70a7dce2ad9e3
parent b25a03452fa414660ea2f326e3d2b2c06c1ca548

Cleanup in the parser

- Move break/continue check from ValidationEnvironment to the Parser
- Remove some differences between BUILD / Skylark parsing mode
- Fix location off-by-one error in the break/continue tokens
- Remove duplicated error message ('for loops are not allowed on top-level')

--
MOS_MIGRATED_REVID=137259929

src/main/java/com/google/devtools/build/lib/syntax/Parser.java

diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Parser.java b/src/main/java/com/google/devtools/build/lib/syntax/Parser.java
index 4101062..8b28b3e 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/Parser.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/Parser.java
@@ -135,6 +135,7 @@
 
   private Token token; // current lookahead token
   private Token pushedToken = null; // used to implement LL(2)
+  private int loopCount; // break/continue keywords can be used only inside a loop
 
   private static final boolean DEBUGGING = false;
 
@@ -1171,8 +1172,7 @@
     int start = token.left;
     if (token.kind == TokenKind.RETURN) {
       return parseReturnStatement();
-    } else if ((parsingMode == SKYLARK)
-        && (token.kind == TokenKind.BREAK || token.kind == TokenKind.CONTINUE)) {
+    } else if (token.kind == TokenKind.BREAK || token.kind == TokenKind.CONTINUE) {
       return parseFlowStatement(token.kind);
     }
     Expression expression = parseExpression();
@@ -1232,9 +1232,14 @@
     expect(TokenKind.IN);
     Expression collection = parseExpression();
     expect(TokenKind.COLON);
-    List<Statement> block = parseSuite();
-    Statement stmt = new ForStatement(loopVar, collection, block);
-    list.add(setLocation(stmt, start, token.right));
+    enterLoop();
+    try {
+      List<Statement> block = parseSuite();
+      Statement stmt = new ForStatement(loopVar, collection, block);
+      list.add(setLocation(stmt, start, token.right));
+    } finally {
+      exitLoop();
+    }
   }
 
   // def foo(bar1, bar2):
@@ -1401,10 +1406,16 @@
   // flow_stmt ::= break_stmt | continue_stmt
   private FlowStatement parseFlowStatement(TokenKind kind) {
     int start = token.left;
+    int end = token.right;
     expect(kind);
+    if (loopCount == 0) {
+      reportError(
+          lexer.createLocation(start, end),
+          kind.getPrettyName() + " statement must be inside a for loop");
+    }
     FlowStatement.Kind flowKind =
         kind == TokenKind.BREAK ? FlowStatement.Kind.BREAK : FlowStatement.Kind.CONTINUE;
-    return setLocation(new FlowStatement(flowKind), start, token.right);
+    return setLocation(new FlowStatement(flowKind), start, end);
   }
 
   // return_stmt ::= RETURN [expr]
@@ -1429,7 +1440,7 @@
     int start = token.left;
     Token blockToken = token;
     syncTo(EnumSet.of(TokenKind.COLON, TokenKind.EOF)); // skip over expression or name
-    if (blockToken.kind == TokenKind.ELSE && parsingMode == SKYLARK) {
+    if (blockToken.kind == TokenKind.ELSE) {
       reportError(
           lexer.createLocation(blockToken.left, blockToken.right),
           "syntax error at 'else': not allowed here.");
@@ -1450,4 +1461,13 @@
   private void makeComment(Token token) {
     comments.add(setLocation(new Comment((String) token.value), token.left, token.right));
   }
+
+  private void enterLoop() {
+    loopCount++;
+  }
+
+  private void exitLoop() {
+    Preconditions.checkState(loopCount > 0);
+    loopCount--;
+  }
 }