Merge pull request JuliaLang#22985 from JuliaLang/jn/22307

fix some macro expansion scope bugs

Author: vtjnash

base/deprecated.jl

@@ -232,25 +232,19 @@ end
 
 # For deprecating vectorized functions in favor of compact broadcast syntax
 macro dep_vectorize_1arg(S, f)
-    S = esc(S)
-    f = esc(f)
-    T = esc(:T)
-    x = esc(:x)
-    AbsArr = esc(:AbstractArray)
-    :( @deprecate $f($x::$AbsArr{$T}) where {$T<:$S} $f.($x) )
+    x = esc(:x) # work around macro hygiene bug
+    T = esc(:T) # work around macro hygiene bug
+    return :( @deprecate $f($x::AbstractArray{$T}) where {$T<:$S} $f.($x) )
 end
 macro dep_vectorize_2arg(S, f)
-    S = esc(S)
-    f = esc(f)
-    T1 = esc(:T1)
-    T2 = esc(:T2)
-    x = esc(:x)
-    y = esc(:y)
-    AbsArr = esc(:AbstractArray)
-    quote
-        @deprecate $f($x::$S, $y::$AbsArr{$T1}) where {$T1<:$S} $f.($x,$y)
-        @deprecate $f($x::$AbsArr{$T1}, $y::$S) where {$T1<:$S} $f.($x,$y)
-        @deprecate $f($x::$AbsArr{$T1}, $y::$AbsArr{$T2}) where {$T1<:$S,$T2<:$S} $f.($x,$y)
+    x = esc(:x) # work around macro hygiene bug
+    y = esc(:y) # work around macro hygiene bug
+    T1 = esc(:T1) # work around macro hygiene bug
+    T2 = esc(:T2) # work around macro hygiene bug
+    return quote
+        @deprecate $f($x::$S, $y::AbstractArray{$T1}) where {$T1<:$S} $f.($x, $y)
+        @deprecate $f($x::AbstractArray{$T1}, $y::$S) where {$T1<:$S} $f.($x, $y)
+        @deprecate $f($x::AbstractArray{$T1}, $y::AbstractArray{$T2}) where {$T1<:$S, $T2<:$S} $f.($x, $y)
     end
 end
 
@@ -338,20 +332,20 @@ for f in (
 end
 
 # Deprecate @vectorize_1arg and @vectorize_2arg themselves
-macro vectorize_1arg(S,f)
+macro vectorize_1arg(S, f)
     depwarn(string("`@vectorize_1arg` is deprecated in favor of compact broadcast syntax. ",
         "Instead of `@vectorize_1arg`'ing function `f` and calling `f(arg)`, call `f.(arg)`."),
         :vectorize_1arg)
     quote
-        @dep_vectorize_1arg($(esc(S)),$(esc(f)))
+        @dep_vectorize_1arg($S, $f)
     end
 end
-macro vectorize_2arg(S,f)
+macro vectorize_2arg(S, f)
     depwarn(string("`@vectorize_2arg` is deprecated in favor of compact broadcast syntax. ",
         "Instead of `@vectorize_2arg`'ing function `f` and calling `f(arg1, arg2)`, call ",
-        "`f.(arg1,arg2)`. "), :vectorize_2arg)
+        "`f.(arg1, arg2)`. "), :vectorize_2arg)
     quote
-        @dep_vectorize_2arg($(esc(S)),$(esc(f)))
+        @dep_vectorize_2arg($S, $f)
     end
 end
 export @vectorize_1arg, @vectorize_2arg

base/distributed/macros.jl

