Skip to content

Commit

Permalink
RFC: don't mutate lhs while destructuring
Browse files Browse the repository at this point in the history
Jeff 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 consistent. 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 be good to run PkgEval if we decide to go with this change.

closes #40574
  • Loading branch information
simeonschaub committed May 27, 2021
1 parent 1913382 commit 823c6e0
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 10 deletions.
38 changes: 28 additions & 10 deletions src/julia-syntax.scm
Original file line number Diff line number Diff line change
Expand Up @@ -2143,22 +2143,40 @@
(- 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)
(let ((lhs- (cadr lhs)))
(if (or (symbol? lhs-) (ssavalue? lhs-))
lhs
`(|...| ,(if (eventually-call? lhs-)
(gensy)
(make-ssavalue))))))
;; can't use ssavalues if it's a function definition
((eventually-call? lhs) (gensy))
(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 @@ -2819,3 +2819,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 823c6e0

Please sign in to comment.