From 6247a899b60f50bee3a0432ca5b96b8fbd2a8c55 Mon Sep 17 00:00:00 2001
From: Simeon Schaub <simeondavidschaub99@gmail.com>
Date: Thu, 1 Dec 2022 21:42:20 +0100
Subject: [PATCH] fix unescaping in `global` expressions (#47719)

This fixes some issues around macro hygiene in `global` expressions.
Apparently we always treat l-values in global expressions as being
escaped, but we still need to be careful to handle type annotations and
destructuring correctly.

(cherry picked from commit cc25a1369472756c63c4da81abbc106e2790b4f0)
---
 src/macroexpand.scm | 31 ++++++++++++++++++++++---------
 test/syntax.jl      | 23 +++++++++++++++++++++++
 2 files changed, 45 insertions(+), 9 deletions(-)

diff --git a/src/macroexpand.scm b/src/macroexpand.scm
index 8a42b26fa79a1..2933ca4888c4e 100644
--- a/src/macroexpand.scm
+++ b/src/macroexpand.scm
@@ -183,6 +183,19 @@
       (cadr e)
       e))
 
+(define (unescape-global-lhs e env m parent-scope inarg)
+  (cond ((not (pair? e)) e)
+        ((eq? (car e) 'escape) (cadr e))
+        ((memq (car e) '(parameters tuple))
+         (list* (car e) (map (lambda (e)
+                          (unescape-global-lhs e env m parent-scope inarg))
+                        (cdr e))))
+        ((and (memq (car e) '(|::| kw)) (length= e 3))
+         (list (car e) (unescape-global-lhs (cadr e) env m parent-scope inarg)
+                       (resolve-expansion-vars-with-new-env (caddr e) env m parent-scope inarg)))
+        (else
+         (resolve-expansion-vars-with-new-env e env m parent-scope inarg))))
+
 (define (typedef-expr-name e)
   (cond ((atom? e) e)
         ((or (eq? (car e) 'curly) (eq? (car e) '<:)) (typedef-expr-name (cadr e)))
@@ -344,15 +357,15 @@
                      (m (cadr scope))
                      (parent-scope (cdr parent-scope)))
                 (resolve-expansion-vars-with-new-env (cadr e) env m parent-scope inarg))))
-           ((global) (let ((arg (cadr e)))
-                       (cond ((symbol? arg) e)
-                             ((assignment? arg)
-                              `(global
-                                (= ,(unescape (cadr arg))
-                                   ,(resolve-expansion-vars-with-new-env (caddr arg) env m parent-scope inarg))))
-                             (else
-                              `(global ,(resolve-expansion-vars-with-new-env arg env m parent-scope inarg))))))
-           ((using import export meta line inbounds boundscheck loopinfo) (map unescape e))
+           ((global)
+            `(global
+               ,@(map (lambda (arg)
+                       (if (assignment? arg)
+                           `(= ,(unescape-global-lhs (cadr arg) env m parent-scope inarg)
+                               ,(resolve-expansion-vars-with-new-env (caddr arg) env m parent-scope inarg))
+                           (unescape-global-lhs arg env m parent-scope inarg)))
+                      (cdr e))))
+           ((using import export meta line inbounds boundscheck loopinfo inline noinline) (map unescape e))
            ((macrocall) e) ; invalid syntax anyways, so just act like it's quoted.
            ((symboliclabel) e)
            ((symbolicgoto) e)
diff --git a/test/syntax.jl b/test/syntax.jl
index 2c393af22a2cf..92169c2b85dd9 100644
--- a/test/syntax.jl
+++ b/test/syntax.jl
@@ -2802,3 +2802,26 @@ end
 let ex = :(const $(esc(:x)) = 1; (::typeof(2))() = $(esc(:x)))
     @test macroexpand(Main, Expr(:var"hygienic-scope", ex, Main)).args[3].args[1] == :((::$(GlobalRef(Main, :typeof))(2))())
 end
+
+macro _macroexpand(x, m=__module__)
+    :($__source__; macroexpand($m, Expr(:var"hygienic-scope", $(esc(Expr(:quote, x))), $m)))
+end
+
+@testset "unescaping in :global expressions" begin
+    m = @__MODULE__
+    @test @_macroexpand(global x::T) == :(global x::$(GlobalRef(m, :T)))
+    @test @_macroexpand(global (x, $(esc(:y)))) == :(global (x, y))
+    @test @_macroexpand(global (x::S, $(esc(:y))::$(esc(:T)))) ==
+        :(global (x::$(GlobalRef(m, :S)), y::T))
+    @test @_macroexpand(global (; x, $(esc(:y)))) == :(global (; x, y))
+    @test @_macroexpand(global (; x::S, $(esc(:y))::$(esc(:T)))) ==
+        :(global (; x::$(GlobalRef(m, :S)), y::T))
+
+    @test @_macroexpand(global x::T = a) == :(global x::$(GlobalRef(m, :T)) = $(GlobalRef(m, :a)))
+    @test @_macroexpand(global (x, $(esc(:y))) = a) == :(global (x, y) = $(GlobalRef(m, :a)))
+    @test @_macroexpand(global (x::S, $(esc(:y))::$(esc(:T))) = a) ==
+        :(global (x::$(GlobalRef(m, :S)), y::T) = $(GlobalRef(m, :a)))
+    @test @_macroexpand(global (; x, $(esc(:y))) = a) == :(global (; x, y) = $(GlobalRef(m, :a)))
+    @test @_macroexpand(global (; x::S, $(esc(:y))::$(esc(:T))) = a) ==
+        :(global (; x::$(GlobalRef(m, :S)), y::T) = $(GlobalRef(m, :a)))
+end