Skip to content

support old-style macros #21

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 2 commits into from
Closed

support old-style macros #21

wants to merge 2 commits into from

Conversation

@ericphanson
Copy link

@ericphanson ericphanson commented Jul 31, 2025

attempts to address a TODO in the code. Currently if a macro is lowered by non-JuliaLowering (e.g. Base lowering), that lowering generates a method with arguments (__source__::LineNumberNode, __module__::Module, args...), while macros lowered by JuliaLowering generate a method with arguments (ctx::MacroContext, args...). In ExplicitImports, I attempting to lower code using JuliaLowering which invokes macros from Base/elsewhere without re-lowering those macros. This fix seems to make that work.

Note: I developed this based on #10, but following #18 I pointed this PR at main. However I can only run JuliaLowering on #10 so while these changes are tested there, I don't know that they will work here. On #10 I checked my change/test is effective; with only the new test, I get

julia> @test JuliaLowering.include_string(test_mod, "@hi") == "hi"
Error During Test at REPL[35]:3
  Test threw exception
  Expression: JuliaLowering.include_string(test_mod, "@hi") == "hi"
  MacroExpansionError while expanding @hi in module Main.anonymous:
  @hi
  └─┘ ── Error expanding macro
  Stacktrace:
    [1] expand_macro(ctx::JuliaLowering.MacroExpansionContext{JuliaLowering.SyntaxGraph{@NamedTuple{__macro_ctx__::Dict{Int64, Nothing}, kind::Dict{Int64, JuliaSyntax.Kind}, value::Dict{Int64, Any}, var_id::Dict{Int64, Int64}, name_val::Dict{Int64, String}, syntax_flags::Dict{Int64, UInt16}, meta::Dict{Int64, Base.ImmutableDict{Symbol, Any}}, source::Dict{Int64, Union{Int64, LineNumberNode, Tuple, JuliaLowering.SourceRef}}, scope_layer::Dict{Int64, Int64}}}}, ex::SyntaxTree{JuliaLowering.SyntaxGraph{@NamedTuple{__macro_ctx__::Dict{Int64, Nothing}, kind::Dict{Int64, JuliaSyntax.Kind}, value::Dict{Int64, Any}, var_id::Dict{Int64, Int64}, name_val::Dict{Int64, String}, syntax_flags::Dict{Int64, UInt16}, meta::Dict{Int64, Base.ImmutableDict{Symbol, Any}}, source::Dict{Int64, Union{Int64, LineNumberNode, Tuple, JuliaLowering.SourceRef}}, scope_layer::Dict{Int64, Int64}}}})
      @ JuliaLowering ~/JuliaLowering.jl/src/macro_expansion.jl:156
    [2] expand_forms_1(ctx::JuliaLowering.MacroExpansionContext{JuliaLowering.SyntaxGraph{@NamedTuple{__macro_ctx__::Dict{Int64, Nothing}, kind::Dict{Int64, JuliaSyntax.Kind}, value::Dict{Int64, Any}, var_id::Dict{Int64, Int64}, name_val::Dict{Int64, String}, syntax_flags::Dict{Int64, UInt16}, meta::Dict{Int64, Base.ImmutableDict{Symbol, Any}}, source::Dict{Int64, Union{Int64, LineNumberNode, Tuple, JuliaLowering.SourceRef}}, scope_layer::Dict{Int64, Int64}}}}, ex::SyntaxTree{JuliaLowering.SyntaxGraph{@NamedTuple{__macro_ctx__::Dict{Int64, Nothing}, kind::Dict{Int64, JuliaSyntax.Kind}, value::Dict{Int64, Any}, var_id::Dict{Int64, Int64}, name_val::Dict{Int64, String}, syntax_flags::Dict{Int64, UInt16}, meta::Dict{Int64, Base.ImmutableDict{Symbol, Any}}, source::Dict{Int64, Union{Int64, LineNumberNode, Tuple, JuliaLowering.SourceRef}}, scope_layer::Dict{Int64, Int64}}}})
      @ JuliaLowering ~/JuliaLowering.jl/src/macro_expansion.jl:253
    [3] expand_forms_1(mod::Module, ex::SyntaxTree{JuliaLowering.SyntaxGraph{Dict{Symbol, Any}}})
      @ JuliaLowering ~/JuliaLowering.jl/src/macro_expansion.jl:332
    [4] lower(mod::Module, ex0::SyntaxTree{JuliaLowering.SyntaxGraph{Dict{Symbol, Any}}})
      @ JuliaLowering ~/JuliaLowering.jl/src/eval.jl:2
    [5] eval(mod::Module, ex::SyntaxTree{JuliaLowering.SyntaxGraph{Dict{Symbol, Any}}})
      @ JuliaLowering ~/JuliaLowering.jl/src/eval.jl:342
    [6] eval(mod::Module, ex::SyntaxTree{JuliaLowering.SyntaxGraph{Dict{Symbol, Any}}})
      @ JuliaLowering ~/JuliaLowering.jl/src/eval.jl:338
    [7] include_string
      @ ~/JuliaLowering.jl/src/eval.jl:381 [inlined]
    [8] include_string(mod::Module, code::String)
      @ JuliaLowering ~/JuliaLowering.jl/src/eval.jl:381
    [9] top-level scope
      @ REPL[35]:3
   [10] macro expansion
      @ ~/.julia/juliaup/julia-nightly/share/julia/stdlib/v1.13/Test/src/Test.jl:738 [inlined]
  caused by: MethodError: no method matching var"@hi"(::JuliaLowering.MacroContext)
  The function `@hi` exists, but no method is defined for this combination of argument types.
  Closest candidates are:
    var"@hi"(::LineNumberNode, ::Module)
     @ Main.anonymous REPL[34]:2
  Stacktrace:
    [1] expand_macro(ctx::JuliaLowering.MacroExpansionContext{JuliaLowering.SyntaxGraph{@NamedTuple{__macro_ctx__::Dict{Int64, Nothing}, kind::Dict{Int64, JuliaSyntax.Kind}, value::Dict{Int64, Any}, var_id::Dict{Int64, Int64}, name_val::Dict{Int64, String}, syntax_flags::Dict{Int64, UInt16}, meta::Dict{Int64, Base.ImmutableDict{Symbol, Any}}, source::Dict{Int64, Union{Int64, LineNumberNode, Tuple, JuliaLowering.SourceRef}}, scope_layer::Dict{Int64, Int64}}}}, ex::SyntaxTree{JuliaLowering.SyntaxGraph{@NamedTuple{__macro_ctx__::Dict{Int64, Nothing}, kind::Dict{Int64, JuliaSyntax.Kind}, value::Dict{Int64, Any}, var_id::Dict{Int64, Int64}, name_val::Dict{Int64, String}, syntax_flags::Dict{Int64, UInt16}, meta::Dict{Int64, Base.ImmutableDict{Symbol, Any}}, source::Dict{Int64, Union{Int64, LineNumberNode, Tuple, JuliaLowering.SourceRef}}, scope_layer::Dict{Int64, Int64}}}})
      @ JuliaLowering ~/JuliaLowering.jl/src/macro_expansion.jl:149
    [2] expand_forms_1(ctx::JuliaLowering.MacroExpansionContext{JuliaLowering.SyntaxGraph{@NamedTuple{__macro_ctx__::Dict{Int64, Nothing}, kind::Dict{Int64, JuliaSyntax.Kind}, value::Dict{Int64, Any}, var_id::Dict{Int64, Int64}, name_val::Dict{Int64, String}, syntax_flags::Dict{Int64, UInt16}, meta::Dict{Int64, Base.ImmutableDict{Symbol, Any}}, source::Dict{Int64, Union{Int64, LineNumberNode, Tuple, JuliaLowering.SourceRef}}, scope_layer::Dict{Int64, Int64}}}}, ex::SyntaxTree{JuliaLowering.SyntaxGraph{@NamedTuple{__macro_ctx__::Dict{Int64, Nothing}, kind::Dict{Int64, JuliaSyntax.Kind}, value::Dict{Int64, Any}, var_id::Dict{Int64, Int64}, name_val::Dict{Int64, String}, syntax_flags::Dict{Int64, UInt16}, meta::Dict{Int64, Base.ImmutableDict{Symbol, Any}}, source::Dict{Int64, Union{Int64, LineNumberNode, Tuple, JuliaLowering.SourceRef}}, scope_layer::Dict{Int64, Int64}}}})
      @ JuliaLowering ~/JuliaLowering.jl/src/macro_expansion.jl:253
    [3] expand_forms_1(mod::Module, ex::SyntaxTree{JuliaLowering.SyntaxGraph{Dict{Symbol, Any}}})
      @ JuliaLowering ~/JuliaLowering.jl/src/macro_expansion.jl:332
    [4] lower(mod::Module, ex0::SyntaxTree{JuliaLowering.SyntaxGraph{Dict{Symbol, Any}}})
      @ JuliaLowering ~/JuliaLowering.jl/src/eval.jl:2
    [5] eval(mod::Module, ex::SyntaxTree{JuliaLowering.SyntaxGraph{Dict{Symbol, Any}}})
      @ JuliaLowering ~/JuliaLowering.jl/src/eval.jl:342
    [6] eval(mod::Module, ex::SyntaxTree{JuliaLowering.SyntaxGraph{Dict{Symbol, Any}}})
      @ JuliaLowering ~/JuliaLowering.jl/src/eval.jl:338
    [7] include_string
      @ ~/JuliaLowering.jl/src/eval.jl:381 [inlined]
    [8] include_string(mod::Module, code::String)
      @ JuliaLowering ~/JuliaLowering.jl/src/eval.jl:381
    [9] top-level scope
      @ REPL[35]:3
   [10] macro expansion
      @ ~/.julia/juliaup/julia-nightly/share/julia/stdlib/v1.13/Test/src/Test.jl:738 [inlined]