@@ -157,7 +157,7 @@ processes to have execute the expression.
 Equivalent to calling `remotecall_eval(Main, procs, expr)`.
 """
 macro everywhere(ex)
-    return :(@everywhere $procs() $(esc(ex))) # interpolation needs to work around hygiene bugs (#22307)
+    return :(@everywhere procs() $ex)
 end
 
 macro everywhere(procs, ex)

base/docs/utils.jl

@@ -195,15 +195,15 @@ end
 isregex(x) = isexpr(x, :macrocall, 3) && x.args[1] === Symbol("@r_str") && !isempty(x.args[3])
 repl(io::IO, ex::Expr) = isregex(ex) ? :(apropos($io, $ex)) : _repl(ex)
 repl(io::IO, str::AbstractString) = :(apropos($io, $str))
-repl(io::IO, other) = :(@doc $(esc(other)))
+repl(io::IO, other) = :(@doc $other)
 
 repl(x) = repl(STDOUT, x)
 
 function _repl(x)
     if (isexpr(x, :call) && !any(isexpr(x, :(::)) for x in x.args))
         x.args[2:end] = [:(::typeof($arg)) for arg in x.args[2:end]]
     end
-    docs = :(@doc $(esc(x)))
+    docs = :(@doc $x)
     if isfield(x)
         quote
             if isa($(esc(x.args[1])), DataType)

base/interactiveutil.jl

@@ -670,7 +670,7 @@ function workspace()
         :(const LastMain = $last),
         :(include(x) = $include($m, x))))
     empty!(package_locks)
-    nothing
+    return m
 end
 
 # testing

src/ast.c

@@ -173,17 +173,16 @@ value_t fl_invoke_julia_macro(fl_context_t *fl_ctx, value_t *args, uint32_t narg
     JL_TIMING(MACRO_INVOCATION);
     jl_ptls_t ptls = jl_get_ptls_states();
     jl_ast_context_t *ctx = jl_ast_ctx(fl_ctx);
-    if (nargs < 2) // macro name and location
-        argcount(fl_ctx, "invoke-julia-macro", nargs, 2);
-    nargs++; // add __module__ argument
+    if (nargs < 3) // macro name and location
+        argcount(fl_ctx, "invoke-julia-macro", nargs, 3);
     jl_method_instance_t *mfunc = NULL;
     jl_value_t **margs;
     // Reserve one more slot for the result
     JL_GC_PUSHARGS(margs, nargs + 1);
     int i;
-    margs[0] = scm_to_julia(fl_ctx, args[0], 1);
+    margs[0] = scm_to_julia(fl_ctx, args[1], 1);
     // __source__ argument
-    jl_value_t *lno = scm_to_julia(fl_ctx, args[1], 1);
+    jl_value_t *lno = scm_to_julia(fl_ctx, args[2], 1);
     margs[1] = lno;
     if (jl_is_expr(lno) && ((jl_expr_t*)lno)->head == line_sym) {
         jl_value_t *file = jl_nothing;
@@ -206,12 +205,16 @@ value_t fl_invoke_julia_macro(fl_context_t *fl_ctx, value_t *args, uint32_t narg
     }
     margs[2] = (jl_value_t*)ctx->module;
     for (i = 3; i < nargs; i++)
-        margs[i] = scm_to_julia(fl_ctx, args[i - 1], 1);
+        margs[i] = scm_to_julia(fl_ctx, args[i], 1);
+    margs[nargs] = scm_to_julia(fl_ctx, args[0], 1); // module context for @macrocall argument
     jl_value_t *result = NULL;
     size_t world = jl_get_ptls_states()->world_age;
 
     JL_TRY {
-        margs[0] = jl_toplevel_eval(ctx->module, margs[0]);
+        jl_module_t *m = (jl_module_t*)margs[nargs];
+        if (!jl_is_module(m))
+            m = ctx->module;
+        margs[0] = jl_toplevel_eval(m, margs[0]);
         mfunc = jl_method_lookup(jl_gf_mtable(margs[0]), margs, nargs, 1, world);
         if (mfunc == NULL) {
             jl_method_error((jl_function_t*)margs[0], margs, nargs, world);

src/macroexpand.scm

@@ -46,6 +46,10 @@
                         (call (top append_any) ,@forms)))
                (loop (cdr p) (cons (julia-bq-bracket (car p) d) q)))))))
 
+(define (julia-bq-expand-hygienic x unhygienic)
+  (let ((expanded (julia-bq-expand x 0)))
+    (if unhygienic expanded `(escape ,expanded))))
+
 ;; hygiene
 
 ;; return the names of vars introduced by forms, instead of their transformations.
@@ -191,6 +195,7 @@
          (case (car v)
            ((... kw |::|) (try-arg-name (cadr v)))
            ((escape) (list v))
+           ((hygienic-scope) (try-arg-name (cadr v)))
            ((meta)  ;; allow certain per-argument annotations
             (if (nospecialize-meta? v #t)
                 (try-arg-name (caddr v))
@@ -230,15 +235,15 @@
 ;; resolve-expansion-vars-with-new-env, but turn on `inarg` once we get inside
 ;; the formal argument list. `e` in general might be e.g. `(f{T}(x)::T) where T`,
 ;; and we want `inarg` to be true for the `(x)` part.
-(define (resolve-in-function-lhs e env m inarg)
-  (define (recur x) (resolve-in-function-lhs x env m inarg))
-  (define (other x) (resolve-expansion-vars-with-new-env x env m inarg))
+(define (resolve-in-function-lhs e env m parent-scope inarg)
+  (define (recur x) (resolve-in-function-lhs x env m parent-scope inarg))
+  (define (other x) (resolve-expansion-vars-with-new-env x env m parent-scope inarg))
   (case (car e)
     ((where) `(where ,(recur (cadr e)) ,@(map other (cddr e))))
     ((|::|)  `(|::| ,(recur (cadr e)) ,(other (caddr e))))
     ((call)  `(call ,(other (cadr e))
                     ,@(map (lambda (x)
-                             (resolve-expansion-vars-with-new-env x env m #t))
+                             (resolve-expansion-vars-with-new-env x env m parent-scope #t))
                            (cddr e))))
     (else (other e))))
 
@@ -268,17 +273,17 @@
                             (diff (keywords-introduced-by x) globals))))
               env)))))))
 
-(define (resolve-expansion-vars-with-new-env x env m inarg (outermost #f))
+(define (resolve-expansion-vars-with-new-env x env m parent-scope inarg (outermost #f))
   (resolve-expansion-vars-
    x
    (if (and (pair? x) (eq? (car x) 'let))
        ;; let is strange in that it needs both old and new envs within
        ;; the same expression
        env
        (new-expansion-env-for x env outermost))
-   m inarg))
+   m parent-scope inarg))
 
-(define (resolve-expansion-vars- e env m inarg)
+(define (resolve-expansion-vars- e env m parent-scope inarg)
   (cond ((or (eq? e 'true) (eq? e 'false) (eq? e 'end) (eq? e 'ccall))
          e)
         ((symbol? e)
@@ -291,7 +296,13 @@
         (else
          (case (car e)
            ((ssavalue) e)
-           ((escape) (cadr e))
+           ((escape) (if (null? parent-scope)
+              (julia-expand-macroscopes (cadr e))
+              (let* ((scope (car parent-scope))
+                     (env (car scope))
+                     (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)
@@ -301,77 +312,78 @@
                              (else
                               `(global ,(resolve-expansion-vars-with-new-env arg env m inarg))))))
            ((using import importall export meta line inbounds boundscheck simdloop) (map unescape e))
-           ((macrocall)
-            (if (or (eq? (cadr e) '@label) (eq? (cadr e) '@goto)) e
-                `(macrocall ,.(map (lambda (x)
-                                     (resolve-expansion-vars-with-new-env x env m inarg))
-                                   (cdr e)))))
+           ((macrocall) e) ; invalid syntax anyways, so just act like it's quoted.
            ((symboliclabel) e)
            ((symbolicgoto) e)
            ((type)
-            `(type ,(cadr e) ,(resolve-expansion-vars- (caddr e) env m inarg)
+            `(type ,(cadr e) ,(resolve-expansion-vars- (caddr e) env m parent-scope inarg)
                    ;; type has special behavior: identifiers inside are
                    ;; field names, not expressions.
                    ,(map (lambda (x)
                            (cond ((atom? x) x)
                                  ((and (pair? x) (eq? (car x) '|::|))
                                   `(|::| ,(cadr x)
-                                    ,(resolve-expansion-vars- (caddr x) env m inarg)))
+                                    ,(resolve-expansion-vars- (caddr x) env m parent-scope inarg)))
                                  (else
-                                  (resolve-expansion-vars-with-new-env x env m inarg))))
+                                  (resolve-expansion-vars-with-new-env x env m parent-scope inarg))))
                          (cadddr e))))
 
            ((parameters)
             (cons 'parameters
                   (map (lambda (x)
-                         (resolve-expansion-vars- x env m #f))
+                         (resolve-expansion-vars- x env m parent-scope #f))
                        (cdr e))))
 
            ((= function)
             (if (and (pair? (cadr e)) (function-def? e))
                 ;; in (kw x 1) inside an arglist, the x isn't actually a kwarg
-                `(,(car e) ,(resolve-in-function-lhs (cadr e) env m inarg)
-                  ,(resolve-expansion-vars-with-new-env (caddr e) env m inarg))
+                `(,(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)
-                                    (resolve-expansion-vars-with-new-env x env m inarg))
+                                    (resolve-expansion-vars-with-new-env x env m parent-scope inarg))
                                   (cdr e)))))
 
            ((kw)
             (if (and (pair? (cadr e))
                      (eq? (caadr e) '|::|))
                 `(kw (|::|
                       ,(if inarg
-                           (resolve-expansion-vars- (cadr (cadr e)) env m inarg)
+                           (resolve-expansion-vars- (cadr (cadr e)) env m parent-scope inarg)
                            ;; in keyword arg A=B, don't transform "A"
                            (unescape (cadr (cadr e))))
-                      ,(resolve-expansion-vars- (caddr (cadr e)) env m inarg))
-                     ,(resolve-expansion-vars- (caddr e) env m inarg))
+                      ,(resolve-expansion-vars- (caddr (cadr e)) env m parent-scope inarg))
+                     ,(resolve-expansion-vars- (caddr e) env m parent-scope inarg))
                 `(kw ,(if inarg
-                          (resolve-expansion-vars- (cadr e) env m inarg)
+                          (resolve-expansion-vars- (cadr e) env m parent-scope inarg)
                           (unescape (cadr e)))
-                     ,(resolve-expansion-vars- (caddr e) env m inarg))))
+                     ,(resolve-expansion-vars- (caddr e) env m parent-scope inarg))))
 
            ((let)
             (let* ((newenv (new-expansion-env-for e env))
-                   (body   (resolve-expansion-vars- (cadr e) newenv m inarg)))
+                   (body   (resolve-expansion-vars- (cadr e) newenv m parent-scope inarg)))
               `(let ,body
                  ,@(map
                     (lambda (bind)
                       (if (assignment? bind)
                           (make-assignment
                            ;; expand binds in old env with dummy RHS
                            (cadr (resolve-expansion-vars- (make-assignment (cadr bind) 0)
-                                                          newenv m inarg))
+                                                          newenv m parent-scope inarg))
                            ;; expand initial values in old env
-                           (resolve-expansion-vars- (caddr bind) env m inarg))
+                           (resolve-expansion-vars- (caddr bind) env m parent-scope inarg))
                           bind))
                     (cddr e)))))
+           ((hygienic-scope) ; TODO: move this lowering to resolve-scopes, instead of reimplementing it here badly
+             (let ((parent-scope (cons (list env m) parent-scope))
+                   (body (cadr e))
+                   (m (caddr e)))
+              (resolve-expansion-vars-with-new-env body env m parent-scope inarg)))
 
            ;; todo: trycatch
            (else
             (cons (car e)
                   (map (lambda (x)
-                         (resolve-expansion-vars-with-new-env x env m inarg))
+                         (resolve-expansion-vars-with-new-env x env m parent-scope inarg))
                        (cdr e))))))))
 
 ;; decl-var that also identifies f in f()=...
@@ -398,6 +410,7 @@
 (define (find-declared-vars-in-expansion e decl (outer #t))
   (cond ((or (not (pair? e)) (quoted? e)) '())
         ((eq? (car e) 'escape)  '())
+        ((eq? (car e) 'hygienic-scope)  '())
         ((eq? (car e) decl)     (map decl-var* (cdr e)))
         ((and (not outer) (function-def? e)) '())
         (else
@@ -408,6 +421,7 @@
 (define (find-assigned-vars-in-expansion e (outer #t))
   (cond ((or (not (pair? e)) (quoted? e))  '())
         ((eq? (car e) 'escape)  '())
+        ((eq? (car e) 'hygienic-scope)  '())
         ((and (not outer) (function-def? e))
          ;; pick up only function name
          (let ((fname (cond ((eq? (car e) '=) (decl-var* (cadr e)))
@@ -436,8 +450,8 @@
 (define (resolve-expansion-vars e m)
   ;; expand binding form patterns
   ;; keep track of environment, rename locals to gensyms
-  ;; and wrap globals in (getfield module var) for macro's home module
-  (resolve-expansion-vars-with-new-env e '() m #f #t))
+  ;; 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))
@@ -470,24 +484,45 @@
 ;; macro expander entry point
 
 (define (julia-expand-macros e (max-depth -1))
+  (julia-expand-macroscopes
+    (julia-expand-macros- '() e max-depth)))
+
+(define (julia-expand-macros- m e max-depth)
   (cond ((= max-depth 0)   e)
-        ((not (pair? e))     e)
+        ((not (pair? e)) e)
         ((eq? (car e) 'quote)
-         ;; backquote is essentially a built-in macro at the moment
-         (julia-expand-macros (julia-bq-expand (cadr e) 0) max-depth))
+         ;; backquote is essentially a built-in unhygienic macro at the moment
+         (julia-expand-macros- m (julia-bq-expand-hygienic (cadr e) (null? m)) max-depth))
         ((eq? (car e) 'inert) e)
         ((eq? (car e) 'macrocall)
          ;; expand macro
-         (let ((form (apply invoke-julia-macro (cadr e) (cddr e))))
+         (let ((form (apply invoke-julia-macro (if (null? m) 'false (car m)) (cdr e))))
            (if (not form)
                (error (string "macro \"" (cadr e) "\" not defined")))
            (if (and (pair? form) (eq? (car form) 'error))
                (error (cadr form)))
-           (let ((form (car form))
-                 (m    (cdr form)))
-             ;; m is the macro's def module
-             (rename-symbolic-labels
-              (julia-expand-macros (resolve-expansion-vars form m) (- max-depth 1))))))
+           (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))
+               ,modu))))
+        ((eq? (car e) 'module) e)
+        ((eq? (car e) 'escape)
+         (let ((m (if (null? m) m (cdr m))))
+           `(escape ,(julia-expand-macros- m (cadr e) max-depth))))
+        (else
+         (map (lambda (ex)
+                (julia-expand-macros- m ex max-depth))
+              e))))
+
+;; TODO: delete this file and fold this operation into resolve-scopes
+(define (julia-expand-macroscopes e)
+  (cond ((not (pair? e)) e)
+        ((eq? (car e) 'inert) e)
         ((eq? (car e) 'module) e)
+        ((eq? (car e) 'hygienic-scope)
+           (let ((form (cadr e)) ;; form is the expression returned from expand-macros
+                 (modu (caddr e))) ;; m is the macro's def module
+             (resolve-expansion-vars form modu)))
         (else
-         (map (lambda (ex) (julia-expand-macros ex max-depth)) e))))
+         (map julia-expand-macroscopes e))))