macroexpand: preserve hygiene markers during early expansion

This is to preserve the line number. But we make slightly more changes
than strictly necessary to prepare for future improvements in this area.

Author: vtjnash

base/boot.jl

@@ -533,11 +533,10 @@ import Core: CodeInfo, MethodInstance, CodeInstance, GotoNode, GotoIfNot, Return
 end # module IR
 
 # docsystem basics
-const unescape = Symbol("hygienic-scope")
 macro doc(x...)
     docex = atdoc(__source__, __module__, x...)
     isa(docex, Expr) && docex.head === :escape && return docex
-    return Expr(:escape, Expr(unescape, docex, typeof(atdoc).name.module))
+    return Expr(:escape, Expr(:var"hygienic-scope", docex, typeof(atdoc).name.module, __source__))
 end
 macro __doc__(x)
     return Expr(:escape, Expr(:block, Expr(:meta, :doc), x))

base/docs/Docs.jl

@@ -286,12 +286,26 @@ catdoc(xs...) = vcat(xs...)
 const keywords = Dict{Symbol, DocStr}()
 
 function unblock(@nospecialize ex)
+    while isexpr(ex, :var"hygienic-scope")
+        isexpr(ex.args[1], :escape) || break
+        ex = ex.args[1].args[1]
+    end
     isexpr(ex, :block) || return ex
     exs = filter(ex -> !(isa(ex, LineNumberNode) || isexpr(ex, :line)), ex.args)
     length(exs) == 1 || return ex
     return unblock(exs[1])
 end
 
+# peek through ex to figure out what kind of expression it may eventually act like
+# but ignoring scopes and line numbers
+function unescape(@nospecialize ex)
+    ex = unblock(ex)
+    while isexpr(ex, :escape) || isexpr(ex, :var"hygienic-scope")
+       ex = unblock(ex.args[1])
+    end
+    return ex
+end
+
 uncurly(@nospecialize ex) = isexpr(ex, :curly) ? ex.args[1] : ex
 
 namify(@nospecialize x) = astname(x, isexpr(x, :macro))::Union{Symbol,Expr,GlobalRef}
@@ -351,18 +365,19 @@ function metadata(__source__, __module__, expr, ismodule)
         fields = P[]
         last_docstr = nothing
         for each in (expr.args[3]::Expr).args
-            if isa(each, Symbol) || isexpr(each, :(::))
+            eachex = unescape(each)
+            if isa(eachex, Symbol) || isexpr(eachex, :(::))
                 # a field declaration
                 if last_docstr !== nothing
-                    push!(fields, P(namify(each::Union{Symbol,Expr}), last_docstr))
+                    push!(fields, P(namify(eachex::Union{Symbol,Expr}), last_docstr))
                     last_docstr = nothing
                 end
-            elseif isexpr(each, :function) || isexpr(each, :(=))
+            elseif isexpr(eachex, :function) || isexpr(eachex, :(=))
                 break
-            elseif isa(each, String) || isexpr(each, :string) || isexpr(each, :call) ||
-                (isexpr(each, :macrocall) && each.args[1] === Symbol("@doc_str"))
+            elseif isa(eachex, String) || isexpr(eachex, :string) || isexpr(eachex, :call) ||
+                (isexpr(eachex, :macrocall) && eachex.args[1] === Symbol("@doc_str"))
                 # forms that might be doc strings
-                last_docstr = each::Union{String,Expr}
+                last_docstr = each
             end
         end
         dict = :($(Dict{Symbol,Any})($([(:($(P)($(quot(f)), $d)))::Expr for (f, d) in fields]...)))
@@ -627,8 +642,9 @@ function loaddocs(docs::Vector{Core.SimpleVector})
     for (mod, ex, str, file, line) in docs
         data = Dict{Symbol,Any}(:path => string(file), :linenumber => line)
         doc = docstr(str, data)
