Skip to content

Commit

Permalink
allow function definitions with operators that use comparison syntax
Browse files Browse the repository at this point in the history
  • Loading branch information
@JeffBezanson
JeffBezanson committed Mar 13, 2016
1 parent f576e87 commit f07485f
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 14 deletions.
37 changes: 23 additions & 14 deletions src/julia-syntax.scm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -960,7 +960,9 @@
(= ,vname ,(caddar binds))
,blk))))))
((and (pair? (cadar binds))
(eq? (caadar binds) 'call))
(or (eq? (caadar binds) 'call)
(and (eq? (caadar binds) 'comparison)
(length= (cadar binds) 4))))
;; f()=c
(let* ((asgn (butlast (expand-forms (car binds))))
(name (cadr (cadar binds)))
Expand Down Expand Up @@ -1526,13 +1528,20 @@

'=
(lambda (e)
(define lhs (cadr e))
(cond
((and (pair? (cadr e))
(eq? (car (cadr e)) 'call))
((and (pair? lhs)
(eq? (car lhs) 'call))
(expand-forms (cons 'function (cdr e))))
((and (pair? lhs)
(eq? (car lhs) 'comparison)
(length= lhs 4))
;; allow defining functions that use comparison syntax
(expand-forms (list* 'function
`(call ,(caddr lhs) ,(cadr lhs) ,(cadddr lhs)) (cddr e))))
((assignment? (caddr e))
;; chain of assignments - convert a=b=c to `b=c; a=c`
(let loop ((lhss (list (cadr e)))
(let loop ((lhss (list lhs))
(rhs (caddr e)))
(if (assignment? rhs)
(loop (cons (cadr rhs) lhss) (caddr rhs))
Expand All @@ -1542,12 +1551,12 @@
,@(map (lambda (l) `(= ,l ,rr))
lhss)
,rr))))))
((symbol-like? (cadr e))
((symbol-like? lhs)
`(= ,(cadr e) ,(expand-forms (caddr e))))
((atom? (cadr e))
(error (string "invalid assignment location \"" (deparse (cadr e)) "\"")))
((atom? lhs)
(error (string "invalid assignment location \"" (deparse lhs) "\"")))
(else
(case (car (cadr e))
(case (car lhs)
((|.|)
;; a.b =
(let ((a (cadr (cadr e)))
Expand All @@ -1568,7 +1577,7 @@
,rr))))
((tuple)
;; multiple assignment
(let ((lhss (cdr (cadr e)))
(let ((lhss (cdr lhs))
(x (caddr e)))
(if (and (pair? x) (pair? lhss) (eq? (car x) 'tuple)
(length= lhss (length (cdr x))))
Expand Down Expand Up @@ -1598,8 +1607,8 @@
(error "unexpected \";\" in left side of indexed assignment"))
((ref)
;; (= (ref a . idxs) rhs)
(let ((a (cadr (cadr e)))
(idxs (cddr (cadr e)))
(let ((a (cadr lhs))
(idxs (cddr lhs))
(rhs (caddr e)))
(let* ((reuse (and (pair? a)
(contains (lambda (x)
Expand All @@ -1623,8 +1632,8 @@
,r)))))
((|::|)
;; (= (|::| x T) rhs)
(let ((x (cadr (cadr e)))
(T (caddr (cadr e)))
(let ((x (cadr lhs))
(T (caddr lhs))
(rhs (caddr e)))
(let ((e (remove-argument-side-effects x)))
(expand-forms
Expand All @@ -1635,7 +1644,7 @@
;; (= (vcat . args) rhs)
(error "use \"(a, b) = ...\" to assign multiple values"))
(else
(error (string "invalid assignment location \"" (deparse (cadr e)) "\"")))))))
(error (string "invalid assignment location \"" (deparse lhs) "\"")))))))

'abstract
(lambda (e)
Expand Down
7 changes: 7 additions & 0 deletions test/parse.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -358,3 +358,10 @@ call2(f,x,y) = f(x,y)
@test (call0() do; 42 end) == 42
@test (call1(42) do x; x+1 end) == 43
@test (call2(42,1) do x,y; x+y+1 end) == 44

# definitions using comparison syntax
let a⊂b = reduce(&, x ∈ b for x in a) && length(b)>length(a)
@test [1,2] ⊂ [1,2,3,4]
@test !([1,2] ⊂ [1,3,4])
@test !([1,2] ⊂ [1,2])
end

6 comments on commit f07485f

@tkelman
Copy link
Contributor

Choose a reason for hiding this comment

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

New syntax without any discussion or review? We don't allow this for any other infix operators, why just comparisons? edit: oh, we do. When did that happen?

@Ismael-VC
Copy link
Contributor

Choose a reason for hiding this comment

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

Why sometimes things go into Julia without peer review as Tony mentions? When is it safe for the repo owners to commit directly? Everything being peer reviewed is the correct thing am I right?

Whenever I make a PR I'm always asked to change the docs and add tests for those changes. So in this case we would also need to add a change to the documentation acordingly in order to include this feature.

@tkelman
Copy link
Contributor

Choose a reason for hiding this comment

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

I had no idea infix function definition syntax was allowed, so it's certainly underdocumented (#15483)

@JeffBezanson
Copy link
Member Author

Choose a reason for hiding this comment

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

This is not new syntax. We have always supported definitions with infix syntax, and missing the case of comparison operators was a bug due to their slightly different parsing. Such a trivial matter. Honestly.

@tkelman
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think I've ever seen docs, examples, tests, or code in packages that used infix definition syntax. That's why this surprised me. If the only evidence of it working is in julia-syntax.scm, then it's pretty much a hidden feature.

@StefanKarpinski
Copy link
Member

Choose a reason for hiding this comment

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

The definition and usage have always been parsed the same, but it is definitely an undocumented feature.

Please sign in to comment.