Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix macro expansion of property destructuring #48726

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

fix macro expansion of property destructuring #48726

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
dbe3c99
fix macro expansion of property destructuring
simeonschaub Feb 19, 2023
0c6bada
Merge branch 'master' into sds/prop_destruct_macroexpand
KristofferC May 4, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 15 additions & 6 deletions src/julia-syntax.scm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2239,6 +2239,19 @@
(gensy))
(else (make-ssavalue))))

(define (extract-dest-prop field lhs)
(cond ((symbol? field) (list field field))
((and (pair? field) (eq? (car field) '|::|))
(extract-prop (cadr field)))
((globalref? field)
(list field (caddr field)))
((outerref? field)
(list field (cadr field)))
((and (pair? field) (eq? (car field) 'renamed))
(cdr field))
(else
(error (string "invalid assignment location \"" (deparse `(tuple ,lhs)) "\"")))))

(define (expand-property-destruct lhs x)
(if (not (length= lhs 1))
(error (string "invalid assignment location \"" (deparse `(tuple ,lhs)) "\"")))
Expand All @@ -2249,12 +2262,8 @@
,@ini
,@(map
(lambda (field)
(let ((prop (cond ((symbol? field) field)
((and (pair? field) (eq? (car field) '|::|) (symbol? (cadr field)))
(cadr field))
(else
(error (string "invalid assignment location \"" (deparse `(tuple ,lhs)) "\""))))))
(expand-forms `(= ,field (call (top getproperty) ,xx (quote ,prop))))))
(let ((dest-prop (extract-dest-prop field lhs)))
(expand-forms `(= ,(car dest-prop) (call (top getproperty) ,xx (quote ,(cadr dest-prop)))))))
lhss)
(unnecessary ,xx))))

Expand Down
20 changes: 13 additions & 7 deletions src/macroexpand.scm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -385,11 +385,16 @@
((parameters)
(cons 'parameters
(map (lambda (x)
;; `x` by itself after ; means `x=x`
(let ((x (if (and (not inarg) (symbol? x))
`(kw ,x ,x)
x)))
(resolve-expansion-vars- x env m parent-scope #f)))
(if (and inarg (try-arg-name x))
(let ((resolved (resolve-expansion-vars- x env m parent-scope inarg)))
(if (equal? resolved x)
x
`(renamed ,resolved ,(arg-name x))))
;; `x` by itself after ; means `x=x`
(let ((x (if (and (not inarg) (symbol? x))
`(kw ,x ,x)
x)))
(resolve-expansion-vars- x env m parent-scope inarg))))
(cdr e))))

((->)
Expand All @@ -401,9 +406,10 @@
;; in (kw x 1) inside an arglist, the x isn't actually a kwarg
`(,(car e) ,(resolve-in-function-lhs (cadr e) env m parent-scope inarg)
,(resolve-expansion-vars-with-new-env (caddr e) env m parent-scope inarg))
`(,(car e) ,@(map (lambda (x)
`(,(car e) ,(resolve-expansion-vars-with-new-env (cadr e) env m parent-scope #t)
,@(map (lambda (x)
(resolve-expansion-vars-with-new-env x env m parent-scope inarg))
(cdr e)))))
(cddr e)))))

((kw)
(cond
Expand Down
18 changes: 18 additions & 0 deletions test/syntax.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3464,3 +3464,21 @@ end
@test @_macroexpand(global (; x::S, $(esc(:y))::$(esc(:T))) = a) ==
:(global (; x::$(GlobalRef(m, :S)), y::T) = $(GlobalRef(m, :a)))
end

macro prop_destruct_macroexpand1()
:((; foo_prop_destruct1) = (foo_prop_destruct1 = 7,))
end
macro prop_destruct_macroexpand2()
:(let (; foo_prop_destruct2) = (foo_prop_destruct2 = 8,)
foo_prop_destruct2
end)
end

@testset "macro expansion of property destructuring" begin
m = @__MODULE__
@test @prop_destruct_macroexpand1() == (foo_prop_destruct1 = 7,)
@test m.foo_prop_destruct1 == 7

@test @prop_destruct_macroexpand2() == 8
@test !isdefined(m, :foo_prop_destruct2)
end