fix macro expansion of property destructuring

This got a bit messier than I would have liked, since macro expansion
can't simply rename the property without changing the meaning. This
introduces an internal form `:renamed` which still encodes the original
name.

Author: simeonschaub

src/julia-syntax.scm

@@ -2236,6 +2236,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)) "\"")))
@@ -2246,12 +2259,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))))
 

src/macroexpand.scm

@@ -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))))
 
            ((->)
@@ -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

test/syntax.jl

@@ -3465,3 +3465,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