Macro expansion for old-style Expr macros

c42f · c42f · commit cbfa88e00645 · 2025-07-17T18:45:14.000+10:00
The main difficulty here is managing hygiene correctly. We choose to
represent new-style scoped identifiers passed to old macros using
`Expr(:scope_layer, name, layer_id)` where necessary. But only where
necessary - in most contexts, old-style macros will see unadorned
identifiers just as they currently do. The only time the new `Expr`
construct is visible is when new macros interpolate an expression into a
call to an old-style macro in the returned code. Previously, such
macro-calling-macro situations would result in the inner macro call
seeing `Expr(:escape, ...)` and it's rare for old-style macros to handle
this correctly.

Old-style macros may still return `Expr(:escape)` expressions resulting
from manual escaping. When consuming the output of old macros, we
process these manual escapes by escaping up the macro expansion stack in
the same way we currently do.
diff --git a/README.md b/README.md
@@ -288,6 +288,57 @@ discussed in Adams' paper:
 
 TODO: Write more here...
 
+
+### Compatibility with `Expr` macros
+
+Suppose a manually escaped old-style macro `@oldstyle` is implemented as
+
+```julia
+macro oldstyle(a, b)
+    quote
+        x = "x in @oldstyle"
+        @newstyle $(esc(a)) $(esc(b)) x
+    end
+end
+```
+
+along with two correctly escaped new-style macros:
+
+```julia
+macro call_oldstyle_macro(y)
+    quote
+        x = "x in call_oldstyle_macro"
+        @oldstyle $y x
+    end
+end
+
+macro newstyle(x, y, z)
+    quote
+        x = "x in @newstyle"
+        ($x, $y, $z, x)
+    end
+end
+```
+
+Then want some code like the following to "just work" with respect to hygiene
+
+```julia
+let
+    x = "x in outer ctx"
+    @call_oldstyle_macro x
+end
+```
+
+When calling `@oldstyle`, we must convert `SyntaxTree` into `Expr`, but we need
+to preserve the scope layer of the `x` from the outer context as it is passed
+into `@oldstyle` as a macro argument. To do this, we use `Expr(:scope_layer,
+:x, outer_layer_id)`. (In the old system, this would be `Expr(:escape, :x)`
+instead, presuming that `@call_oldstyle_macro` was implemented using `esc()`.)
+
+When receiving output from old style macro invocations, we preserve the escape
+handling of the existing system for any symbols which aren't tagged with a
+scope layer.
+
 ## Pass 2: Syntax desugaring
 
 This pass recursively converts many special surface syntax forms to a smaller
diff --git a/src/desugaring.jl b/src/desugaring.jl
@@ -15,7 +15,7 @@ function DesugaringContext(ctx)
                               scope_type=Symbol, # :hard or :soft
                               var_id=IdTag,
                               is_toplevel_thunk=Bool)
-    DesugaringContext(graph, ctx.bindings, ctx.scope_layers, ctx.current_layer.mod)
+    DesugaringContext(graph, ctx.bindings, ctx.scope_layers, first(ctx.scope_layers).mod)
 end
 
 #-------------------------------------------------------------------------------
diff --git a/src/kinds.jl b/src/kinds.jl
@@ -7,6 +7,9 @@ function _register_kinds()
         # expansion, and known to lowering. These are part of the AST API but
         # without having surface syntax.
         "BEGIN_EXTENSION_KINDS"
+            # Used for converting `esc()`'d expressions arising from old macro
+            # invocations during macro expansion
+            "escape"
             # atomic fields or accesses (see `@atomic`)
             "atomic"
             # Flag for @generated parts of a functon
diff --git a/src/macro_expansion.jl b/src/macro_expansion.jl
@@ -18,9 +18,12 @@ struct MacroExpansionContext{GraphType} <: AbstractLoweringContext
     graph::GraphType
     bindings::Bindings
     scope_layers::Vector{ScopeLayer}
-    current_layer::ScopeLayer
+    scope_layer_stack::Vector{LayerId}
 end
 
+current_layer(ctx::MacroExpansionContext) = ctx.scope_layers[last(ctx.scope_layer_stack)]
+current_layer_id(ctx::MacroExpansionContext) = last(ctx.scope_layer_stack)
+
 #--------------------------------------------------
 # Expansion of quoted expressions
 function collect_unquoted!(ctx, unquoted, ex, depth)
