fix macro goto/label hygiene

same basic idea as PR #22985 did for macro variables
also fixes #23135

Author: vtjnash

base/essentials.jl

@@ -300,11 +300,11 @@ macro inbounds(blk)
 end
 
 macro label(name::Symbol)
-    Expr(:symboliclabel, name)
+    return esc(Expr(:symboliclabel, name))
 end
 
 macro goto(name::Symbol)
-    Expr(:symbolicgoto, name)
+    return esc(Expr(:symbolicgoto, name))
 end
 
 # SimpleVector

src/julia-syntax.scm

@@ -2538,7 +2538,7 @@
 (define (resolve-scopes- e env outerglobals implicitglobals lam renames newlam)
   (cond ((symbol? e) (let ((r (assq e renames)))
                        (if r (cdr r) e))) ;; return the renaming for e, or e
-        ((or (not (pair? e)) (quoted? e) (memq (car e) '(toplevel global))) e)
+        ((or (not (pair? e)) (quoted? e) (memq (car e) '(toplevel global symbolicgoto symboliclabel))) e)
         ((eq? (car e) 'local) '(null)) ;; remove local decls
         ((eq? (car e) 'local-def) '(null)) ;; remove local decls
         ((eq? (car e) 'implicit-global) '(null)) ;; remove implicit-global decls

src/macroexpand.scm

@@ -453,39 +453,36 @@
   ;; and wrap globals in (globalref module var) for macro's home module
   (resolve-expansion-vars-with-new-env e '() m '() #f #t))
 
-(define (find-symbolic-labels e)
-  (let ((defs (table))
-        (refs (table)))
-    (find-symbolic-label-defs e defs)
-    (find-symbolic-label-refs e refs)
-    (table.foldl
-     (lambda (label v labels)
-       (if (has? refs label)
-           (cons label labels)
-           labels))
-     '() defs)))
-
-(define (rename-symbolic-labels- e relabel)
+(define (rename-symbolic-labels- e relabels parent-scope)
   (cond
    ((or (not (pair? e)) (quoted? e)) e)
-   ((eq? (car e) 'symbolicgoto)
-    (let ((newlabel (assq (cadr e) relabel)))
-      (if newlabel `(symbolicgoto ,(cdr newlabel)) e)))
-   ((eq? (car e) 'symboliclabel)
-    (let ((newlabel (assq (cadr e) relabel)))
-      (if newlabel `(symboliclabel ,(cdr newlabel)) e)))
-   (else (map (lambda (x) (rename-symbolic-labels- x relabel)) e))))
+   ((eq? (car e) 'hygienic-scope)
+     (let ((parent-scope (list relabels parent-scope))
+           (body (cadr e))
+           (m (caddr e)))
+     `(hygienic-scope ,(rename-symbolic-labels- (cadr e) (table) parent-scope) ,m)))
+   ((and (eq? (car e) 'escape) (not (null? parent-scope)))
+     `(escape ,(apply rename-symbolic-labels- (cadr e) parent-scope)))
+   ((or (eq? (car e) 'symbolicgoto) (eq? (car e) 'symboliclabel))
+    (let* ((s (cadr e))
+           (havelabel (if (or (null? parent-scope) (not (symbol? s))) s (get relabels s #f)))
+           (newlabel (if havelabel havelabel (named-gensy s))))
+      (if (not havelabel) (put! relabels s newlabel))
+      `(,(car e) ,newlabel)))
+   (else
+     (cons (car e)
+           (map (lambda (x) (rename-symbolic-labels- x relabels parent-scope))
+                (cdr e))))))
 
 (define (rename-symbolic-labels e)
-  (let* ((labels (find-symbolic-labels e))
-         (relabel (pair-with-gensyms labels)))
-    (rename-symbolic-labels- e relabel)))
+  (rename-symbolic-labels- e (table) '()))
 
 ;; macro expander entry point
 
 (define (julia-expand-macros e (max-depth -1))
   (julia-expand-macroscopes
-    (julia-expand-macros- '() e max-depth)))
+    (rename-symbolic-labels
+     (julia-expand-macros- '() e max-depth))))
 
 (define (julia-expand-macros- m e max-depth)
   (cond ((= max-depth 0)   e)
@@ -504,7 +501,7 @@
            (let ((form (car form)) ;; form is the expression returned from expand-macros
                  (modu (cdr form))) ;; modu is the macro's def module
              `(hygienic-scope
-               ,(julia-expand-macros- (cons modu m) (rename-symbolic-labels form) (- max-depth 1))
+               ,(julia-expand-macros- (cons modu m) form (- max-depth 1))
                ,modu))))
         ((eq? (car e) 'module) e)
         ((eq? (car e) 'escape)

test/goto.jl

@@ -1,6 +1,7 @@
 # This file is a part of Julia. License is MIT: https://julialang.org/license
 
 # Basic goto tests
+expand(x) = Base.expand(@__MODULE__, x)
 
 function goto_test1()
     @goto a
@@ -11,8 +12,13 @@ end
 @test goto_test1()
 
 
-@test_throws ErrorException eval(
-    quote
+@test eval(:(@label a)) === nothing
+
+@test Expr(:error, "label \"a\" referenced but not defined") ==
+    expand(:(@goto a))
+
+@test Expr(:error, "label \"a\" defined multiple times") ==
+    expand(quote
         function goto_test2()
             @goto a
             @label a
@@ -22,17 +28,16 @@ end
     end)
 
 
-@test_throws ErrorException eval(
-    quote
+@test Expr(:error, "label \"a\" referenced but not defined") ==
+    expand(quote
         function goto_test3()
             @goto a
             return
         end
     end)
 
-
-@test_throws ErrorException eval(
-    quote
+@test Expr(:error, "misplaced label") ==
+    expand(quote
         function goto_test4()
             @goto a
             try
@@ -43,23 +48,48 @@ end
     end)
 
 
-# test that labels in macros are reassigned
-macro goto_test5_macro()
-    @label a
+# test that labels in macros are reassigned if unescaped
+macro goto_test5_macro1()
+    return :(@label a)
+end
+macro goto_test5_macro2()
+    return :(@goto a)
+end
+macro goto_test5_macro3()
+    return esc(:(@label a))
 end
 
-@test_throws ErrorException eval(
-    quote
-        function goto_test5()
+@test Expr(:error, "label \"a\" referenced but not defined") ==
+    expand(quote
+        function goto_test5_1()
             @goto a
-            @goto_test5_macro
+            @goto_test5_macro1
             return
         end
     end)
 
+let e = expand(quote
+        function goto_test5_2()
+            @goto_test5_macro2
+            @label a
+            return
+        end
+    end)
+    @test (e::Expr).head === :error
+    @test ismatch(r"label \"#\d+#a\" referenced but not defined", e.args[1])
+end
+
+function goto_test5_3()
+    @goto a
+    return false
+    @goto_test5_macro3
+    return true
+end
+@test goto_test5_3()
+
 
-@test_throws ErrorException eval(
-    quote
+@test Expr(:error, "goto from a try/finally block is not permitted") ==
+    expand(quote
         function goto_test6()
             try
                 @goto a