From d692b897be0afca7939f325b46bbd7007bd7503b Mon Sep 17 00:00:00 2001 From: Simeon Schaub Date: Fri, 28 May 2021 09:35:15 +0200 Subject: [PATCH] don't mutate lhs while destructuring (#40737) 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 --- NEWS.md | 3 +++ src/julia-syntax.scm | 38 ++++++++++++++++++++++++++++---------- test/syntax.jl | 11 +++++++++++ 3 files changed, 42 insertions(+), 10 deletions(-) diff --git a/NEWS.md b/NEWS.md index 3f9a7c582b7ef..515fc4c236ead 100644 --- a/NEWS.md +++ b/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 ----------------------------- diff --git a/src/julia-syntax.scm b/src/julia-syntax.scm index 6bc8401c9b3dd..d4d6711f2577f 100644 --- a/src/julia-syntax.scm +++ b/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 diff --git a/test/syntax.jl b/test/syntax.jl index 5771fcee00e6c..5a3af3b1863cb 100644 --- a/test/syntax.jl +++ b/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