parser: fix precedence of nested 'if' clauses in list comprehensions (g…

…oogle#55)

* parser: fix precedence of nested 'if' clauses in list comprehensions

Before, the parser would consume an arbitrary expression after the
'if' even though it should reduce after x in
  [a for b in c if x if y else z]
Now it consumes only a precedence zero ("or"-level) expression.

The parser similarly reduces after x when parsing
  [a for b in c if lambda: x if y else z]

* fix three comment typos

doc/spec.md

@@ -322,7 +322,6 @@ decimals  = decimal_digit {decimal_digit} .
 exponent  = ('e'|'E') ['+'|'-'] decimals .
 
 decimal_digit = '0' … '9' .
-
 octal_digit   = '0' … '7' .
 hex_digit     = '0' … '9' | 'A' … 'F' | 'a' … 'f' .
 binary_digit  = '0' | '1' .
@@ -2043,10 +2042,11 @@ assignment:
 ```
 
 Skylark, following Python 3, does not accept an unparenthesized
-tuple as the operand of a `for` clause:
+tuple or lambda expression as the operand of a `for` clause:
 
 ```python
 [x*x for x in 1, 2, 3]		# parse error: unexpected comma
+[x*x for x in lambda: 0]	# parse error: unexpected lambda
 ```
 
 Comprehensions in Skylark, again following Python 3, define a new lexical
@@ -2866,7 +2866,7 @@ truncating towards zero; it is an error if x is not finite (`NaN`,
 If x is a `bool`, the result is 0 for `False` or 1 for `True`.
 
 If x is a string, it is interpreted like a string literal;
-an optional base prefix (`0`, `0x`, `0X`) determines which base to use.
+an optional base prefix (`0`, `0b`, `0B`, `0x`, `0X`) determines which base to use.
 The string may specify an arbitrarily large integer,
 whereas true integer literals are restricted to 64 bits.
 If a non-zero `base` argument is provided, the string is interpreted

syntax/parse.go

@@ -466,21 +466,7 @@ func (p *parser) parseExprs(exprs []Expr, allowTrailingComma bool) []Expr {
 // parseTest parses a 'test', a single-component expression.
 func (p *parser) parseTest() Expr {
 	if p.tok == LAMBDA {
-		lambda := p.nextToken()
-		var params []Expr
-		if p.tok != COLON {
-			params = p.parseParams()
-		}
-		p.consume(COLON)
-		body := p.parseTest()
-		return &LambdaExpr{
-			Lambda: lambda,
-			Function: Function{
-				StartPos: lambda,
-				Params:   params,
-				Body:     []Stmt{&ReturnStmt{Result: body}},
-			},
-		}
+		return p.parseLambda(true)
 	}
 
 	x := p.parseTestPrec(0)
@@ -500,6 +486,42 @@ func (p *parser) parseTest() Expr {
 	return x
 }
 
+// parseTestNoCond parses a a single-component expression without
+// consuming a trailing 'if expr else expr'.
+func (p *parser) parseTestNoCond() Expr {
+	if p.tok == LAMBDA {
+		return p.parseLambda(false)
+	}
+	return p.parseTestPrec(0)
+}
+
+// parseLambda parses a lambda expression.
+// The allowCond flag allows the body to be an 'a if b else c' conditional.
+func (p *parser) parseLambda(allowCond bool) Expr {
+	lambda := p.nextToken()
+	var params []Expr
+	if p.tok != COLON {
+		params = p.parseParams()
+	}
+	p.consume(COLON)
+
+	var body Expr
+	if allowCond {
+		body = p.parseTest()
+	} else {
+		body = p.parseTestNoCond()
+	}
+
+	return &LambdaExpr{
+		Lambda: lambda,
+		Function: Function{
+			StartPos: lambda,
+			Params:   params,
+			Body:     []Stmt{&ReturnStmt{Result: body}},
+		},
+	}
+}
+
 func (p *parser) parseTestPrec(prec int) Expr {
 	if prec >= len(preclevels) {
 		return p.parsePrimaryWithSuffix()
@@ -888,7 +910,7 @@ func (p *parser) parseComprehensionSuffix(lbrace Position, body Expr, endBrace T
 			clauses = append(clauses, &ForClause{For: pos, Vars: vars, In: in, X: x})
 		} else if p.tok == IF {
 			pos := p.nextToken()
-			cond := p.parseTest()
+			cond := p.parseTestNoCond()
 			clauses = append(clauses, &IfClause{If: pos, Cond: cond})
 		} else {
 			p.in.errorf(p.in.pos, "got %#v, want '%s', for, or if", p.tok, endBrace)

syntax/parse_test.go

@@ -104,6 +104,8 @@ func TestExprParseTrees(t *testing.T) {
 			`(CondExpr Cond=b True=a False=c)`},
 		{`a and not b`,
 			`(BinaryExpr X=a Op=and Y=(UnaryExpr Op=not X=b))`},
+		{`[e for x in y if cond1 if cond2]`,
+			`(Comprehension Body=e Clauses=((ForClause Vars=x X=y) (IfClause Cond=cond1) (IfClause Cond=cond2)))`}, // github.com/google/skylark issue 53
 	} {
 		e, err := syntax.ParseExpr("foo.sky", test.input)
 		if err != nil {

syntax/testdata/errors.sky

@@ -85,8 +85,20 @@ for x in 1, 2, 3:
 _ = [x for x in 1, 2, 3] ### `got ',', want ']', for, or if`
 ---
 # Unparenthesized tuple is not allowed as operand of 'if' in comprehension.
-
 _ = [a for b in c if 1, 2] ### `got ',', want ']', for, or if`
+
+---
+# Lambda is ok though.
+_ = [a for b in c if lambda: d] # ok
+
+# But the body of such a lambda may not be a conditional:
+_ = [a for b in c if (lambda: d if e else f)] # ok
+_ = [a for b in c if lambda: d if e else f]   ### "got else, want ']'"
+
+---
+# A lambda is not allowed as the operand of a 'for' clause.
+_ = [a for b in lambda: c] ### `got lambda, want primary`
+
 ---
 # Comparison operations are not associative.
 

testdata/list.sky

@@ -61,6 +61,10 @@ assert.eq([2 * x for x in (1, 2, 3)], [2, 4, 6])
 assert.eq([x for x in "abc".split_bytes()], ["a", "b", "c"])
 assert.eq([x for x in {"a": 1, "b": 2}], ["a", "b"])
 assert.eq([(y, x) for x, y in {1: 2, 3: 4}.items()], [(2, 1), (4, 3)])
+# corner cases of parsing:
+assert.eq([x for x in range(12) if x%2 == 0 if x%3 == 0], [0, 6])
+assert.eq([x for x in [1, 2] if lambda: None], [1, 2])
+assert.eq([x for x in [1, 2] if (lambda: 3 if True else 4)], [1, 2])
 
 # list function
 assert.eq(list(), [])