@@ -122,58 +125,119 @@ function eval_macro_name(ctx, ex)
     ctx3, ex3 = resolve_scopes(ctx2, ex2)
     ctx4, ex4 = convert_closures(ctx3, ex3)
     ctx5, ex5 = linearize_ir(ctx4, ex4)
-    mod = ctx.current_layer.mod
+    mod = current_layer(ctx).mod
     expr_form = to_lowered_expr(mod, ex5)
     eval(mod, expr_form)
 end
 
+# Record scope layer information for symbols passed to a macro by setting
+# scope_layer for each expression and also processing any K"escape" arising
+# from previous expansion of old-style macros.
+#
+# See also set_scope_layer()
+function set_macro_arg_hygiene(ctx, ex, layer_ids, layer_idx)
+    k = kind(ex)
+    scope_layer = get(ex, :scope_layer, layer_ids[layer_idx])
+    if k == K"module" || k == K"toplevel" || k == K"inert"
+        makenode(ctx, ex, ex, children(ex);
+                 scope_layer=scope_layer)
+    elseif k == K"."
+        makenode(ctx, ex, ex, set_macro_arg_hygiene(ctx, ex[1], layer_ids, layer_idx), ex[2],
+                 scope_layer=scope_layer)
+    elseif !is_leaf(ex)
+        inner_layer_idx = layer_idx
+        if k == K"escape"
+            inner_layer_idx = layer_idx - 1
+            if inner_layer_idx < 1
+                # If we encounter too many escape nodes, there's probably been
+                # an error in the previous macro expansion.
+                # todo: The error here isn't precise about that - maybe we
+                # should record that macro call expression with the scope layer
+                # if we want to report the error against the macro call?
+                throw(MacroExpansionError(ex, "`escape` node in outer context"))
+            end
+        end
+        mapchildren(e->set_macro_arg_hygiene(ctx, e, layer_ids, inner_layer_idx),
+                    ctx, ex; scope_layer=scope_layer)
+    else
+        makeleaf(ctx, ex, ex; scope_layer=scope_layer)
+    end
+end
+
 function expand_macro(ctx, ex)
     @assert kind(ex) == K"macrocall"
 
     macname = ex[1]
     macfunc = eval_macro_name(ctx, macname)
-    # Macro call arguments may be either
-    # * Unprocessed by the macro expansion pass
-    # * Previously processed, but spliced into a further macro call emitted by
-    #   a macro expansion.
-    # In either case, we need to set any unset scope layers before passing the
-    # arguments to the macro call.
-    mctx = MacroContext(ctx.graph, ex, ctx.current_layer)
-    macro_args = Any[mctx]
-    for i in 2:numchildren(ex)
-        push!(macro_args, set_scope_layer(ctx, ex[i], ctx.current_layer.id, false))
-    end
+    mctx = MacroContext(ctx.graph, ex, current_layer(ctx))
+    raw_args = ex[2:end]
     macro_invocation_world = Base.get_world_counter()
-    expanded = try
-        # TODO: Allow invoking old-style macros for compat
-        invokelatest(macfunc, macro_args...)
-    catch exc
-        if exc isa MacroExpansionError
-            # Add context to the error.
-            # TODO: Using rethrow() is kinda ugh. Is there a way to avoid it?
-            rethrow(MacroExpansionError(mctx, exc.ex, exc.msg, exc.position))
+    if hasmethod(macfunc, Tuple{typeof(mctx), typeof.(raw_args)...}; world=Base.get_world_counter())
+        macro_args = Any[mctx]
+        for arg in raw_args
+            # Add hygiene information to be carried along with macro arguments.
+            #
+            # Macro call arguments may be either
+            # * Unprocessed by the macro expansion pass
+            # * Previously processed, but spliced into a further macro call emitted by
+            #   a macro expansion.
+            # In either case, we need to set scope layers before passing the
+            # arguments to the macro call.
+            push!(macro_args, set_macro_arg_hygiene(ctx, arg, ctx.scope_layer_stack,
+                                                    length(ctx.scope_layer_stack)))
+        end
+        expanded = try
+            invokelatest(macfunc, macro_args...)
+        catch exc
+            if exc isa MacroExpansionError
+                # Add context to the error.
+                # TODO: Using rethrow() is kinda ugh. Is there a way to avoid it?
+                rethrow(MacroExpansionError(mctx, exc.ex, exc.msg, exc.position))
+            else
+                throw(MacroExpansionError(mctx, ex, "Error expanding macro", :all))
+            end
+        end
+        if expanded isa SyntaxTree
+            if !is_compatible_graph(ctx, expanded)
+                # If the macro has produced syntax outside the macro context,
+                # copy it over. TODO: Do we expect this always to happen?  What
+                # is the API for access to the macro expansion context?
+                expanded = copy_ast(ctx, expanded)
+            end
         else
