Make the parser look at only one Token at a time.

Remove all references to `Token` (except one) in the parser. In particular,
remove the mechanism to push tokens back. This change will make possible for
the lexer to reuse tokens instead of allocating new objects each time.

RELNOTES: None.
PiperOrigin-RevId: 197585185

3 files changed, 68 insertions, 83 deletions

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 155c05a6a8..d593dc0d36 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
@@ -108,8 +108,8 @@ public class Parser {
           TokenKind.RPAREN,
           TokenKind.SLASH);
 
-  private Token token; // current lookahead token
-  private Token pushedToken = null; // used to implement LL(2)
+  /** Current lookahead token. May be mutated by the parser. */
+  private Token token;
 
   private static final boolean DEBUGGING = false;
 
@@ -300,7 +300,7 @@ public class Parser {
     }
   }
 
-  private void syntaxError(Token token, String message) {
+  private void syntaxError(String message) {
     if (!recoveryMode) {
       String msg = token.kind == TokenKind.INDENT
           ? "indentation error"
@@ -317,7 +317,7 @@ public class Parser {
   private boolean expect(TokenKind kind) {
     boolean expected = token.kind == kind;
     if (!expected) {
-      syntaxError(token, "expected " + kind.getPrettyName());
+      syntaxError("expected " + kind.getPrettyName());
     }
     nextToken();
     return expected;
@@ -391,7 +391,7 @@ public class Parser {
           TokenKind.WHILE,
           TokenKind.YIELD);
 
-  private void checkForbiddenKeywords(Token token) {
+  private void checkForbiddenKeywords() {
     if (!FORBIDDEN_KEYWORDS.contains(token.kind)) {
       return;
     }
@@ -413,33 +413,20 @@ public class Parser {
   }
 
   private void nextToken() {
-    if (pushedToken != null) {
-      token = pushedToken;
-      pushedToken = null;
-    } else {
-      if (token == null || token.kind != TokenKind.EOF) {
+    if (token == null || token.kind != TokenKind.EOF) {
+      token = lexer.nextToken();
+      // transparently handle comment tokens
+      while (token.kind == TokenKind.COMMENT) {
+        makeComment();
         token = lexer.nextToken();
-        // transparently handle comment tokens
-        while (token.kind == TokenKind.COMMENT) {
-          makeComment(token);
-          token = lexer.nextToken();
-        }
       }
     }
-    checkForbiddenKeywords(token);
+    checkForbiddenKeywords();
     if (DEBUGGING) {
       System.err.print(token);
     }
   }
 
-  private void pushToken(Token tokenToPush) {
-    if (pushedToken != null) {
-      throw new IllegalStateException("Exceeded LL(2) lookahead!");
-    }
-    pushedToken = token;
-    token = tokenToPush;
-  }
-
   // create an error expression
   private Identifier makeErrorExpression(int start, int end) {
     return setLocation(new Identifier("$error$"), start, end);
@@ -463,33 +450,32 @@ public class Parser {
   //       | **kwargs
   private Argument.Passed parseFuncallArgument() {
     final int start = token.left;
+    Expression expr;
     // parse **expr
     if (token.kind == TokenKind.STAR_STAR) {
       nextToken();
-      Expression expr = parseNonTupleExpression();
+      expr = parseNonTupleExpression();
       return setLocation(new Argument.StarStar(expr), start, expr);
     }
     // parse *expr
     if (token.kind == TokenKind.STAR) {
       nextToken();
-      Expression expr = parseNonTupleExpression();
+      expr = parseNonTupleExpression();
       return setLocation(new Argument.Star(expr), start, expr);
     }
-    // parse keyword = expr
-    if (token.kind == TokenKind.IDENTIFIER) {
-      Token identToken = token;
-      String name = (String) token.value;
-      nextToken();
-      if (token.kind == TokenKind.EQUALS) { // it's a named argument
+
+    expr = parseNonTupleExpression();
+    if (expr instanceof Identifier) {
+      // parse a named argument
+      if (token.kind == TokenKind.EQUALS) {
+        String name = ((Identifier) expr).getName();
         nextToken();
-        Expression expr = parseNonTupleExpression();
-        return setLocation(new Argument.Keyword(name, expr), start, expr);
-      } else { // oops, back up!
-        pushToken(identToken);
+        Expression val = parseNonTupleExpression();
+        return setLocation(new Argument.Keyword(name, val), start, val);
       }
     }
+
     // parse a positional argument
-    Expression expr = parseNonTupleExpression();
     return setLocation(new Argument.Positional(expr), start, expr);
   }
 
@@ -550,7 +536,7 @@ public class Parser {
       Identifier ident = parseIdent();
       return setLocation(new DotExpression(receiver, ident), start, ident);
     } else {
-      syntaxError(token, "expected identifier after dot");
+      syntaxError("expected identifier after dot");
       int end = syncTo(EXPR_TERMINATOR_SET);
       return makeErrorExpression(start, end);
     }
@@ -686,7 +672,7 @@ public class Parser {
         }
       default:
         {
-          syntaxError(token, "expected expression");
+          syntaxError("expected expression");
           int end = syncTo(EXPR_TERMINATOR_SET);
           return makeErrorExpression(start, end);
         }
@@ -807,7 +793,7 @@ public class Parser {
         nextToken();
         return expr;
       } else {
-        syntaxError(token, "expected '" + closingBracket.getPrettyName() + "', 'for' or 'if'");
+        syntaxError("expected '" + closingBracket.getPrettyName() + "', 'for' or 'if'");
         syncPast(LIST_TERMINATOR_SET);
         return makeErrorExpression(comprehensionStartOffset, token.right);
       }
@@ -866,7 +852,7 @@ public class Parser {
         }
       default:
         {
-          syntaxError(token, "expected ',', 'for' or ']'");
+          syntaxError("expected ',', 'for' or ']'");
           int end = syncPast(LIST_TERMINATOR_SET);
           return makeErrorExpression(start, end);
         }
@@ -939,8 +925,10 @@ public class Parser {
         // If NOT appears when we expect a binary operator, it must be followed by IN.
         // Since the code expects every operator to be a single token, we push a NOT_IN token.
         expect(TokenKind.NOT);
-        expect(TokenKind.IN);
-        pushToken(new Token(TokenKind.NOT_IN, token.left, token.right));
+        if (token.kind != TokenKind.IN) {
+          syntaxError("expected 'in'");
+        }
+        token.kind = TokenKind.NOT_IN;
       }
 
       if (!binaryOperators.containsKey(token.kind)) {
@@ -1056,7 +1044,7 @@ public class Parser {
 
     StringLiteral importString = parseStringLiteral();
     if (token.kind == TokenKind.RPAREN) {
-      syntaxError(token, "expected at least one symbol to load");
+      syntaxError("expected at least one symbol to load");
       return;
     }
     expect(TokenKind.COMMA);
@@ -1088,43 +1076,33 @@ public class Parser {
    * entry in the map.
    */
   private void parseLoadSymbol(Map<Identifier, String> symbols) {
-    Token nameToken;
-    Token declaredToken;
+    if (token.kind != TokenKind.STRING && token.kind != TokenKind.IDENTIFIER) {
+      syntaxError("expected either a literal string or an identifier");
+      return;
+    }
+
+    String name = (String) token.value;
+    Identifier identifier = new Identifier(name);
+    if (symbols.containsKey(identifier)) {
+      syntaxError(
+          String.format("Identifier '%s' is used more than once", identifier.getName()));
+    }
+    setLocation(identifier, token.left, token.right);
 
+    String declared;
     if (token.kind == TokenKind.STRING) {
-      nameToken = token;
-      declaredToken = nameToken;
+      declared = name;
     } else {
-      if (token.kind != TokenKind.IDENTIFIER) {
-        syntaxError(token, "Expected either a literal string or an identifier");
-      }
-
-      nameToken = token;
-
       expect(TokenKind.IDENTIFIER);
       expect(TokenKind.EQUALS);
-
-      declaredToken = token;
-    }
-
-    expect(TokenKind.STRING);
-
-    try {
-      Identifier identifier = new Identifier(nameToken.value.toString());
-
-      if (symbols.containsKey(identifier)) {
-        syntaxError(
-            nameToken, String.format("Identifier '%s' is used more than once",
-                identifier.getName()));
-      } else {
-        symbols.put(
-            setLocation(identifier, nameToken.left, nameToken.right),
-            declaredToken.value.toString());
+      if (token.kind != TokenKind.STRING) {
+        syntaxError("expected string");
+        return;
       }
-    } catch (NullPointerException npe) {
-      // This means that the value of at least one token is null. In this case, the previous
-      // expect() call has already logged an error.
+      declared = token.value.toString();
     }
+    nextToken();
+    symbols.put(identifier, declared);
   }
 
   private void parseTopLevelStatement(List<Statement> list) {
@@ -1368,7 +1346,7 @@ public class Parser {
   }
 
   // create a comment node
-  private void makeComment(Token token) {
+  private void makeComment() {
     comments.add(setLocation(new Comment((String) token.value), token.left, token.right));
   }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Token.java b/src/main/java/com/google/devtools/build/lib/syntax/Token.java
index aef2d0e11a..3c99df5f8c 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/Token.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/Token.java
@@ -13,22 +13,29 @@
 // limitations under the License.
 package com.google.devtools.build.lib.syntax;
 
+import javax.annotation.Nullable;
+
 /**
  * A Token represents an actual lexeme; that is, a lexical unit, its location in
  * the input text, its lexical kind (TokenKind) and any associated value.
  */
-public class Token {
+class Token {
 
-  public final TokenKind kind;
-  public final int left;
-  public final int right;
-  public final Object value;
+  TokenKind kind;
+  final int left;
+  final int right;
+  /**
+   * value is an Integer if the kind is INT.
+   * It is a String if the kind is STRING, IDENTIFIER, or COMMENT.
+   * It is null otherwise.
+   */
+  @Nullable final Object value;
 
-  public Token(TokenKind kind, int left, int right) {
+  Token(TokenKind kind, int left, int right) {
     this(kind, left, right, null);
   }
 
-  public Token(TokenKind kind, int left, int right, Object value) {
+  Token(TokenKind kind, int left, int right, Object value) {
     this.kind = kind;
     this.left = left;
     this.right = right;
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/ValidationTest.java b/src/test/java/com/google/devtools/build/lib/syntax/ValidationTest.java
index 518e871391..3a003b1d66 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/ValidationTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/ValidationTest.java
@@ -212,7 +212,7 @@ public class ValidationTest extends EvaluationTestCase {
     setFailFast(false);
     parseFile(
         "def GenerateMapNames():", "  a = 2", "  b = [3, 4]", "  if a not b:", "    print(a)");
-    assertContainsError("syntax error at 'b': expected in");
+    assertContainsError("syntax error at 'b': expected 'in'");
     // Parser uses "$error" symbol for error recovery.
     // It should not be used in error messages.
     for (Event event : getEventCollector()) {