`__module__` not defined in macro defined by a macro

[mgkurtz]

Under both julia 1.9.3 and 1.10.0-beta2, I get

julia> macro macrobuilder()
           quote
               macro $(esc(:foo))()
                   display(__module__)
               end
           end
       end
@macrobuilder (macro with 1 method)

julia> @macrobuilder
@foo (macro with 1 method)

julia> @foo
ERROR: LoadError: UndefVarError: `__module__` not defined
Stacktrace:
 [1] var"@foo"(__source__::LineNumberNode, __module__::Module)
   @ Main ./REPL[1]:4
in expression starting at REPL[3]:1

The error in combination with its stacktrace suggest that something is weird here.

[mgkurtz]

My actual use case is that I have functions such as polynomial_ring(R::Ring, vars::Vector{Symbol}), free_group(vars::Vector{Symbol}) etc. and users who want something like

julia> Rx = @polynomial_ring R x[0:3]
polynomial_ring(R, [:x0, :x1, :x2, :x3])

julia> x0 + x1*x2
x0 + x1*x2

So, @polynomial_ring and its siblings need to evaluate 0:3 or whatever one gives there to create appropriate code. As this is intended for REPL use mostly, using Main.eval(…) should be fine, even Base.eval(Module(), …) works in most use cases. Still Base.eval(__module__, …) seemed more plausible to me. Only that __module__ is somehow not defined.

[vtjnash]

The macroexpand.scm code is ignorant of the special values injected into the :macro syntax. We can try to teach it about them, but this will fail because it cannot rename the hidden argument.

diff --git a/src/macroexpand.scm b/src/macroexpand.scm
index e0e809eee0..fec6919e9a 100644
--- a/src/macroexpand.scm
+++ b/src/macroexpand.scm
@@ -128,7 +128,7 @@

    ;; macro definition
    (pattern-lambda (macro (call name . argl) body)
-                   `(-> (tuple ,@argl) ,body))
+                   `(-> (tuple __module__ __source__ ,@argl) ,body))

    (pattern-lambda (try tryb var catchb finalb)
                    (if var (list 'varlist var) '()))

Instead, the whole expand-macro-def function would need to run first (or really, all of expand-forms should be run first, if we could). You can get around this bug with esc(:__module__) however (though that will break in the future when the bug is fixed).

[mgkurtz]

Thanks for the explanation and the workaround. I just noticed a similar need for an esc in

macro macrobuilder()
    quote
        macro $(esc(:foo))()
            :((; $(esc(:(:a=>1)))))
        end
    end
end

Using :((; :a=>1)) without esc, I get

ERROR: syntax: invalid named tuple element "Main.=>(:a, 1)"

and for a function call :(f(; :a=>1)) without the same extra esc, I get

ERROR: syntax: invalid keyword argument syntax "Main.=>(:a, 1)"

Is this related?

Note: My use case is to pass on keyword arguments, so that @polynomial_ring R x option=value calls polynomial_ring(R, :x; :option=value).

[vtjnash]

Yes, => is special syntax there in that place, but macro hygiene is unaware of that, so it tries to treat it as an actual call

[c42f]

or really all of expand-forms should be run first, if we could

This is the approach being taken over at https://github.com/c42f/JuliaLowering.jl - removing all the macro-related pattern matching stuff entirely by doing scope resolution of macro expanded symbols in lowering's normal scope resolution pass after syntax desugaring. It seems to work well and will entirely eliminate this class of bugs. (Eventually. Rewriting all of lowering will take some time :-) )

[vtjnash]

There were quite a few package that rely on macroexpand returning the wrong scope info when I last tried to make that change. I think we should add a legacy=true flag to that function as soon as possible (which enables the existing macroexpand.scm pass for legacy consumers), so we can start to help packages migrate away from relying on this bug. Then in a future release, we can stop using that pass entirely (in favor of explicitly tagging the scope of every symbol, as is my understanding is done in that package).

Refs: https://github.com/JuliaLang/julia/pull/49793 and https://github.com/JuliaLang/julia/compare/master...vtjnash:julia:jn/invert-hygiene (which implemented enough of it to start to look at how much breakage it would cause in the ecosystem due to badly written macros)

[c42f]

There were quite a few package that rely on macroexpand returning the wrong scope info when I last tried to make that change

Fun times :grimacing:

I don't care much about macroexpand() being broken because having "all the good new lowering things" is going to have to be opt-in anyway: By necessity, the new provenance system and macro scope improvements have an entirely new data structure to represent expressions (not Expr!!). So there's no way we can enable that by default - that would be Julia 2.0 material.

Instead, I plan to make it opt-in on a per-module level so we can actually get people using and migrating to it within some reasonable time frame. And to add enough bidirectional compatibility so that modules using the two systems can work together seamlessly. It's going to be tough integrating all this but I think it's the only way to make progress.