From bcccabf34a936fa80531ade5943c3d34b0cdddea Mon Sep 17 00:00:00 2001 From: Simeon Schaub Date: Tue, 27 Apr 2021 09:32:35 +0200 Subject: [PATCH] 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 --- src/julia-syntax.scm | 38 ++++++++++++++++++++++++++++---------- test/syntax.jl | 11 +++++++++++ 2 files changed, 39 insertions(+), 10 deletions(-) diff --git a/src/julia-syntax.scm b/src/julia-syntax.scm index ca274ef552f5b..d4fa954d64528 100644 --- a/src/julia-syntax.scm +++ b/src/julia-syntax.scm @@ -2036,22 +2036,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 d934e9358baac..e4c3f635aa181 100644 --- a/test/syntax.jl +++ b/test/syntax.jl @@ -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