ERROR: There was an error during testing

while the code change fixes it:

julia> @test JuliaLowering.include_string(test_mod, "@hi") == "hi"
Test Passed

ericphanson
ericphanson commented Jul 31, 2025
View reviewed changes
@c42f
Copy link
Member

c42f commented Jul 31, 2025

Ah, I have already implemented this in the caf/Expr-macrocall-compat branch but I didn't put in a PR yet 😅

I wonder if we came to the same conclusions...

c42f
c42f reviewed Jul 31, 2025
View reviewed changes
src/macro_expansion.jl
line, _ = source_location(macname)
file = filename(macname)
line_number_node = Base.LineNumberNode(line, file)
invokelatest(macfunc, line_number_node, ctx.current_layer.mod, args...)
Copy link
Member

@c42f c42f Jul 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah right. So this approach doesn't work if the old macro returns an AST, because that AST will be Expr but the macro expander (and subsequent lowering) is expecting a SyntaxTree.

In that case we need full bidirectional Expr <--> SyntaxTree support. And also need a solution for handling hygiene which is represented rather differently in the two cases. See my WIP branch for how to do those :-)

@ericphanson
Copy link
Author

ericphanson commented Jul 31, 2025

Ah, it did seem “too easy” 😅. That makes sense.

@c42f
Copy link
Member

c42f commented Jul 31, 2025

The full version is definitely a bit more fiddly :-)

Thanks for the PR though! The applicable() test is basically how I go about it in the WIP version I've got sitting around

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@c42f c42f c42f left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@ericphanson @c42f