Skip to content

Commit

Permalink
disallow return inside generators and comprehensions (#27172)
Browse files Browse the repository at this point in the history
  • Loading branch information
JeffBezanson authored May 20, 2018
1 parent d247ca8 commit 9a0ff52
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 9 deletions.
10 changes: 8 additions & 2 deletions src/julia-syntax.scm
Original file line number Diff line number Diff line change
Expand Up @@ -2293,7 +2293,9 @@
(return (expand-forms `(call transpose ,(cadr e))))))

'generator
(lambda (e) (expand-generator e #f '()))
(lambda (e)
(check-no-return e)
(expand-generator e #f '()))

'flatten
(lambda (e) (expand-generator (cadr e) #t '()))
Expand All @@ -2317,7 +2319,6 @@
(error "comprehension syntax with `:` ranges has been removed"))
(and (every (lambda (x) (and (pair? x) (eq? (car x) '=)))
ranges)
(not (has-return? (cadr (caddr e))))
;; TODO: this is a hack to lower simple comprehensions to loops very
;; early, to greatly reduce the # of functions and load on the compiler
(lower-comprehension (cadr e) (cadr (caddr e)) ranges))))
Expand All @@ -2326,13 +2327,18 @@
(define (has-return? e)
(expr-contains-p return? e (lambda (x) (not (function-def? x)))))

(define (check-no-return e)
(if (has-return? e)
(error "\"return\" not allowed inside comprehension or generator")))

(define (has-break-or-continue? e)
(expr-contains-p (lambda (x) (and (pair? x) (memq (car x) '(break continue))))
e
(lambda (x) (not (and (pair? x)
(memq (car x) '(for while)))))))

(define (lower-comprehension ty expr itrs)
(check-no-return expr)
(if (has-break-or-continue? expr)
(error "break or continue outside loop"))
(let ((result (gensy))
Expand Down
7 changes: 0 additions & 7 deletions test/functional.jl
Original file line number Diff line number Diff line change
Expand Up @@ -220,10 +220,3 @@ end
let (:)(a,b) = (i for i in Base.:(:)(1,10) if i%2==0)
@test Int8[ i for i = 1:2 ] == [2,4,6,8,10]
end

# comprehensions with `return`
let f() = [ return 2 for i=1:2 ],
g() = Int[return 2 for i=1:2 ]
@test f() == [2, 2]
@test g() == [2, 2]
end
6 changes: 6 additions & 0 deletions test/syntax.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1438,3 +1438,9 @@ end
@test Meta.lower(@__MODULE__, :(Int[if true; break; end for i = 1:1])) == Expr(:error, "break or continue outside loop")
@test Meta.lower(@__MODULE__, :([if true; continue; end for i = 1:1])) == Expr(:error, "break or continue outside loop")
@test Meta.lower(@__MODULE__, :(Int[if true; continue; end for i = 1:1])) == Expr(:error, "break or continue outside loop")

@test Meta.lower(@__MODULE__, :(return 0 for i=1:2)) == Expr(:error, "\"return\" not allowed inside comprehension or generator")
@test Meta.lower(@__MODULE__, :([ return 0 for i=1:2 ])) == Expr(:error, "\"return\" not allowed inside comprehension or generator")
@test Meta.lower(@__MODULE__, :(Int[ return 0 for i=1:2 ])) == Expr(:error, "\"return\" not allowed inside comprehension or generator")
@test [ ()->return 42 for i = 1:1 ][1]() == 42
@test Function[ identity() do x; return 2x; end for i = 1:1 ][1](21) == 42

2 comments on commit 9a0ff52

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

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

Executing the daily benchmark build, I will reply here when finished:

@nanosoldier runbenchmarks(ALL, isdaily = true)

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

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

Your benchmark job has completed - possible performance regressions were detected. A full report can be found here. cc @ararslan

Please sign in to comment.