Skip to content

Commit

Permalink
RFC: don't mutate lhs while destructuring
Browse files Browse the repository at this point in the history
@JeffBezanson was a bit sceptical about this in #40574, but I think this
is worth doing since it seems strictly more useful than the current
behavior and we already go out of our way in cases like `x, y = y, x` to
only assign to the lhs after the rhs has been evaluated, so I think
handling `x[2], x[1] = x` in a similar way would only be consisten. As a
general rule that assignment will always happen after destructuring does
not seem much less obvious than the current behavior to me. This might
technically be slightly breaking, but I would be surprised if people
relied on the current behavior here. Nevertheless, it would probably
good to run PkgEval if we decide to go with this change.

closes #40574
  • Loading branch information
simeonschaub committed May 6, 2021
1 parent fbe28e4 commit 4d07e2f
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 10 deletions.
32 changes: 22 additions & 10 deletions src/julia-syntax.scm
Original file line number Diff line number Diff line change
Expand Up @@ -2036,22 +2036,34 @@
(- n 1)
n))
n))
(st (gensy)))
(st (gensy))
(end '()))
`(block
,@(if (> n 0) `((local ,st)) '())
,@ini
,@(map (lambda (i lhs)
(expand-forms
(if (vararg? lhs)
`(= ,(cadr lhs) (call (top rest) ,xx ,@(if (eq? i 0) '() `(,st))))
(lower-tuple-assignment
(if (= i (- n 1))
(list lhs)
(list lhs st))
`(call (top indexed_iterate)
,xx ,(+ i 1) ,@(if (eq? i 0) '() `(,st)))))))
(let ((lhs- (cond ((or (symbol? lhs) (ssavalue? lhs))
lhs)
((vararg? lhs)
(if (or (symbol? (cadr lhs)) (ssavalue? (cadr lhs)))
lhs `(|...| ,(make-ssavalue))))
(else (make-ssavalue)))))
(if (not (eq? lhs lhs-))
(if (vararg? lhs)
(set! end (cons (expand-forms `(= ,(cadr lhs) ,(cadr lhs-))) end))
(set! end (cons (expand-forms `(= ,lhs ,lhs-)) end))))
(expand-forms
(if (vararg? lhs-)
`(= ,(cadr lhs-) (call (top rest) ,xx ,@(if (eq? i 0) '() `(,st))))
(lower-tuple-assignment
(if (= i (- n 1))
(list lhs-)
(list lhs- st))
`(call (top indexed_iterate)
,xx ,(+ i 1) ,@(if (eq? i 0) '() `(,st))))))))
(iota n)
lhss)
,@(reverse end)
(unnecessary ,xx))))))

;; move an assignment into the last statement of a block to keep more statements at top level
Expand Down
11 changes: 11 additions & 0 deletions test/syntax.jl
Original file line number Diff line number Diff line change
Expand Up @@ -2787,3 +2787,14 @@ macro m_nospecialize_unnamed_hygiene()
end

@test @m_nospecialize_unnamed_hygiene()(1) === Any

# https://github.com/JuliaLang/julia/issues/40574
@testset "no mutation while destructuring" begin
x = [1, 2]
x[2], x[1] = x
@test x == [2, 1]

x = [1, 2, 3]
x[3], x[1:2]... = x
@test x == [2, 3, 1]
end

0 comments on commit 4d07e2f

Please sign in to comment.