Macro expansion for old-style Expr macros

c42f · c42f · commit c5ed1b5c7e6c · 2025-08-18T20:25:37.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,65 @@ discussed in Adams' paper:
 
 TODO: Write more here...
 
+
+### Compatibility with `Expr` macros
+
+In order to have compatibility with old-style macros which expect an `Expr`-based
+data structure as input, we convert `SyntaxTree` to `Expr`, call the old-style
+macro, then convert `SyntaxTree` back to `Expr` and continue with the expansion
+process. This involves some loss of provenance precision but allows full
+interoperability in the package ecosystem without a need to make breaking
+changes.
+
+Let's look at an example. 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/compat.jl b/src/compat.jl
@@ -24,7 +24,8 @@ function expr_to_syntaxtree(@nospecialize(e), lnn::Union{LineNumberNode, Nothing
         SyntaxGraph(),
         kind=Kind, syntax_flags=UInt16,
         source=SourceAttrType, var_id=Int, value=Any,
-        name_val=String, is_toplevel_thunk=Bool)
+        name_val=String, is_toplevel_thunk=Bool,
+        scope_layer=LayerId)
     expr_to_syntaxtree(graph, e, lnn)
 end
 
@@ -423,6 +424,13 @@ function _insert_convert_expr(@nospecialize(e), graph::SyntaxGraph, src::SourceA
             @assert nargs === 1
             child_exprs[1] = Expr(:quoted_symbol, e.args[1])
         end
+    elseif e.head === :scope_layer 
+        @assert nargs === 2
+        @assert e.args[1] isa Symbol
+        @assert e.args[2] isa LayerId
+        st_id, src = _insert_convert_expr(e.args[1], graph, src)
+        setattr!(graph, st_id, scope_layer=e.args[2])
+        return st_id, src
     elseif e.head === :symbolicgoto || e.head === :symboliclabel
         @assert nargs === 1
         st_k = e.head === :symbolicgoto ? K"symbolic_label" : K"symbolic_goto"
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
@@ -6,6 +6,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)
@@ -130,7 +133,7 @@ function eval_macro_name(ctx::MacroExpansionContext, mctx::MacroContext, ex::Syn
     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)
     try
         eval(mod, expr_form)
@@ -139,54 +142,123 @@ function eval_macro_name(ctx::MacroExpansionContext, mctx::MacroContext, ex::Syn
     end
 end
 
-function expand_macro(ctx::MacroExpansionContext, ex::SyntaxTree)
+# 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]
-    mctx = MacroContext(ctx.graph, ex, ctx.current_layer)
+    mctx = MacroContext(ctx.graph, ex, current_layer(ctx))
     macfunc = eval_macro_name(ctx, mctx, 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.
-    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
-    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.
-            newexc = MacroExpansionError(mctx, exc.ex, exc.msg, exc.position, exc.err)
+    raw_args = ex[2:end]
+    # We use a specific well defined world age for the next checks and macro
+    # expansion invocations. This avoids inconsistencies if the latest world
+    # age changes concurrently.
+    # 
+    # TODO: Allow this to be passed in
+    macro_world = Base.get_world_counter()
+    if hasmethod(macfunc, Tuple{typeof(mctx), typeof.(raw_args)...}; world=macro_world)
+        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
+            Base.invoke_in_world(macro_world, macfunc, macro_args...)
+        catch exc
+            newexc = exc isa MacroExpansionError ?
+                MacroExpansionError(mctx, exc.ex, exc.msg, exc.position, exc.err) :
+                MacroExpansionError(mctx, ex, "Error expanding macro", :all, exc)
+            # TODO: We can delete this rethrow when we move to AST-based error propagation.
+            rethrow(newexc)
+        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
-            newexc = MacroExpansionError(mctx, ex, "Error expanding macro", :all, exc)
+            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_loc = source_location(LineNumberNode, ex)
+        macro_args = Any[macro_loc, 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 slight 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
+        # TODO: Error handling
+        expanded = try
+            Base.invoke_in_world(macro_world, macfunc, macro_args...)
+        catch exc
+            rethrow(MacroExpansionError(mctx, ex, "Error expanding macro", :all, exc))
         end
-        # TODO: We can delete this rethrow when we move to AST-based error propagation.
-        rethrow(newexc)
+        expanded = expr_to_syntaxtree(ctx, expanded, macro_loc)
     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_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
@@ -231,18 +303,24 @@ function expand_forms_1(ctx::MacroExpansionContext, ex::SyntaxTree)
             # turned into normal bindings (eg, assigned to)
             @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)
@@ -330,7 +408,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)
@@ -344,12 +422,12 @@ 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/runtime.jl b/src/runtime.jl
@@ -307,7 +307,7 @@ function (g::GeneratedFunctionStub)(world::UInt, source::Method, @nospecialize a
 
     # Macro expansion
     layers = ScopeLayer[ScopeLayer(1, mod, false)]
-    ctx1 = MacroExpansionContext(graph, Bindings(), layers, layers[1])
+    ctx1 = MacroExpansionContext(graph, Bindings(), layers, LayerId[1])
 
     # Run code generator - this acts like a macro expander and like a macro
     # expander it gets a MacroContext.
@@ -326,11 +326,11 @@ function (g::GeneratedFunctionStub)(world::UInt, source::Method, @nospecialize a
     # Expand any macros emitted by the generator
     ex1 = expand_forms_1(ctx1, reparent(ctx1, ex0))
     ctx1 = MacroExpansionContext(delete_attributes(graph, :__macro_ctx__),
-                                 ctx1.bindings, ctx1.scope_layers, ctx1.current_layer)
+                                 ctx1.bindings, ctx1.scope_layers, LayerId[])
     ex1 = reparent(ctx1, ex1)
 
     # Desugaring
-    ctx2, ex2 = expand_forms_2(  ctx1, ex1)
+    ctx2, ex2 = expand_forms_2(ctx1, ex1)
 
     # Wrap expansion in a non-toplevel lambda and run scope resolution
     ex2 = @ast ctx2 ex0 [K"lambda"(is_toplevel_thunk=false)
@@ -342,12 +342,11 @@ function (g::GeneratedFunctionStub)(world::UInt, source::Method, @nospecialize a
         ]
         ex2
     ]
-    ctx3, ex3 = resolve_scopes(  ctx2, ex2)
-
+    ctx3, ex3 = resolve_scopes(ctx2, ex2)
 
     # Rest of lowering
     ctx4, ex4 = convert_closures(ctx3, ex3)
-    ctx5, ex5 = linearize_ir(    ctx4, ex4)
+    ctx5, ex5 = linearize_ir(ctx4, ex4)
     ci = to_lowered_expr(mod, ex5)
     @assert ci isa Core.CodeInfo
     return ci
diff --git a/src/syntax_graph.jl b/src/syntax_graph.jl
diff --git a/test/demo.jl b/test/demo.jl
diff --git a/test/macros.jl b/test/macros.jl