-            throw(MacroExpansionError(mctx, ex, "Error expanding macro", :all))
+            expanded = @ast ctx ex expanded::K"Value"
         end
+    else
+        # Compat: attempt to invoke an old-style macro if there's no applicable
+        # method for new-style macro arguments.
+        macro_args = Any[source_location(LineNumberNode, ex), current_layer(ctx).mod]
+        for arg in raw_args
+            # For hygiene in old-style macros, we omit any additional scope
+            # layer information from macro arguments. Old-style macros will
+            # handle that using manual escaping in the macro itself.
+            #
+            # Note that there's one somewhat-incompatibility here for
+            # identifiers interpolated into the `raw_args` from outer macro
+            # expansions of new-style macros which call old-style macros.
+            # Instead of seeing `Expr(:escape)` in such situations, old-style
+            # macros will now see `Expr(:scope_layer)` inside `macro_args`.
+            push!(macro_args, Expr(arg))
+        end
+        expanded = invokelatest(macfunc, macro_args...)
+        expanded = expr_to_SyntaxTree(syntax_graph(ctx), expanded)
     end
 
-    if expanded isa SyntaxTree
-        if !is_compatible_graph(ctx, expanded)
-            # If the macro has produced syntax outside the macro context, copy it over.
-            # TODO: Do we expect this always to happen?  What is the API for access
-            # to the macro expansion context?
-            expanded = copy_ast(ctx, expanded)
-        end
+    if kind(expanded) != K"Value"
         expanded = append_sourceref(ctx, expanded, ex)
         # Module scope for the returned AST is the module where this particular
         # method was defined (may be different from `parentmodule(macfunc)`)
-        mod_for_ast = lookup_method_instance(macfunc, macro_args, macro_invocation_world).def.module
+        mod_for_ast = lookup_method_instance(macfunc, macro_args,
+                                             macro_invocation_world).def.module
         new_layer = ScopeLayer(length(ctx.scope_layers)+1, mod_for_ast, true)
         push!(ctx.scope_layers, new_layer)
-        inner_ctx = MacroExpansionContext(ctx.graph, ctx.bindings, ctx.scope_layers, new_layer)
-        expanded = expand_forms_1(inner_ctx, expanded)
-    else
-        expanded = @ast ctx ex expanded::K"Value"
+        push!(ctx.scope_layer_stack, new_layer.id)
+        expanded = expand_forms_1(ctx, expanded)
+        pop!(ctx.scope_layer_stack)
     end
     return expanded
 end
@@ -215,18 +279,24 @@ function expand_forms_1(ctx::MacroExpansionContext, ex::SyntaxTree)
         elseif is_ccall_or_cglobal(name_str)
             @ast ctx ex name_str::K"core"
         else
-            layerid = get(ex, :scope_layer, ctx.current_layer.id)
+            layerid = get(ex, :scope_layer, current_layer_id(ctx))
             makeleaf(ctx, ex, ex, kind=K"Identifier", scope_layer=layerid)
         end
     elseif k == K"Identifier" || k == K"MacroName" || k == K"StringMacroName"
-        layerid = get(ex, :scope_layer, ctx.current_layer.id)
+        layerid = get(ex, :scope_layer, current_layer_id(ctx))
         makeleaf(ctx, ex, ex, kind=K"Identifier", scope_layer=layerid)
     elseif k == K"var" || k == K"char" || k == K"parens"
         # Strip "container" nodes
         @chk numchildren(ex) == 1
         expand_forms_1(ctx, ex[1])
+    elseif k == K"escape"
+        # For processing of old-style macros
+        top_layer = pop!(ctx.scope_layer_stack)
+        escaped_ex = expand_forms_1(ctx, ex[1])
+        push!(ctx.scope_layer_stack, top_layer)
+        escaped_ex
     elseif k == K"juxtapose"
