Skip to content

Commit 2be8847

Browse files
authored
Fix remove-argument-side-effects with keyword call (#60195)
Fix #60152, which impacted both lowering implementations. `remove-argument-side-effects` assumed all `kw` arguments from a `parameters` block had already been dumped into the argument list, which is not true in some cases. In addition: - JuliaLowering hit a MethodError in the dumped-`kw` case regardless. There are other issues with `kw` which I'm ignoring in this PR (see #60162) - Delete some ancient history: `&` [used to be a valid argument](a378b75#diff-5d79463faae0f7f19454c7f9888498d9f876082e258ab3efdca36a0ee64b0c87L72) head sometime in 2012 apparently!
1 parent ddf1b02 commit 2be8847

File tree

4 files changed

+35
-6
lines changed

4 files changed

+35
-6
lines changed

JuliaLowering/src/desugaring.jl

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -539,7 +539,11 @@ function _arg_to_temp(ctx, stmts, ex, eq_is_kw=false)
539539
elseif k == K"..."
540540
@ast ctx ex [k _arg_to_temp(ctx, stmts, ex[1])]
541541
elseif k == K"=" && eq_is_kw
542-
@ast ctx ex [K"=" ex[1] _arg_to_temp(ex[2])]
542+
@ast ctx ex [K"=" ex[1] _arg_to_temp(ctx, stmts, ex[2], false)]
543+
elseif k == K"parameters"
544+
mapchildren(ctx, ex) do e
545+
_arg_to_temp(ctx, stmts, e, true)
546+
end
543547
else
544548
emit_assign_tmp(stmts, ctx, ex)
545549
end

JuliaLowering/test/assignments.jl

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,4 +95,18 @@ let
9595
end
9696
""") == 3
9797

98+
# removing argument side effect in kwcall lhs
99+
@eval test_mod f60152(v, pa; kw) = copy(v)
100+
@test JuliaLowering.include_string(test_mod, """
101+
f60152([1, 2, 3], 0; kw=0) .*= 2
102+
""") == [2,4,6]
103+
@test JuliaLowering.include_string(test_mod, """
104+
let
105+
pa_execs = 0
106+
kw_execs = 0
107+
out = f60152([1, 2, 3], (pa_execs+=1); kw=(kw_execs+=1)) .*= 2
108+
(out, pa_execs, kw_execs)
109+
end
110+
""") == ([2,4,6], 1, 1)
111+
98112
end

src/julia-syntax.scm

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1809,11 +1809,13 @@
18091809
(cons e '())
18101810
(let ((a '()))
18111811
(define (arg-to-temp x)
1812-
(cond ((effect-free? x) x)
1813-
((or (eq? (car x) '...) (eq? (car x) '&))
1814-
`(,(car x) ,(arg-to-temp (cadr x))))
1812+
(cond ((effect-free? x) x)
1813+
((eq? (car x) '...)
1814+
`(... ,(arg-to-temp (cadr x))))
18151815
((eq? (car x) 'kw)
1816-
`(,(car x) ,(cadr x) ,(arg-to-temp (caddr x))))
1816+
`(kw ,(cadr x) ,(arg-to-temp (caddr x))))
1817+
((eq? (car x) 'parameters)
1818+
`(parameters ,@(map arg-to-temp (cdr x))))
18171819
(else
18181820
(let ((g (make-ssavalue)))
18191821
(begin (set! a (cons `(= ,g ,x) a))

test/syntax.jl

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3730,7 +3730,7 @@ end
37303730
@test p("public() = 6") == Expr(:(=), Expr(:call, :public), Expr(:block, 6))
37313731
end
37323732

3733-
@testset "removing argument sideeffects" begin
3733+
@testset "removing argument side effects" begin
37343734
# Allow let blocks in broadcasted LHSes, but only evaluate them once:
37353735
execs = 0
37363736
array = [1]
@@ -3746,6 +3746,15 @@ end
37463746
let; execs += 1; array; end::Vector{Int} .= 7
37473747
@test array == [7]
37483748
@test execs == 4
3749+
3750+
# remove argument side effects on lhs kwcall
3751+
pa_execs = 0
3752+
kw_execs = 0
3753+
f60152(v, pa; kw) = copy(v)
3754+
@test (f60152([1, 2, 3], 0; kw=0) .*= 2) == [2,4,6]
3755+
@test (f60152([1, 2, 3], (pa_execs+=1); kw=(kw_execs+=1)) .*= 2) == [2,4,6]
3756+
@test pa_execs === 1
3757+
@test kw_execs === 1
37493758
end
37503759

37513760
# Allow GlobalRefs in macro definition

0 commit comments

Comments
 (0)