Skip to content

Commit 9f643a0

Browse files
authored
Fix invalid let syntax from LHS views (#53108)
When the frontend lowers an expression like `lhs .+= rhs`, it needs to prevent evaluating the LHS more than once before re-writing to `lhs .= lhs .+ rhs`. If the LHS was a `let` block — commonly generated by `@views` and (since #53064) `@view` — the lowering pass had previously been emitting an invalid `let` temporary. This very directly addresses that case. Fixes #53107. Fixes #44356.
1 parent fdb342c commit 9f643a0

File tree

3 files changed

+66
-2
lines changed

3 files changed

+66
-2
lines changed

src/julia-syntax.scm

+4-2
Original file line numberDiff line numberDiff line change
@@ -1653,8 +1653,10 @@
16531653
(let ((g (make-ssavalue)))
16541654
(begin (set! a (cons `(= ,g ,x) a))
16551655
g)))))
1656-
(cons (cons (car e) (map arg-to-temp (cdr e)))
1657-
(reverse a)))))
1656+
(if (eq? (car e) 'let)
1657+
(cons (arg-to-temp e) (reverse a))
1658+
(cons (cons (car e) (map arg-to-temp (cdr e)))
1659+
(reverse a))))))
16581660

16591661
(define (lower-kw-call f args)
16601662
(let* ((para (if (has-parameters? args) (cdar args) '()))

test/subarray.jl

+44
Original file line numberDiff line numberDiff line change
@@ -637,6 +637,44 @@ end
637637
@test foo == [X, X]
638638
end
639639

640+
# Test as an assignment's left hand side
641+
let x = [1,2,3,4]
642+
@test Meta.@lower(@view(x[1]) = 1).head == :error
643+
@test Meta.@lower(@view(x[1]) += 1).head == :error
644+
@test Meta.@lower(@view(x[end]) = 1).head == :error
645+
@test Meta.@lower(@view(x[end]) += 1).head == :error
646+
@test Meta.@lower(@view(f(x)[end]) = 1).head == :error
647+
@test Meta.@lower(@view(f(x)[end]) += 1).head == :error
648+
@test (@view(x[1]) .+= 1) == fill(2)
649+
@test x == [2,2,3,4]
650+
@test (@view(reshape(x,2,2)[1,1]) .+= 10) == fill(12)
651+
@test x == [12,2,3,4]
652+
@test (@view(x[end]) .+= 1) == fill(5)
653+
@test x == [12,2,3,5]
654+
@test (@view(reshape(x,2,2)[end]) .+= 10) == fill(15)
655+
@test x == [12,2,3,15]
656+
@test (@view(reshape(x,2,2)[[begin],[begin,end]])::AbstractMatrix{Int} .+= [2]) == [14 5]
657+
@test x == [14,2,5,15]
658+
659+
x = [1,2,3,4]
660+
@test Meta.@lower(@views(x[[1]]) = 1).head == :error
661+
@test Meta.@lower(@views(x[[1]]) += 1).head == :error
662+
@test Meta.@lower(@views(x[[end]]) = 1).head == :error
663+
@test Meta.@lower(@views(x[[end]]) += 1).head == :error
664+
@test Meta.@lower(@views(f(x)[end]) = 1).head == :error
665+
@test Meta.@lower(@views(f(x)[end]) += 1).head == :error
666+
@test (@views(x[[1]]) .+= 1) == [2]
667+
@test x == [2,2,3,4]
668+
@test (@views(reshape(x,2,2)[[1],1]) .+= 10) == [12]
669+
@test x == [12,2,3,4]
670+
@test (@views(x[[end]]) .+= 1) == [5]
671+
@test x == [12,2,3,5]
672+
@test (@views(reshape(x,2,2)[[end]]) .+= 10) == [15]
673+
@test x == [12,2,3,15]
674+
@test (@views(reshape(x,2,2)[[begin],[begin,end]])::AbstractMatrix{Int} .+= [2]) == [14 5]
675+
@test x == [14,2,5,15]
676+
end
677+
640678
# test @views macro
641679
@views let f!(x) = x[begin:end-1] .+= x[begin+1:end].^2
642680
x = [1,2,3,4]
@@ -663,6 +701,12 @@ end
663701
@test x == [5,8,12,9] && i == [4,3]
664702
@. x[3:end] = 0 # make sure @. works with end expressions in @views
665703
@test x == [5,8,0,0]
704+
x[begin:end] .+= 1
705+
@test x == [6,9,1,1]
706+
x[[begin,2,end]] .-= [1,2,3]
707+
@test x == [5,7,1,-2]
708+
@. x[[begin,2,end]] .+= [1,2,3]
709+
@test x == [6,9,1,1]
666710
end
667711
@views @test isa(X[1:3], SubArray)
668712
@test X[begin:end] == @views X[begin:end]

test/syntax.jl

+18
Original file line numberDiff line numberDiff line change
@@ -3619,3 +3619,21 @@ end
36193619
@test p("public[7] = 5") == Expr(:(=), Expr(:ref, :public, 7), 5)
36203620
@test p("public() = 6") == Expr(:(=), Expr(:call, :public), Expr(:block, 6))
36213621
end
3622+
3623+
@testset "removing argument sideeffects" begin
3624+
# Allow let blocks in broadcasted LHSes, but only evaluate them once:
3625+
execs = 0
3626+
array = [1]
3627+
let x = array; execs += 1; x; end .+= 2
3628+
@test array == [3]
3629+
@test execs == 1
3630+
let; execs += 1; array; end .= 4
3631+
@test array == [4]
3632+
@test execs == 2
3633+
let x = array; execs += 1; x; end::Vector{Int} .+= 2
3634+
@test array == [6]
3635+
@test execs == 3
3636+
let; execs += 1; array; end::Vector{Int} .= 7
3637+
@test array == [7]
3638+
@test execs == 4
3639+
end

0 commit comments

Comments
 (0)