-        docstring = docm(LineNumberNode(line, file), mod, doc, ex, false) # expand the real @doc macro now
-        Core.eval(mod, Expr(Core.unescape, docstring, Docs))
+        lno = LineNumberNode(line, file)
+        docstring = docm(lno, mod, doc, ex, false) # expand the real @doc macro now
+        Core.eval(mod, Expr(:var"hygienic-scope", docstring, Docs, lno))
     end
     empty!(docs)
     nothing

base/osutils.jl

@@ -16,7 +16,7 @@ macro static(ex)
         @label loop
         hd = ex.head
         if hd ∈ (:if, :elseif, :&&, :||)
-            cond = Core.eval(__module__, ex.args[1])
+            cond = Core.eval(__module__, ex.args[1])::Bool
             if xor(cond, hd === :||)
                 return esc(ex.args[2])
             elseif length(ex.args) == 3

base/threadcall.jl

@@ -47,7 +47,7 @@ macro threadcall(f, rettype, argtypes, argvals...)
     push!(body, :(return Int(Core.sizeof($rettype))))
 
     # return code to generate wrapper function and send work request thread queue
-    wrapper = Expr(Symbol("hygienic-scope"), wrapper, @__MODULE__)
+    wrapper = Expr(:var"hygienic-scope", wrapper, @__MODULE__, __source__)
     return :(let fun_ptr = @cfunction($wrapper, Int, (Ptr{Cvoid}, Ptr{Cvoid}, Ptr{Cvoid}))
         # use cglobal to look up the function on the calling thread
         do_threadcall(fun_ptr, cglobal($f), $rettype, Any[$(argtypes...)], Any[$(argvals...)])

base/util.jl

@@ -604,7 +604,7 @@ macro kwdef(expr)
         kwdefs = nothing
     end
     return quote
-        Base.@__doc__ $(esc(expr))
+        $(esc(:($Base.@__doc__ $expr)))
         $kwdefs
     end
 end

doc/make.jl

@@ -265,12 +265,6 @@ DocMeta.setdocmeta!(
     maybe_revise(:(using Base.BinaryPlatforms));
     recursive=true, warn=false,
 )
-DocMeta.setdocmeta!(
-    Pkg.LazilyInitializedFields,
-    :DocTestSetup,
-    maybe_revise(:(using Pkg.LazilyInitializedFields));
-    recursive=true, warn=false,
-)
 
 let r = r"buildroot=(.+)", i = findfirst(x -> occursin(r, x), ARGS)
     global const buildroot = i === nothing ? (@__DIR__) : first(match(r, ARGS[i]).captures)

doc/src/manual/metaprogramming.md

@@ -517,7 +517,7 @@ Now, consider a slightly more complex macro:
 
 ```jldoctest sayhello2
 julia> macro sayhello(name)
-           return :( println("Hello, ", $name) )
+           return esc(:( println("Hello, ", $name) ))
        end
 @sayhello (macro with 1 method)
 ```
@@ -768,15 +768,20 @@ of expressions inside the macro body.
 ### Hygiene
 
 An issue that arises in more complex macros is that of [hygiene](https://en.wikipedia.org/wiki/Hygienic_macro).
-In short, macros must ensure that the variables they introduce in their returned expressions do
-not accidentally clash with existing variables in the surrounding code they expand into. Conversely,
-the expressions that are passed into a macro as arguments are often *expected* to evaluate in
-the context of the surrounding code, interacting with and modifying the existing variables. Another
-concern arises from the fact that a macro may be called in a different module from where it was
-defined. In this case we need to ensure that all global variables are resolved to the correct
-module. Julia already has a major advantage over languages with textual macro expansion (like
-C) in that it only needs to consider the returned expression. All the other variables (such as
-`msg` in `@assert` above) follow the [normal scoping block behavior](@ref scope-of-variables).
+We saw this a bit earlier with the calls to `esc` that wrapped the return value of the macros so far.
+
+This already gives Julia a major advantage over languages with textual macro expansion (like
+C) in that it only needs to consider the returned expression, and it cannot change the
+parsing of the expression surrounding it. Thus, all the other variables (such as `msg` in
+`@assert` above) follow the [normal scoping block behavior](@ref scope-of-variables).
+
+Hygiene markers exists because macros must ensure that the variables they introduce in their
+returned expressions do not accidentally clash with existing variables in the surrounding
+code they expand into. Conversely, the expressions that are passed into a macro as arguments
+are often *expected* to evaluate in the context of the surrounding code, interacting with
+and modifying the existing variables. Another concern arises from the fact that a macro may
+be called in a different module from where it was defined. In this case we need to ensure
+that all global variables are resolved to the correct module.
 
 To demonstrate these issues, let us consider writing a `@time` macro that takes an expression
 as its argument, records the time, evaluates the expression, records the time again, prints the

src/ast.c

@@ -434,6 +434,8 @@ static jl_value_t *scm_to_julia(fl_context_t *fl_ctx, value_t e, jl_module_t *mo
     }
     JL_CATCH {
         // if expression cannot be converted, replace with error expr
+        //jl_(jl_current_exception());
+        //jlbacktrace();
         jl_expr_t *ex = jl_exprn(jl_error_sym, 1);
         v = (jl_value_t*)ex;
         jl_array_ptr_set(ex->args, 0, jl_cstr_to_string("invalid AST"));
@@ -998,7 +1000,59 @@ int jl_has_meta(jl_array_t *body, jl_sym_t *sym) JL_NOTSAFEPOINT
     return 0;
 }
 
-static jl_value_t *jl_invoke_julia_macro(jl_array_t *args, jl_module_t *inmodule, jl_module_t **ctx, size_t world, int throw_load_error)
+// Utility function to return whether `e` is any of the special AST types or
+// will always evaluate to itself exactly unchanged. This corresponds to
+// `is_self_quoting` in Core.Compiler utilities.
+int jl_is_ast_node(jl_value_t *e) JL_NOTSAFEPOINT
+{
+    return jl_is_newvarnode(e)
+        || jl_is_code_info(e)
+        || jl_is_linenode(e)
+        || jl_is_gotonode(e)
+        || jl_is_gotoifnot(e)
+        || jl_is_returnnode(e)
+        || jl_is_ssavalue(e)
+        || jl_is_slotnumber(e)
+        || jl_is_argument(e)
+        || jl_is_quotenode(e)
+        || jl_is_globalref(e)
+        || jl_is_symbol(e)
+        || jl_is_pinode(e)
+        || jl_is_phinode(e)
+        || jl_is_phicnode(e)
+        || jl_is_upsilonnode(e)
+        || jl_is_expr(e);
+}
+
+static int is_self_quoting_expr(jl_expr_t *e)
+{
+    return (e->head == jl_inert_sym ||
+            e->head == jl_core_sym ||
+            e->head == jl_line_sym ||
+            e->head == jl_lineinfo_sym ||
+            e->head == jl_meta_sym ||
+            e->head == jl_boundscheck_sym ||
+            e->head == jl_inline_sym ||
+            e->head == jl_noinline_sym);
+}
+
+// any AST, except those that cannot contain symbols
+// and have no side effects
+int need_esc_node(jl_value_t *e) JL_NOTSAFEPOINT
+{
+    if (jl_is_linenode(e)
+        || jl_is_ssavalue(e)
+        || jl_is_slotnumber(e)
+        || jl_is_argument(e)
+        || jl_is_quotenode(e))
+        return 0;
+    if (jl_is_expr(e))
+        return !is_self_quoting_expr((jl_expr_t*)e);
+    // note: jl_is_globalref(e) is not included here, since we care a little about about having a line number for it
+    return jl_is_ast_node(e);
+}
+
+static jl_value_t *jl_invoke_julia_macro(jl_array_t *args, jl_module_t *inmodule, jl_module_t **ctx, jl_value_t **lineinfo, size_t world, int throw_load_error)
 {
     jl_task_t *ct = jl_current_task;
     JL_TIMING(MACRO_INVOCATION, MACRO_INVOCATION);
@@ -1010,10 +1064,9 @@ static jl_value_t *jl_invoke_julia_macro(jl_array_t *args, jl_module_t *inmodule
     margs[0] = jl_array_ptr_ref(args, 0);
     // __source__ argument
     jl_value_t *lno = jl_array_ptr_ref(args, 1);
+    if (!jl_is_linenode(lno))
+        lno = jl_new_struct(jl_linenumbernode_type, jl_box_long(0), jl_nothing);
     margs[1] = lno;
-    if (!jl_typetagis(lno, jl_linenumbernode_type)) {
-        margs[1] = jl_new_struct(jl_linenumbernode_type, jl_box_long(0), jl_nothing);
-    }
     margs[2] = (jl_value_t*)inmodule;
     for (i = 3; i < nargs; i++)
         margs[i] = jl_array_ptr_ref(args, i - 1);
@@ -1052,6 +1105,7 @@ static jl_value_t *jl_invoke_julia_macro(jl_array_t *args, jl_module_t *inmodule
         }
     }
     ct->world_age = last_age;
+    *lineinfo = margs[1];
     JL_GC_POP();
     return result;
 }
@@ -1074,36 +1128,47 @@ static jl_value_t *jl_expand_macros(jl_value_t *expr, jl_module_t *inmodule, str
         JL_GC_POP();
         return expr;
     }
-    if (e->head == jl_hygienicscope_sym && jl_expr_nargs(e) == 2) {
+    if (e->head == jl_hygienicscope_sym && jl_expr_nargs(e) >= 2) {
         struct macroctx_stack newctx;
         newctx.m = (jl_module_t*)jl_exprarg(e, 1);
         JL_TYPECHK(hygienic-scope, module, (jl_value_t*)newctx.m);
         newctx.parent = macroctx;
         jl_value_t *a = jl_exprarg(e, 0);
         jl_value_t *a2 = jl_expand_macros(a, inmodule, &newctx, onelevel, world, throw_load_error);
-        if (a != a2)
+        if (jl_is_expr(a2) && ((jl_expr_t*)a2)->head == jl_escape_sym && !need_esc_node(jl_exprarg(a2, 0)))
+            expr = jl_exprarg(a2, 0);
+        else if (!need_esc_node(a2))
+            expr = a2;
+        else if (a != a2)
             jl_array_ptr_set(e->args, 0, a2);
         return expr;
     }
     if (e->head == jl_macrocall_sym) {
         struct macroctx_stack newctx;
         newctx.m = macroctx ? macroctx->m : inmodule;
         newctx.parent = macroctx;
-        jl_value_t *result = jl_invoke_julia_macro(e->args, inmodule, &newctx.m, world, throw_load_error);
+        jl_value_t *lineinfo = NULL;
+        jl_value_t *result = jl_invoke_julia_macro(e->args, inmodule, &newctx.m, &lineinfo, world, throw_load_error);
+        if (!need_esc_node(result))
+            return result;
         jl_value_t *wrap = NULL;
-        JL_GC_PUSH3(&result, &wrap, &newctx.m);
+        JL_GC_PUSH4(&result, &wrap, &newctx.m, &lineinfo);
         // copy and wrap the result in `(hygienic-scope ,result ,newctx)
         if (jl_is_expr(result) && ((jl_expr_t*)result)->head == jl_escape_sym)
             result = jl_exprarg(result, 0);
         else
-            wrap = (jl_value_t*)jl_exprn(jl_hygienicscope_sym, 2);
+            wrap = (jl_value_t*)jl_exprn(jl_hygienicscope_sym, 3);
         result = jl_copy_ast(result);
         if (!onelevel)
             result = jl_expand_macros(result, inmodule, wrap ? &newctx : macroctx, onelevel, world, throw_load_error);
-        if (wrap) {
+        if (wrap && need_esc_node(result)) {
             jl_exprargset(wrap, 0, result);
             jl_exprargset(wrap, 1, newctx.m);
-            result = wrap;
+            jl_exprargset(wrap, 2, lineinfo);
+            if (jl_is_expr(result) && ((jl_expr_t*)result)->head == jl_escape_sym)
+                result = jl_exprarg(result, 0);
+            else
+                result = wrap;
         }
         JL_GC_POP();
         return result;

src/jlfrontend.scm

@@ -175,7 +175,7 @@
 
 (define (jl-expand-macroscope expr)
   (error-wrap (lambda ()
-                (julia-expand-macroscope expr))))
+                (julia-expand-macroscope expr #t))))
 
 ;; construct default definitions of `eval` for non-bare modules
 ;; called by jl_eval_module_expr

src/macroexpand.scm

@@ -446,10 +446,12 @@
                         binds))
                  ,body)))
            ((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 #t)))
+             (let* ((parent-scope (cons (list env m) parent-scope))
+                    (body (cadr e))
+                    (m (caddr e))
+                    (lno  (cdddr e))
+                    (body (resolve-expansion-vars-with-new-env body env m parent-scope inarg #t)))
+               (if *keepmarkers* `(hygienic-scope (escape ,body) ,m ,@lno) body)))
            ((tuple)
             (cons (car e)
                   (map (lambda (x)
@@ -573,9 +575,11 @@
         ((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)))
+          (let* ((form (cadr e)) ;; form is the expression returned from expand-macros
+                 (modu (caddr e)) ;; m is the macro's def module
+                 (lno  (cdddr e)) ;; lno is (optionally) the line number node
+                 (form (resolve-expansion-vars form modu)))
+            (if *keepmarkers* `(hygienic-scope (escape ,form) ,modu ,@lno) form)))
         (else
          (map julia-expand-macroscopes- e))))
 
@@ -585,8 +589,9 @@
    ((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)))
+          (m (caddr e))
+          (lno (cdddr e)))
+      `(hygienic-scope ,(rename-symbolic-labels- (cadr e) (table) parent-scope) ,m ,@lno)))
    ((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))
@@ -603,17 +608,39 @@
 (define (rename-symbolic-labels e)
   (rename-symbolic-labels- e (table) '()))
 
+; find the first line number in this expression, before we eliminate them
+(define (first-lineno blk)
+  (cond ((not (pair? blk)) #f)
+        ((eq? (car blk) 'line) blk)
+        ((and (eq? (car blk) 'hygienic-scope) (pair? (cdddr blk)) (pair? (cadddr blk)) (eq? (car (cadddr blk)) 'line))
+         (cadddr blk))
+        ((memq (car blk) '(toplevel block escape hygienic-scope))
+           (let loop ((xs (cdr blk)))
+             (and (pair? xs)
+               (let ((elt (first-lineno (car xs))))
+                 (or elt (loop (cdr xs)))))))
+        (else #f)))
+
 ;; macro expander entry point
 
 ;; TODO: delete this file and fold this operation into resolve-scopes
-(define (julia-expand-macroscope e)
-  (julia-expand-macroscopes-
-   (rename-symbolic-labels
-    (julia-expand-quotes e))))
+(define (julia-expand-macroscope e (keepmarkers #f))
+  ; if we are expanding macro macroscopes too early, keep the hygiene markers around for later
+  ; they have line numbers attached to them, and we might need those again
+  ; if we are macro expanding as normal, then move the first line number we
+  ; find to the head position of the expression where we will not easily lose it later
+  (set! *keepmarkers* keepmarkers)
+  (let ((lno (and (not keepmarkers) (first-lineno e)))
+        (form (julia-expand-macroscopes-
+               (rename-symbolic-labels
+                (julia-expand-quotes e)))))
+        (if lno `(block ,lno ,form) form)))
 
 (define (contains-macrocall e)
   (and (pair? e)
        (contains (lambda (e) (and (pair? e) (eq? (car e) 'macrocall))) e)))
 
 (define (julia-bq-macro x)
   (julia-bq-expand x 0))
+
+(define *keepmarkers* #f)