don't mutate lhs while destructuring (JuliaLang#40737)

Jeff was a bit sceptical about this in JuliaLang#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 JuliaLang#40574

Author: simeonschaub

NEWS.md

@@ -23,6 +23,9 @@ Language changes
 * `macroexpand`, `@macroexpand`, and `@macroexpand1` no longer wrap errors in a `LoadError`. To reduce breakage, `@test_throws` has been modified so that many affected tests will still pass ([#38379]].
 * The middle dot `·` (`\cdotp` U+00b7) and the Greek interpunct `·` (U+0387) are now treated as equivalent to the dot operator `⋅` (`\cdot` U+22c5) (#25157).
 * The minus sign `−` (`\minus` U+2212) is now treated as equivalent to the hyphen-minus sign `-` (U+002d).
+* Destructuring will no longer mutate values on the left hand side while iterating through values on the right hand side. In the example
+  of an array `x`, `x[2], x[1] = x` will now swap the first and second entry of `x`, whereas it used to fill both entries with `x[1]`
+  because `x[2]` was mutated during the iteration of `x`. ([#40737])
 
 Compiler/Runtime improvements
 -----------------------------

src/julia-syntax.scm

@@ -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

test/syntax.jl

@@ -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