Skip to content

Commit

Permalink
Parser: Support tuples without parens.
Browse files Browse the repository at this point in the history 
In Python, tuples normally don't need parens, e.g.
  a, b = c, d
  for i, j in e: pass

However, they are sometimes required to avoid ambiguity:
  fct((x, y))
  [(1, 2), (3, 4)]

This distinction is handled with parseExpression vs parseNonTupleExpression.

--
MOS_MIGRATED_REVID=89118478
  • Loading branch information
@laurentlb @hanwen
laurentlb authored and hanwen committed Mar 20, 2015
1 parent 46c9f07 commit 5609389
Show file tree
Hide file tree
Showing 2 changed files with 92 additions and 46 deletions.
102 changes: 57 additions & 45 deletions src/main/java/com/google/devtools/build/lib/syntax/Parser.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,10 @@ public ParseResult(List<Statement> statements, List<Comment> comments, boolean c
private static final EnumSet<TokenKind> DICT_TERMINATOR_SET =
EnumSet.of(TokenKind.EOF, TokenKind.RBRACE, TokenKind.SEMI);

private static final EnumSet<TokenKind> EXPR_LIST_TERMINATOR_SET =
EnumSet.of(TokenKind.EOF, TokenKind.RBRACE, TokenKind.RBRACKET,
TokenKind.RPAREN, TokenKind.NEWLINE, TokenKind.SEMI);

private static final EnumSet<TokenKind> EXPR_TERMINATOR_SET = EnumSet.of(
TokenKind.EOF,
TokenKind.COMMA,
Expand Down Expand Up @@ -354,20 +358,20 @@ private Expression makeFuncallExpression(Expression receiver, Ident function,
return setLocation(new FuncallExpression(receiver, function, args), start, end);
}

// arg ::= IDENTIFIER '=' expr
// arg ::= IDENTIFIER '=' nontupleexpr
// | expr
private Argument.Passed parseFuncallArgument() {
final int start = token.left;
// parse **expr
if (token.kind == TokenKind.STAR_STAR) {
nextToken();
Expression expr = parseExpression();
Expression expr = parseNonTupleExpression();
return setLocation(new Argument.StarStar(expr), start, expr);
}
// parse *expr
if (token.kind == TokenKind.STAR) {
nextToken();
Expression expr = parseExpression();
Expression expr = parseNonTupleExpression();
return setLocation(new Argument.Star(expr), start, expr);
}
// parse keyword = expr
Expand All @@ -377,18 +381,18 @@ private Argument.Passed parseFuncallArgument() {
nextToken();
if (token.kind == TokenKind.EQUALS) { // it's a named argument
nextToken();
Expression expr = parseExpression();
Expression expr = parseNonTupleExpression();
return setLocation(new Argument.Keyword(name, expr), start, expr);
} else { // oops, back up!
pushToken(identToken);
}
}
// parse a positional argument
Expression expr = parseExpression();
Expression expr = parseNonTupleExpression();
return setLocation(new Argument.Positional(expr), start, expr);
}

// arg ::= IDENTIFIER '=' expr
// arg ::= IDENTIFIER '=' nontupleexpr
// | IDENTIFIER
private Parameter<Expression, Expression> parseFunctionParameter() {
// TODO(bazel-team): optionally support type annotations
Expand All @@ -412,7 +416,7 @@ private Parameter<Expression, Expression> parseFunctionParameter() {
Ident ident = parseIdent();
if (token.kind == TokenKind.EQUALS) { // there's a default value
nextToken();
Expression expr = parseExpression();
Expression expr = parseNonTupleExpression();
return setLocation(new Parameter.Optional<Expression, Expression>(
ident.getName(), expr), start, expr);
} else {
Expand Down Expand Up @@ -472,17 +476,19 @@ private List<Argument.Passed> parseFuncallArguments() {
return arguments;
}

// expr_list ::= ( (expr ',')* expr ','? )?
// expr_list parses a comma-separated list of expression. It assumes that the
// first expression was already parsed, so it starts with a comma.
// It is used to parse tuples and list elements.
// expr_list ::= ( ',' expr )* ','?
private List<Expression> parseExprList() {
List<Expression> list = new ArrayList<>();
// terminating tokens for an expression list
while (token.kind != TokenKind.RPAREN && token.kind != TokenKind.RBRACKET) {
list.add(parseExpression());
if (token.kind == TokenKind.COMMA) {
nextToken();
} else {
while (token.kind == TokenKind.COMMA) {
expect(TokenKind.COMMA);
if (EXPR_LIST_TERMINATOR_SET.contains(token.kind)) {
break;
}
list.add(parseNonTupleExpression());
}
return list;
}
Expand All @@ -502,12 +508,12 @@ private List<DictionaryEntryLiteral> parseDictEntryList() {
return list;
}

// dict_entry ::= expression ':' expression
// dict_entry ::= nontupleexpr ':' nontupleexpr
private DictionaryEntryLiteral parseDictEntry() {
int start = token.left;
Expression key = parseExpression();
Expression key = parseNonTupleExpression();
expect(TokenKind.COLON);
Expression value = parseExpression();
Expression value = parseNonTupleExpression();
return setLocation(new DictionaryEntryLiteral(key, value), start, value);
}

Expand Down Expand Up @@ -571,7 +577,6 @@ private void include(String labelName, List<Statement> list, Location location)
// | list_expression
// | '(' ')' // a tuple with zero elements
// | '(' expr ')' // a parenthesized expression
// | '(' expr ',' expr_list ')' // a tuple with n elements
// | dict_expression
// | '-' primary_with_suffix
private Expression parsePrimary() {
Expand Down Expand Up @@ -605,7 +610,7 @@ private Expression parsePrimary() {
}
}
case LBRACKET: { // it's a list
return parseListExpression();
return parseListMaker();
}
case LBRACE: { // it's a dictionary
return parseDictExpression();
Expand All @@ -622,16 +627,6 @@ private Expression parsePrimary() {
}
// parse the first expression
Expression expression = parseExpression();
if (token.kind == TokenKind.COMMA) { // it's a tuple
nextToken();
// parse the rest of the expression tuple
List<Expression> tuple = parseExprList();
// add the first expression to the front of the tuple
tuple.add(0, expression);
expect(TokenKind.RPAREN);
return setLocation(
ListLiteral.makeTuple(tuple), start, token.right);
}
setLocation(expression, start, token.right);
if (token.kind == TokenKind.RPAREN) {
nextToken();
Expand Down Expand Up @@ -686,7 +681,7 @@ private Expression parseSubstringSuffix(int start, Expression receiver) {
if (token.kind == TokenKind.COLON) {
startExpr = setLocation(new IntegerLiteral(0), token.left, token.right);
} else {
startExpr = parseExpression();
startExpr = parseNonTupleExpression();
}
args.add(setLocation(new Argument.Positional(startExpr), loc1, startExpr));
// This is a dictionary access
Expand All @@ -701,7 +696,7 @@ private Expression parseSubstringSuffix(int start, Expression receiver) {
if (token.kind == TokenKind.RBRACKET) {
endExpr = setLocation(new IntegerLiteral(Integer.MAX_VALUE), token.left, token.right);
} else {
endExpr = parseExpression();
endExpr = parseNonTupleExpression();
}
expect(TokenKind.RBRACKET);

Expand Down Expand Up @@ -743,11 +738,11 @@ private Ident parseForLoopVariables() {
return multipleVariables ? makeErrorExpression(start, end) : firstIdent;
}

// list_expression ::= '[' ']'
// |'[' expr ']'
// |'[' expr ',' expr_list ']'
// |'[' expr ('FOR' loop_variables 'IN' expr)+ ']'
private Expression parseListExpression() {
// list_maker ::= '[' ']'
// |'[' expr ']'
// |'[' expr expr_list ']'
// |'[' expr ('FOR' loop_variables 'IN' expr)+ ']'
private Expression parseListMaker() {
int start = token.left;
expect(TokenKind.LBRACKET);
if (token.kind == TokenKind.RBRACKET) { // empty List
Expand All @@ -756,7 +751,7 @@ private Expression parseListExpression() {
nextToken();
return literal;
}
Expression expression = parseExpression();
Expression expression = parseNonTupleExpression();
Preconditions.checkNotNull(expression,
"null element in list in AST at %s:%s", token.left, token.right);
switch (token.kind) {
Expand Down Expand Up @@ -791,7 +786,6 @@ private Expression parseListExpression() {
return makeErrorExpression(start, end);
}
case COMMA: {
nextToken();
List<Expression> list = parseExprList();
Preconditions.checkState(!list.contains(null),
"null element in list in AST at %s:%s", token.left, token.right);
Expand Down Expand Up @@ -871,7 +865,7 @@ private Ident parseIdent() {
// the order), and it assumes left-to-right associativity.
private Expression parseBinOpExpression(int prec) {
int start = token.left;
Expression expr = parseExpression(prec + 1);
Expression expr = parseNonTupleExpression(prec + 1);
// The loop is not strictly needed, but it prevents risks of stack overflow. Depth is
// limited to number of different precedence levels (operatorPrecedence.size()).
for (;;) {
Expand All @@ -883,7 +877,7 @@ private Expression parseBinOpExpression(int prec) {
return expr;
}
nextToken();
Expression secondary = parseExpression(prec + 1);
Expression secondary = parseNonTupleExpression(prec + 1);
expr = optimizeBinOpExpression(operator, expr, secondary);
setLocation(expr, start, secondary);
}
Expand All @@ -906,15 +900,33 @@ private Expression optimizeBinOpExpression(
return new BinaryOperatorExpression(operator, expr, secondary);
}

// Equivalent to 'testlist' rule in Python grammar. It can parse every
// kind of expression.
// In many cases, we need to use parseNonTupleExpression to avoid ambiguity
// e.g. fct(x, y) vs fct((x, y))
private Expression parseExpression() {
int start = token.left;
Expression expr = parseExpression(0);
Expression expression = parseNonTupleExpression();
if (token.kind != TokenKind.COMMA) {
return expression;
}

// It's a tuple
List<Expression> tuple = parseExprList();
tuple.add(0, expression); // add the first expression to the front of the tuple
return setLocation(ListLiteral.makeTuple(tuple), start, token.right);
}

// Equivalent to 'test' rule in Python grammar.
private Expression parseNonTupleExpression() {
int start = token.left;
Expression expr = parseNonTupleExpression(0);
if (token.kind == TokenKind.IF) {
nextToken();
Expression condition = parseExpression(0);
Expression condition = parseNonTupleExpression(0);
if (token.kind == TokenKind.ELSE) {
nextToken();
Expression elseClause = parseExpression();
Expression elseClause = parseNonTupleExpression();
return setLocation(new ConditionalExpression(expr, condition, elseClause),
start, elseClause);
} else {
Expand All @@ -926,7 +938,7 @@ private Expression parseExpression() {
return expr;
}

private Expression parseExpression(int prec) {
private Expression parseNonTupleExpression(int prec) {
if (prec >= operatorPrecedence.size()) {
return parsePrimaryWithSuffix();
}
Expand All @@ -940,7 +952,7 @@ private Expression parseExpression(int prec) {
private Expression parseNotExpression(int prec) {
int start = token.left;
expect(TokenKind.NOT);
Expression expression = parseExpression(prec + 1);
Expression expression = parseNonTupleExpression(prec + 1);
NotExpression notExpression = new NotExpression(expression);
return setLocation(notExpression, start, token.right);
}
Expand Down Expand Up @@ -1121,7 +1133,7 @@ private IfStatement parseIfStatement() {
private ConditionalStatements parseConditionalStatements(TokenKind tokenKind) {
int start = token.left;
expect(tokenKind);
Expression expr = parseExpression();
Expression expr = parseNonTupleExpression();
expect(TokenKind.COLON);
List<Statement> thenBlock = parseSuite();
ConditionalStatements stmt = new ConditionalStatements(expr, thenBlock);
Expand Down
36 changes: 35 additions & 1 deletion src/test/java/com/google/devtools/build/lib/syntax/ParserTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -313,10 +313,22 @@ public void testAssignLocation() {
}

@Test
public void testAssign() {
public void testTupleAssign() {
String expr = "list[0] = 5; dict['key'] = value\n";
List<Statement> statements = parseFile(expr);
assertThat(statements).hasSize(2);
assertThat(statements.get(0)).isInstanceOf(AssignmentStatement.class);
assertThat(statements.get(1)).isInstanceOf(AssignmentStatement.class);
}

@Test
public void testAssign() {
String expr = "a, b = 5\n";
List<Statement> statements = parseFile(expr);
assertThat(statements).hasSize(1);
assertThat(statements.get(0)).isInstanceOf(AssignmentStatement.class);
AssignmentStatement assign = (AssignmentStatement) statements.get(0);
assertThat(assign.getLValue().getExpression()).isInstanceOf(ListLiteral.class);
}

@Test
Expand Down Expand Up @@ -412,6 +424,28 @@ public void testTupleLiterals2() throws Exception {
}
}

@Test
public void testTupleWithoutParens() throws Exception {
ListLiteral tuple = (ListLiteral) parseExpr("0, 1, 2");
assertTrue(tuple.isTuple());
assertThat(tuple.getElements()).hasSize(3);
assertTrue(tuple.isTuple());
for (int i = 0; i < 3; ++i) {
assertEquals(i, getIntElem(tuple, i));
}
}

@Test
public void testTupleWithoutParensWithTrailingComma() throws Exception {
ListLiteral tuple = (ListLiteral) parseExpr("0, 1, 2, 3,");
assertTrue(tuple.isTuple());
assertThat(tuple.getElements()).hasSize(4);
assertTrue(tuple.isTuple());
for (int i = 0; i < 4; ++i) {
assertEquals(i, getIntElem(tuple, i));
}
}

@Test
public void testTupleLiterals3() throws Exception {
ListLiteral emptyTuple = (ListLiteral) parseExpr("()");
Expand Down

0 comments on commit 5609389

Please sign in to comment.