diff options
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()) { |