fresh variables on each iteration in nested loop and generator syntax

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.
 

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
 

src/julia-syntax.scm

@@ -1562,50 +1562,102 @@
 ;; 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
+ (filter (lambda (x) (not (underscore-symbol? x)))
+ (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)
  (eq? (caddr expr) argname)
  (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)) (filter (lambda (x) (not (underscore-symbol? x)))
+ 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))
@@ -2202,12 +2254,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))
@@ -2218,16 +2264,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)))
@@ -2331,26 +2371,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)

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