From 39ab43a3ab9171a8ed1e66d665bde60ff46c5993 Mon Sep 17 00:00:00 2001 From: Jeff Bezanson Date: Thu, 15 Mar 2018 15:35:43 -0400 Subject: [PATCH] fresh variables on each iteration in nested loop and generator syntax fixes #330 --- NEWS.md | 4 + doc/src/manual/control-flow.md | 20 ++++- src/julia-syntax.scm | 144 +++++++++++++++++++-------------- test/functional.jl | 24 ++++++ 4 files changed, 130 insertions(+), 62 deletions(-) diff --git a/NEWS.md b/NEWS.md index 684a5b28cef24..a57fcfbcdf3be 100644 --- a/NEWS.md +++ b/NEWS.md @@ -151,6 +151,10 @@ Language changes * In `for i in x`, `x` used to be evaluated in a new scope enclosing the `for` loop. Now it is evaluated in the scope outside the `for` loop. + * In `for i in x, j in y`, all variables now have fresh bindings on each iteration of the + innermost loop. For example, an assignment to `i` will not be visible on the next `j` + loop iteration ([#330]). + * Variable bindings local to `while` loop bodies are now freshly allocated on each loop iteration, matching the behavior of `for` loops. diff --git a/doc/src/manual/control-flow.md b/doc/src/manual/control-flow.md index 458789a8650a1..ac7e213952f06 100644 --- a/doc/src/manual/control-flow.md +++ b/doc/src/manual/control-flow.md @@ -527,7 +527,25 @@ julia> for i = 1:2, j = 3:4 (2, 4) ``` -A `break` statement inside such a loop exits the entire nest of loops, not just the inner one. +With this syntax, iterables may still refer to outer loop variables; e.g. `for i = 1:n, j = 1:i` +is valid. +However a `break` statement inside such a loop exits the entire nest of loops, not just the inner one. +Both variables (`i` and `j`) are set to their current iteration values each time the inner loop runs. +Therefore, assignments to `i` will not be visible to subsequent iterations: + +```jldoctest +julia> for i = 1:2, j = 3:4 + println((i, j)) + i = 0 + end +(1, 3) +(1, 4) +(2, 3) +(2, 4) +``` + +If this example were rewritten to use a `for` keyword for each variable, then the output would +be different: the second and fourth values would contain `0`. ## Exception Handling diff --git a/src/julia-syntax.scm b/src/julia-syntax.scm index bb9c590ee5df5..71dd94f0f9832 100644 --- a/src/julia-syntax.scm +++ b/src/julia-syntax.scm @@ -1577,42 +1577,63 @@ ;; If false, this will try to warn only for uses of the last value after the loop. (define *warn-all-loop-vars* #f) -(define (expand-for while lhs X body) - ;; (for (= lhs X) body) - (let* ((coll (make-ssavalue)) - (state (gensy)) - (outer? (and (pair? lhs) (eq? (car lhs) 'outer))) - (lhs (if outer? (cadr lhs) lhs))) - `(block (= ,coll ,(expand-forms X)) - (= ,state (call (top start) ,coll)) - ;; TODO avoid `local declared twice` error from this - ;;,@(if outer? `((local ,lhs)) '()) - ,@(if outer? `((require-existing-local ,lhs)) '()) - ,(expand-forms - `(,while - (call (top not_int) (call (core typeassert) (call (top done) ,coll ,state) (core Bool))) - (block - ;; NOTE: enable this to force loop-local var - #;,@(map (lambda (v) `(local ,v)) (lhs-vars lhs)) - ,@(if (not outer?) - (map (lambda (v) `(warn-if-existing ,v)) (lhs-vars lhs)) - '()) - ,(lower-tuple-assignment (list lhs state) - `(call (top next) ,coll ,state)) - ,body)))))) +(define (expand-for lhss itrs body) + (define (outer? x) (and (pair? x) (eq? (car x) 'outer))) + (let ((copied-vars ;; variables not declared `outer` are copied in the innermost loop + ;; TODO: maybe filter these to remove vars not assigned in the loop + (delete-duplicates + (apply append + (map lhs-vars + (filter (lambda (x) (not (outer? x))) (butlast lhss))))))) + `(break-block + loop-exit + ,(let nest ((lhss lhss) + (itrs itrs)) + (if (null? lhss) + body + (let* ((coll (make-ssavalue)) + (state (gensy)) + (outer (outer? (car lhss))) + (lhs (if outer (cadar lhss) (car lhss))) + (body + `(block + ;; NOTE: enable this to force loop-local var + #;,@(map (lambda (v) `(local ,v)) (lhs-vars lhs)) + ,@(if (not outer) + (map (lambda (v) `(warn-if-existing ,v)) (lhs-vars lhs)) + '()) + ,(lower-tuple-assignment (list lhs state) + `(call (top next) ,coll ,state)) + ,(nest (cdr lhss) (cdr itrs)))) + (body + (if (null? (cdr lhss)) + `(break-block + loop-cont + (let (block ,@(map (lambda (v) `(= ,v ,v)) copied-vars)) + ,body)) + `(scope-block ,body)))) + `(block (= ,coll ,(car itrs)) + (= ,state (call (top start) ,coll)) + ;; TODO avoid `local declared twice` error from this + ;;,@(if outer `((local ,lhs)) '()) + ,@(if outer `((require-existing-local ,lhs)) '()) + (_while + (call (top not_int) (call (core typeassert) (call (top done) ,coll ,state) (core Bool))) + ,body)))))))) ;; wrap `expr` in a function appropriate for consuming values from given ranges -(define (func-for-generator-ranges expr range-exprs) +(define (func-for-generator-ranges expr range-exprs flat outervars) (let* ((vars (map cadr range-exprs)) (argname (if (and (length= vars 1) (symbol? (car vars))) (car vars) (gensy))) + (myvars (lhs-vars `(tuple ,@vars))) (splat (cond ((eq? argname (car vars)) '()) ((length= vars 1) - `(,@(map (lambda (v) `(local ,v)) (lhs-vars (car vars))) + `(,@(map (lambda (v) `(local ,v)) myvars) (= ,(car vars) ,argname))) (else - `(,@(map (lambda (v) `(local ,v)) (lhs-vars `(tuple ,@vars))) + `(,@(map (lambda (v) `(local ,v)) myvars) (= (tuple ,@vars) ,argname)))))) (if (and (null? splat) (length= expr 3) (eq? (car expr) 'call) @@ -1620,7 +1641,36 @@ (not (dotop? (cadr expr))) (not (expr-contains-eq argname (cadr expr)))) (cadr expr) ;; eta reduce `x->f(x)` => `f` - `(-> ,argname (block ,@splat ,expr))))) + (let ((expr (cond ((and flat (pair? expr) (eq? (car expr) 'generator)) + (expand-generator expr #f (delete-duplicates (append outervars myvars)))) + ((and flat (pair? expr) (eq? (car expr) 'flatten)) + (expand-generator (cadr expr) #t (delete-duplicates (append outervars myvars)))) + ((pair? outervars) + `(let (block ,@(map (lambda (v) `(= ,v ,v)) outervars)) + ,expr)) + (else expr)))) + `(-> ,argname (block ,@splat ,expr)))))) + +(define (expand-generator e flat outervars) + (let* ((expr (cadr e)) + (filt? (eq? (car (caddr e)) 'filter)) + (range-exprs (if filt? (cddr (caddr e)) (cddr e))) + (ranges (map caddr range-exprs)) + (iter (if (length= ranges 1) + (car ranges) + `(call (top product) ,@ranges))) + (iter (if filt? + `(call (|.| (top Iterators) 'Filter) + ,(func-for-generator-ranges (cadr (caddr e)) range-exprs #f '()) + ,iter) + iter)) + (gen `(call (top Generator) + ,(func-for-generator-ranges expr range-exprs flat outervars) + ,iter))) + (expand-forms + (if flat + `(call (top Flatten) ,gen) + gen)))) (define (ref-to-view expr) (if (and (pair? expr) (eq? (car expr) 'ref)) @@ -2217,12 +2267,6 @@ (break-block loop-cont (scope-block ,(blockify (expand-forms (caddr e)))))))) - 'inner-while - (lambda (e) - `(_while ,(expand-forms (cadr e)) - (break-block loop-cont - (scope-block ,(blockify (expand-forms (caddr e))))))) - 'break (lambda (e) (if (pair? (cdr e)) @@ -2233,16 +2277,10 @@ 'for (lambda (e) - (let nest ((ranges (if (eq? (car (cadr e)) 'block) - (cdr (cadr e)) - (list (cadr e)))) - (first #t)) - (expand-for (if first 'while 'inner-while) - (cadr (car ranges)) - (caddr (car ranges)) - (if (null? (cdr ranges)) - (caddr e) ;; body - (nest (cdr ranges) #f))))) + (let ((ranges (if (eq? (car (cadr e)) 'block) + (cdr (cadr e)) + (list (cadr e))))) + (expand-forms (expand-for (map cadr ranges) (map caddr ranges) (caddr e))))) '&& (lambda (e) (expand-forms (expand-and e))) '|\|\|| (lambda (e) (expand-forms (expand-or e))) @@ -2346,26 +2384,10 @@ (return (expand-forms `(call transpose ,(cadr e)))))) 'generator - (lambda (e) - (let* ((expr (cadr e)) - (filt? (eq? (car (caddr e)) 'filter)) - (range-exprs (if filt? (cddr (caddr e)) (cddr e))) - (ranges (map caddr range-exprs)) - (iter (if (length= ranges 1) - (car ranges) - `(call (top product) ,@ranges))) - (iter (if filt? - `(call (|.| (top Iterators) 'Filter) - ,(func-for-generator-ranges (cadr (caddr e)) range-exprs) - ,iter) - iter))) - (expand-forms - `(call (top Generator) - ,(func-for-generator-ranges expr range-exprs) - ,iter)))) + (lambda (e) (expand-generator e #f '())) 'flatten - (lambda (e) (expand-forms `(call (top Flatten) ,(cadr e)))) + (lambda (e) (expand-generator (cadr e) #t '())) 'comprehension (lambda (e) diff --git a/test/functional.jl b/test/functional.jl index a835ec693ae0a..8125884c41ba1 100644 --- a/test/functional.jl +++ b/test/functional.jl @@ -144,6 +144,30 @@ end @test [(i,j) for i=1:3 for j=1:i if j>1] == [(2,2), (3,2), (3,3)] +# issue #330 +@test [(t=(i,j); i=nothing; t) for i = 1:3 for j = 1:i] == + [(1, 1), (2, 1), (2, 2), (3, 1), (3, 2), (3, 3)] + +@test map(collect, (((t=(i,j); i=nothing; t) for j = 1:i) for i = 1:3)) == + [[(1, 1)], + [(2, 1), (nothing, 2)], + [(3, 1), (nothing, 2), (nothing, 3)]] + +let a = [] + for x = 1:3, y = 1:3 + push!(a, x) + x = 0 + end + @test a == [1,1,1,2,2,2,3,3,3] +end + +let i, j + for outer i = 1:2, j = 1:0 + end + @test i == 2 + @test !@isdefined(j) +end + # issue #18707 @test [(q,d,n,p) for q = 0:25:100 for d = 0:10:100-q