Skip to content

Commit bcccabf

Browse files
committed
RFC: don't mutate lhs while destructuring
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
1 parent fbe28e4 commit bcccabf

File tree

2 files changed

+39
-10
lines changed

2 files changed

+39
-10
lines changed

src/julia-syntax.scm

+28-10
Original file line numberDiff line numberDiff line change
@@ -2036,22 +2036,40 @@
20362036
(- n 1)
20372037
n))
20382038
n))
2039-
(st (gensy)))
2039+
(st (gensy))
2040+
(end '()))
20402041
`(block
20412042
,@(if (> n 0) `((local ,st)) '())
20422043
,@ini
20432044
,@(map (lambda (i lhs)
2044-
(expand-forms
2045-
(if (vararg? lhs)
2046-
`(= ,(cadr lhs) (call (top rest) ,xx ,@(if (eq? i 0) '() `(,st))))
2047-
(lower-tuple-assignment
2048-
(if (= i (- n 1))
2049-
(list lhs)
2050-
(list lhs st))
2051-
`(call (top indexed_iterate)
2052-
,xx ,(+ i 1) ,@(if (eq? i 0) '() `(,st)))))))
2045+
(let ((lhs- (cond ((or (symbol? lhs) (ssavalue? lhs))
2046+
lhs)
2047+
((vararg? lhs)
2048+
(let ((lhs- (cadr lhs)))
2049+
(if (or (symbol? lhs-) (ssavalue? lhs-))
2050+
lhs
2051+
`(|...| ,(if (eventually-call? lhs-)
2052+
(gensy)
2053+
(make-ssavalue))))))
2054+
;; can't use ssavalues if it's a function definition
2055+
((eventually-call? lhs) (gensy))
2056+
(else (make-ssavalue)))))
2057+
(if (not (eq? lhs lhs-))
2058+
(if (vararg? lhs)
2059+
(set! end (cons (expand-forms `(= ,(cadr lhs) ,(cadr lhs-))) end))
2060+
(set! end (cons (expand-forms `(= ,lhs ,lhs-)) end))))
2061+
(expand-forms
2062+
(if (vararg? lhs-)
2063+
`(= ,(cadr lhs-) (call (top rest) ,xx ,@(if (eq? i 0) '() `(,st))))
2064+
(lower-tuple-assignment
2065+
(if (= i (- n 1))
2066+
(list lhs-)
2067+
(list lhs- st))
2068+
`(call (top indexed_iterate)
2069+
,xx ,(+ i 1) ,@(if (eq? i 0) '() `(,st))))))))
20532070
(iota n)
20542071
lhss)
2072+
,@(reverse end)
20552073
(unnecessary ,xx))))))
20562074

20572075
;; move an assignment into the last statement of a block to keep more statements at top level

test/syntax.jl

+11
Original file line numberDiff line numberDiff line change
@@ -2787,3 +2787,14 @@ macro m_nospecialize_unnamed_hygiene()
27872787
end
27882788

27892789
@test @m_nospecialize_unnamed_hygiene()(1) === Any
2790+
2791+
# https://github.com/JuliaLang/julia/issues/40574
2792+
@testset "no mutation while destructuring" begin
2793+
x = [1, 2]
2794+
x[2], x[1] = x
2795+
@test x == [2, 1]
2796+
2797+
x = [1, 2, 3]
2798+
x[3], x[1:2]... = x
2799+
@test x == [2, 3, 1]
2800+
end

0 commit comments

Comments
 (0)