-        layerid = get(ex, :scope_layer, ctx.current_layer.id)
+        layerid = get(ex, :scope_layer, current_layer_id(ctx))
         @chk numchildren(ex) == 2
         @ast ctx ex [K"call"
             "*"::K"Identifier"(scope_layer=layerid)
@@ -311,7 +381,7 @@ function expand_forms_1(ctx::MacroExpansionContext, ex::SyntaxTree)
     elseif k == K"<:" || k == K">:" || k == K"-->"
         # TODO: Should every form get layerid systematically? Or only the ones
         # which expand_forms_2 needs?
-        layerid = get(ex, :scope_layer, ctx.current_layer.id)
+        layerid = get(ex, :scope_layer, current_layer_id(ctx))
         mapchildren(e->expand_forms_1(ctx,e), ctx, ex; scope_layer=layerid)
     else
         mapchildren(e->expand_forms_1(ctx,e), ctx, ex)
@@ -325,13 +395,13 @@ function expand_forms_1(mod::Module, ex::SyntaxTree)
                               __macro_ctx__=Nothing,
                               meta=CompileHints)
     layers = ScopeLayer[ScopeLayer(1, mod, false)]
-    ctx = MacroExpansionContext(graph, Bindings(), layers, layers[1])
+    ctx = MacroExpansionContext(graph, Bindings(), layers, LayerId[1])
     ex2 = expand_forms_1(ctx, reparent(ctx, ex))
     graph2 = delete_attributes(graph, :__macro_ctx__)
     # TODO: Returning the context with pass-specific mutable data is a bad way
-    # to carry state into the next pass.
-    ctx2 = MacroExpansionContext(graph2, ctx.bindings, ctx.scope_layers,
-                                 ctx.current_layer)
+    # to carry state into the next pass. We might fix this by attaching such
+    # data to the graph itself as global attributes?
+    ctx2 = MacroExpansionContext(graph2, ctx.bindings, ctx.scope_layers, LayerId[])
     return ctx2, reparent(ctx2, ex2)
 end
 
diff --git a/src/syntax_graph.jl b/src/syntax_graph.jl
@@ -422,7 +422,7 @@ attrsummary(name, value::Number) = "$name=$value"
 function _value_string(ex)
     k = kind(ex)
     str = k == K"Identifier" || k == K"MacroName" || is_operator(k) ? ex.name_val :
-          k == K"Placeholder" ? ex.name_val :
+          k == K"Placeholder" ? ex.name_val           :
           k == K"SSAValue"    ? "%"                   :
           k == K"BindingId"   ? "#"                   :
           k == K"label"       ? "label"               :
@@ -540,7 +540,12 @@ JuliaSyntax.byte_range(ex::SyntaxTree) = byte_range(sourceref(ex))
 function JuliaSyntax._expr_leaf_val(ex::SyntaxTree)
     name = get(ex, :name_val, nothing)
     if !isnothing(name)
-        Symbol(name)
+        n = Symbol(name)
+        if hasattr(ex, :scope_layer)
+            Expr(:scope_layer, n, ex.scope_layer)
+        else
+            n
+        end
     else
         ex.value
     end
diff --git a/test/demo.jl b/test/demo.jl
@@ -43,7 +43,7 @@ end
 
 #-------------------------------------------------------------------------------
 # Module containing macros used in the demo.
-define_macros = false
+define_macros = true
 if !define_macros
     eval(:(module M end))
 else
@@ -95,6 +95,27 @@ eval(JuliaLowering.@SyntaxTree :(baremodule M
         end
     end
 
+    macro call_show(x)
+        quote
+            z = "z in @call_show"
+            @show z $x
+        end
+    end
+
+    macro call_oldstyle_macro(y)
+        quote
+            x = "x in call_oldstyle_macro"
+            @oldstyle $y x
+        end
+    end
+
+    macro newstyle(x, y, z)
+        quote
+            x = "x in @newstyle"
+            ($x, $y, $z, x)
+        end
+    end
+
     macro set_a_global(val)
         quote
             global a_global = $val
@@ -182,6 +203,16 @@ eval(JuliaLowering.@SyntaxTree :(baremodule M
 
 end))
 end
+
+Base.eval(M, :(
+macro oldstyle(a, b)
+    quote
+        x = "x in @oldstyle"
+        @newstyle $(esc(a)) $(esc(b)) x
+    end
+end
+))
+
 #
 #-------------------------------------------------------------------------------
 # Demos of the prototype
@@ -794,7 +825,17 @@ end
 """
 
 src = """
-cglobal(:jl_uv_stdin, Ptr{Cvoid})
+let
+    z = "z in outer ctx"
+    @call_show z
+end
+"""
+
+src = """
+let
+    x = "x in outer ctx"
+    @call_oldstyle_macro x
+end
 """
 
 ex = parsestmt(SyntaxTree, src, filename="foo.jl")
