Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RFC: fix #26137, change parsing of unary ops on parenthesized expressions #26154

Merged
merged 3 commits into from
Feb 27, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
improve rule for distinguishing tuples (arg lists) from parenthesized…
… blocks

a parenthesized expression is a tuple when at least one of these is true:

- it is empty
- there is a comma
- it begins with `x...` and isn't just `(x...)`
- it begins with `(;` (corresponding to the positional part being empty)

add line numbers to blocks written as `(a; b; c)`
  • Loading branch information
JeffBezanson committed Feb 23, 2018
commit 952361f340066d4a02a34065853bf6c120c18539
1 change: 1 addition & 0 deletions src/ast.scm
Original file line number Diff line number Diff line change
Expand Up @@ -406,6 +406,7 @@
(eq? (cadr (caddr x)) 'Vararg)))))
(define (trans? x) (and (pair? x) (eq? (car x) '|.'|)))
(define (ctrans? x) (and (pair? x) (eq? (car x) '|'|)))
(define (linenum? x) (and (pair? x) (eq? (car x) 'line)))

(define (make-assignment l r) `(= ,l ,r))
(define (assignment? e) (and (pair? e) (eq? (car e) '=)))
Expand Down
97 changes: 62 additions & 35 deletions src/julia-parser.scm
Original file line number Diff line number Diff line change
Expand Up @@ -675,8 +675,7 @@
(memv (peek-token s) ops))
(loop ex #f (peek-token s))
(if (and add-linenums
(not (and (pair? (car ex))
(eq? (caar ex) 'line))))
(not (linenum? (car ex))))
(let ((loc (line-number-node s)))
(loop (list* (down s) loc ex) #f (peek-token s)))
(loop (cons (down s) ex) #f (peek-token s))))))))))
Expand Down Expand Up @@ -1350,7 +1349,7 @@
(blk (parse-block s (lambda (s) (parse-docstring s parse-eq)))))
(expect-end s word)
(let ((blk (if (and (length> blk 1)
(pair? (cadr blk)) (eq? (caadr blk) 'line))
(linenum? (cadr blk)))
(list* 'block loc (cddr blk))
blk)))
(if (eq? word 'quote)
Expand All @@ -1374,7 +1373,7 @@
(error "let variables should end in \";\" or newline"))
(let* ((ex (begin0 (parse-block s)
(expect-end s word)))
(ex (if (and (length= ex 2) (pair? (cadr ex)) (eq? (caadr ex) 'line))
(ex (if (and (length= ex 2) (linenum? (cadr ex)))
`(block) ;; don't need line info in an empty let block
ex)))
`(let ,(if (and (length= binds 1) (or (assignment? (car binds)) (decl? (car binds))
Expand Down Expand Up @@ -1527,8 +1526,7 @@
catch-block
`(block ,loc ,var
,@(if (and (length= catch-block 2)
(pair? (cadr catch-block))
(eq? (caadr catch-block) 'line))
(linenum? (cadr catch-block)))
'()
(cdr catch-block))))
(if var? var 'false)
Expand Down Expand Up @@ -1725,7 +1723,7 @@
;; . an extra comma at the end is allowed
;; . expressions after a ; are enclosed in (parameters ...)
;; . an expression followed by ... becomes (... x)
(define (parse-arglist s closer)
(define (parse-arglist s closer (add-linenums #f))
(with-bindings ((range-colon-enabled #t)
(space-sensitive #f)
(where-enabled #t)
Expand All @@ -1739,12 +1737,17 @@
(to-kws (reverse! lst))
(reverse! lst)))
(if (eqv? t #\;)
(begin (take-token s)
(let ((params (loop '()))
(begin (take-token s) (require-token s)
(let ((loc (line-number-node s))
(params (loop '()))
(lst (if (eqv? closer #\) )
(to-kws (reverse lst))
(reverse lst))))
(cons (cons 'parameters params)
(cons `(parameters
,@(if add-linenums
(list loc)
'())
,@params)
lst)))
(let* ((nxt (parse-eq* s))
(c (require-token s)))
Expand Down Expand Up @@ -1910,20 +1913,36 @@
;; this allows us to first parse tuples using parse-arglist
(define (parameters-to-block e)
(if (and (pair? e) (eq? (car e) 'parameters))
(cond ((length= e 1) '())
((length= e 2) (parameters-to-block (cadr e)))
((length= e 3)
(let ((fst (cadr e))
(snd (caddr e)))
(if (and (pair? fst) (eq? (car fst) 'parameters))
(let ((rec (parameters-to-block fst))
(snd (parameters-to-block snd)))
(and rec snd
(cons (car snd) rec)))
#f)))
(else #f))
(let ((e2 (filter (lambda (x) (not (linenum? x))) e))
(lnum (if (and (pair? (cdr e))
(linenum? (cadr e)))
(cadr e)
#f)))
(cond ((length= e2 1) '())
((length= e2 2)
(let ((rec (parameters-to-block (cadr e2))))
(if (null? rec)
rec
(cons lnum rec))))
((length= e2 3)
(let ((fst (cadr e2))
(snd (caddr e2)))
(if (and (pair? fst) (eq? (car fst) 'parameters))
(let ((rec (parameters-to-block fst))
(snd (parameters-to-block snd)))
(and rec snd
(append (if lnum (list lnum) '()) (cons (car snd) rec))))
#f)))
(else #f)))
(list (kw-to-= e))))

(define (rm-linenums e)
(if (atom? e) e
(map rm-linenums
(if (eq? (car e) 'parameters)
(filter (lambda (x) (not (linenum? x))) e)
e))))

;; convert an arglist to a tuple or block expr
;; leading-semi? means we saw (; ...)
;; comma? means there was a comma after the first expression
Expand All @@ -1938,9 +1957,10 @@
`(block)))))) ;; all semicolons inside ()
(and (null? first) (null? args) (not comma?)
`(block)) ;; this case is (;)
(if (and (pair? args) (pair? (car args)) (eq? (caar args) 'parameters))
`(tuple ,(car args) ,@first ,@(map kw-to-= (cdr args)))
`(tuple ,@first ,@(map kw-to-= args))))))
(rm-linenums
(if (and (pair? args) (pair? (car args)) (eq? (caar args) 'parameters))
`(tuple ,(car args) ,@first ,@(map kw-to-= (cdr args)))
`(tuple ,@first ,@(map kw-to-= args)))))))

(define (tuple-to-arglist e)
(cond ((eq? (car e) 'tuple) (map =-to-kw (cdr e)))
Expand Down Expand Up @@ -1986,8 +2006,8 @@
(take-token s) ;; take #\)
'(|::| . #f))
((eqv? nxt #\;)
(cons (arglist-to-tuple #t #f (parse-arglist s #\) ))
#t))
(let ((ex (arglist-to-tuple #t #f (parse-arglist s #\) ))))
(cons ex (eq? (car ex) 'tuple))))
(else
;; here we parse the first subexpression separately, so
;; we can look for a comma to see if it's a tuple.
Expand All @@ -1997,7 +2017,7 @@
(cond ((eqv? t #\) )
(take-token s)
;; value in parentheses (x)
(if (and (pair? ex) (eq? (car ex) '...))
(if (vararg? ex)
(let ((lineno (input-port-line (ts:port s))))
(if (or accept-dots-without-comma (eq? (with-bindings ((whitespace-newline #f))
(peek-token s))
Expand All @@ -2008,6 +2028,19 @@
(string "(" (deparse (cadr ex)) "...,)"))
(cons `(tuple ,ex) #t))))
(cons ex #f)))
((eqv? t #\,)
;; tuple (x,) (x,y) etc.
(take-token s)
(cons (arglist-to-tuple #f #t (parse-arglist s #\) ) ex)
#t))
((eqv? t #\;)
(cons (arglist-to-tuple
#f
;; consider `(x...; ` the start of an arglist, since it's not useful as a block
(vararg? ex)
(parse-arglist s #\) #t)
ex)
(vararg? ex)))
((eq? t 'for)
(expect-space-before s 'for)
(take-token s)
Expand All @@ -2017,13 +2050,7 @@
(error "expected \")\""))
(cons gen #f)))
(else
;; tuple (x,) (x,y) (x...) etc.
(if (eqv? t #\, )
(take-token s)
(if (not (eqv? t #\;))
(error "missing comma or ) in argument list")))
(cons (arglist-to-tuple #f (eqv? t #\,) (parse-arglist s #\) ) ex)
#t)))))))))
(error "missing comma or ) in argument list")))))))))

(define (not-eof-for delim c)
(if (eof-object? c)
Expand Down
16 changes: 7 additions & 9 deletions src/julia-syntax.scm
Original file line number Diff line number Diff line change
Expand Up @@ -766,7 +766,7 @@
(if (and (pair? body) (eq? (car body) 'block))
(cond ((atom? (cdr body))
`(block ,stmt (null)))
((and (pair? (cadr body)) (eq? (caadr body) 'line))
((linenum? (cadr body))
`(block ,(cadr body) ,stmt ,@(cddr body)))
(else
`(block ,stmt ,@(cdr body))))
Expand Down Expand Up @@ -805,8 +805,7 @@
,(ctor-body body curlyargs))))))

(define (function-body-lineno body)
(let ((lnos (filter (lambda (e) (and (pair? e) (eq? (car e) 'line)))
body)))
(let ((lnos (filter linenum? body)))
(if (null? lnos) '() (car lnos))))

;; rewrite calls to `new( ... )` to `new` expressions on the appropriate
Expand Down Expand Up @@ -863,7 +862,7 @@
(fields defs) (separate (lambda (x) (or (symbol? x) (decl? x)))
fields0)
(let* ((defs (filter (lambda (x) (not (effect-free? x))) defs))
(locs (if (and (pair? fields0) (pair? (car fields0)) (eq? (caar fields0) 'line))
(locs (if (and (pair? fields0) (linenum? (car fields0)))
(list (car fields0))
'()))
(field-names (map decl-var fields))
Expand Down Expand Up @@ -1103,7 +1102,7 @@
(set! a (cadr w))))
#f))
(argl (if (pair? a)
(tuple-to-arglist a)
(tuple-to-arglist (filter (lambda (x) (not (linenum? x))) a))
(list a)))
;; TODO: always use a specific special name like #anon# or _, then ignore
;; this as a local variable name.
Expand Down Expand Up @@ -1238,7 +1237,7 @@
(if (null? f)
'()
(let ((x (car f)))
(cond ((or (symbol? x) (decl? x) (and (pair? x) (eq? (car x) 'line)))
(cond ((or (symbol? x) (decl? x) (linenum? x))
(loop (cdr f)))
((and (assignment? x) (or (symbol? (cadr x)) (decl? (cadr x))))
(error (string "\"" (deparse x) "\" inside type definition is reserved")))
Expand Down Expand Up @@ -1893,8 +1892,7 @@
(lambda (e)
(cond ((null? (cdr e)) '(null))
((and (null? (cddr e))
(not (and (pair? (cadr e))
(eq? (car (cadr e)) 'line))))
(not (linenum? (cadr e))))
(expand-forms (cadr e)))
(else
(cons 'block
Expand Down Expand Up @@ -3608,7 +3606,7 @@ f(x) = yt(x)
((block body)
(let* ((last-fname filename)
(fnm (first-non-meta e))
(fname (if (and (length> e 1) (pair? fnm) (eq? (car fnm) 'line)
(fname (if (and (length> e 1) (linenum? fnm)
(length> fnm 2))
(caddr fnm)
filename))
Expand Down
37 changes: 28 additions & 9 deletions test/syntax.jl
Original file line number Diff line number Diff line change
Expand Up @@ -176,13 +176,21 @@ macro test999_str(args...); args; end
@test_throws ParseError Meta.parse("(,)")
@test_throws ParseError Meta.parse("(;,)")
@test_throws ParseError Meta.parse("(,;)")
# TODO: would be nice to make these errors, but needed to parse e.g. `(x;y,)->x`
#@test_throws ParseError Meta.parse("(1;2,)")
#@test_throws ParseError Meta.parse("(1;2,;)")
#@test_throws ParseError Meta.parse("(1;2,;3)")
@test Meta.parse("(x;)") == Expr(:block, :x)
@test Meta.parse("(;x)") == Expr(:tuple, Expr(:parameters, :x))
@test Meta.parse("(;x,)") == Expr(:tuple, Expr(:parameters, :x))
@test Meta.parse("(x,)") == Expr(:tuple, :x)
@test Meta.parse("(x,;)") == Expr(:tuple, Expr(:parameters), :x)
@test Meta.parse("(x;y)") == Expr(:block, :x, :y)
@test Meta.parse("(x=1;y=2)") == Expr(:block, Expr(:(=), :x, 1), Expr(:(=), :y, 2))
@test Meta.parse("(x;y)") == Expr(:block, :x, LineNumberNode(1,:none), :y)
@test Meta.parse("(x...;)") == Expr(:tuple, Expr(:parameters), Expr(:(...), :x))
@test Meta.parse("(;x...)") == Expr(:tuple, Expr(:parameters, Expr(:(...), :x)))
@test Meta.parse("(x...;y)") == Expr(:tuple, Expr(:parameters, :y), Expr(:(...), :x))
Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this require a , like (1...) now does?

Copy link
Sponsor Member Author

@JeffBezanson JeffBezanson Feb 22, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It parses, but lowering gives unexpected semicolon in tuple in this case.

@test Meta.parse("(x;y...)") == Expr(:block, :x, LineNumberNode(1,:none), Expr(:(...), :y))
@test Meta.parse("(x=1;y=2)") == Expr(:block, Expr(:(=), :x, 1), LineNumberNode(1,:none), Expr(:(=), :y, 2))
@test Meta.parse("(x,;y)") == Expr(:tuple, Expr(:parameters, :y), :x)
@test Meta.parse("(x,;y=1)") == Expr(:tuple, Expr(:parameters, Expr(:kw, :y, 1)), :x)
@test Meta.parse("(x,a;y=1)") == Expr(:tuple, Expr(:parameters, Expr(:kw, :y, 1)), :x, :a)
Expand Down Expand Up @@ -1310,13 +1318,24 @@ end
# f(x) = x", 1)[1] === "x"

# issue #26137
@test Meta.parse("-()^2") == Expr(:call, :^, Expr(:call, :-), 2)
@test Meta.parse("-(x)^2") == Expr(:call, :-, Expr(:call, :^, :x, 2))
@test Meta.parse("-(x,)^2") == Expr(:call, :^, Expr(:call, :-, :x), 2)
@test Meta.parse("-(x,y)^2") == Expr(:call, :^, Expr(:call, :-, :x, :y), 2)
@test Meta.parse("+((1,2))") == Expr(:call, :+, Expr(:tuple, 1, 2))
@test Meta.parse("-(x;y)^2") == Expr(:call, :^, Expr(:call, :-, Expr(:parameters, :y), :x), 2)
@test Meta.parse("-(x...)^2") == Expr(:call, :^, Expr(:call, :-, Expr(:(...), :x)), 2)
# cases where parens enclose argument lists
@test Meta.parse("-()^2") == Expr(:call, :^, Expr(:call, :-), 2)
@test Meta.parse("-(x,)^2") == Expr(:call, :^, Expr(:call, :-, :x), 2)
@test Meta.parse("-(x,;)^2") == Expr(:call, :^, Expr(:call, :-, Expr(:parameters), :x), 2)
@test Meta.parse("-(;x)^2") == Expr(:call, :^, Expr(:call, :-, Expr(:parameters, :x)), 2)
@test Meta.parse("-(x,y)^2") == Expr(:call, :^, Expr(:call, :-, :x, :y), 2)
@test Meta.parse("-(x...)^2") == Expr(:call, :^, Expr(:call, :-, Expr(:(...), :x)), 2)
@test Meta.parse("-(x...;)^2") == Expr(:call, :^, Expr(:call, :-, Expr(:parameters), Expr(:(...), :x)), 2)
@test Meta.parse("-(x...;)") == Expr(:call, :-, Expr(:parameters), Expr(:(...), :x))

# cases where parens are just grouping
@test Meta.parse("-(x)^2") == Expr(:call, :-, Expr(:call, :^, :x, 2))
@test Meta.parse("-(a=1)^2") == Expr(:call, :-, Expr(:call, :^, Expr(:(=), :a, 1), 2))
@test Meta.parse("-(x;y)^2") == Expr(:call, :-, Expr(:call, :^, Expr(:block, :x, LineNumberNode(1,:none), :y), 2))
@test Meta.parse("-(;)^2") == Expr(:call, :-, Expr(:call, :^, Expr(:block), 2))
@test Meta.parse("-(;;;;)^2") == Expr(:call, :-, Expr(:call, :^, Expr(:block), 2))
@test Meta.parse("-(x;;;)^2") == Expr(:call, :-, Expr(:call, :^, Expr(:block, :x), 2))
@test Meta.parse("+((1,2))") == Expr(:call, :+, Expr(:tuple, 1, 2))

@test_throws ParseError("space before \"(\" not allowed in \"+ (\"") Meta.parse("1 -+ (a=1, b=2)")

Expand Down