macro hygine escape stripped late for macros calling macros

[c42f]

In julia 0.7, escaping with esc seems to be stripped out later than in julia 0.6.
This means that macros which are called by other macros need to have code for
dealing with input ASTs including Expr(:escape, ...). For example:

macro a(ex)
    # ex will be Expr(:escape, :(x=10)) on julia 0.7, but :(x=10) on 0.6.
    @show ex
    esc(ex)
end

macro b(ex)
    quote
        @a $(esc(ex)) # esc() required for julia 0.6.0
        #@a $ex       # Can be used in 0.7.0-DEV.1300
    end
end

@b x=10
x

Is this intentional? From a user perspective the 0.6 behavior made more sense
to me, as it allowed the inner macro to work with surface syntax not including
Expr(:escape, ...) nodes. It also meant that the outer macro would be required
to properly escape $ex in the example above.

[vtjnash]

Yes, incorrect placement of esc was required as a workaround for #22307

Most of the time, the simplest replacement is simply to move the esc outside:

esc(:( @a $ex ))

However, note that this also affects the scope resolution of @a too.

[yuyichao]

What is the correct way to combine expressions that needs to be resolved in caller scope and callee scope then?

[yuyichao]

i.e. the equivalent of

macro m(ex)
    quote
        a = 1
        @another_macro_in_callee_module a $(esc(ex)) global_in_callee_module
    end
end

[vtjnash]

If you know it must be a global, my current recommendation is to splice it in explicitly: global_in_callee_module = GlobalRef(@__MODULE__, :myglobal)

We also may need to update some macro implementations to be hygiene-annotation aware.

[yuyichao]

Seems to be a step backward since there's almost no way one can get things working without wrapping everything in a big esc if there's nested macros.....

And there's also the a. Is it possible to do that correctly without gensym? Doing gensym will mess up the error messages (UndefVarError and debug info) so it does not provide an equivalence.

[vtjnash]

manual hygiene (gensym(:a)) is definitely always a feasible option

[yuyichao]

That's why I mentioned why gensym isn't really a solution.

It also means that whenever one include a nested macro, they'll likely need to replace all local variables that is used in the macro with gensyms. That can be a pretty tricky work.

This doesn't seem like the behavior we want. We should (and previous did) make sure that at macro templates can be written as normal code apart from the part that needs to deal with user input. That's the lowest level of hygiene-by-default we can/should provide.

[vtjnash]

We should (and previous did) make sure that at macro templates

Let's not romanticize past bugs. In the past, we got macro hygiene wrong, but did so randomly and unpredictably. Hygiene doesn't (and can't) know whether to replace a symbol with a gensym or a globalref until after it has finished expanding all macros. Using esc is tricky work, but it was made exponentially worse in the past by not behaving predictably (ref your PRs on the topic as well).

[yuyichao]

Let's not romanticize past bugs

Well, in both cases there are correct way to write code that will give the correct result and the previous behavior was much easier to use. None of them should be the final solution but I don't think we should make writing correct macros much harder.

[yuyichao]

And so the predictable behavior now is that

1. Basically writing macro template like normal code never work
2. Due to 1, you basically always want to escape the whole result
3. Due to 2, you will get hygiene wrong by default
4. If you managed to overcome 3, you will not get good debug info

[ararslan]

This may be expected behavior, but the change introduced in #22985 has proven to be hugely breaking, so something about this should be addressed.

[StefanKarpinski]

@vtjnash, closing this with no comment is not ok. This situation is unpopular, severely breaking, and as far as I can see, you've made zero attempt to clarify what the new rules for macros are or help mitigate the massive fallout that we're seeing.

[c42f]

Thanks guys for taking this one seriously. I haven't had the time to understand the subtle differences between the three possible solutions (#10940 vs #22985 vs the previous hygiene situation in 0.6) so I can't comment in detail.

I would certainly like the rules for how hygiene interacts with scoping (in multiple macros vs multiple modules) described clearly somewhere. If that's not possible, perhaps it's a sign that the right solution hasn't yet arrived :-)

[ararslan]

I would certainly like the rules for how hygiene interacts with scoping [...] described clearly somewhere.

Yeah definitely. At the very least, the recent change should have documentation and/or NEWS so that people know how to write 0.7-compatible macros and can fix the large number of currently broken packages.

[JeffBezanson]

I think we're not done here yet and the macro system will continue to evolve before 1.0. Hopefully some form of #10940 will be merged, and then there will be a better and more complete story to tell.

[StefanKarpinski]

Do we leave it as-is for now in a state where so many things are broken with the story known to be incomplete so that people are actively discouraged from trying to fix it because they know it will change again? That seems less than ideal. I know master is unstable, but this is unusually bad and seems like work that might be better suited for a long-lived branch.

[vtjnash]

The two changes proposed are independent. The one already merged is a bugfix, while the proposed change (in #10940) is a significant behavioral shift. When #23247 is merged, that will address the accidental breakage (for example, with eval-poly).

[c42f]

For anyone who hits this and wants to try to work around the issue with escaping, I tried tracking the escaping depth inside expressions passed to FastClosures.@closure, and reproducing the depth when rewriting the expression. I'm not sure this was the right thing, but it seemed to work.

[tkoolen]

If there's no desire to change the current situation, could the hygiene section of the documentation at least be updated with an example of how to handle a case like #23221 (comment)? I still don't know what the correct way to handle such cases is.

[vtjnash]

You're apparently just using esc on everything to explicitly opt-out of macro hygiene and handle everything yourself. The correct way to use it is to call esc on all of the arguments to the macro.

[tkoolen]

In Parametron.jl, that's what I chose for now, yes. I know this is not correct. I also don't know how to do it correctly, while I did in 0.6.

The correct way to use it is to call esc on all of the arguments to the macro

No it isn't, apparently. In >=0.7 that doesn't work, because @lazyexpression is called from @objective and @constraint, so I'm in the same situation as #23221 (comment).

Note by the way that I'm not claiming that the version before the changes above was correctly escaping inputs on 0.6, but I tried doing what you proposed and ran into the current issue.

[tkoolen]

If you want proof that I grasp the basic concept of escaping, see e.g. https://github.com/tkoolen/LoopThrottle.jl/blob/master/src/LoopThrottle.jl. But again note that if I were to call @throttle from another macro I'd run into #23221 (comment).

[c42f]

@tkoolen you're not crazy; there's not been a satisfactory resolution to this yet. While it's great that Jameson has fixed the bugs in the old system, #23221 (comment) is still a good assessment of the usability problems.

[angeris]

Hey all! What's the current state of this issue? Currently there's some need for calling a macro within a macro such that it runs in the outer scope (as with @tkoolen, calling JuMP's @variable and @constraint to be run in the calling context/outer scope on some variables also defined in the outer scope).

Currently, the only solution @akshayka (CC) and I have found was to do something like the original approach, where we simply escape everything (which... doesn't really follow hygiene rules)

macro fancy_variable(m, v)
return quote
    _do_something($(esc(m)), $(quot(v)))
    $(esc(quote
        @variable($m, $v)
        @constraint($m, $v = 1)
    end))
end

Here, _do_something is not visible to the caller context, but should be run on the variables defined in the outer (caller) scope. We also need @variable and @constraint (really only the former, but besides the point) to be run on the caller context, since it will define variables in the caller context that the user may later interact with. Is this the correct approach? If not, what should this look like?

In particular, it would also be great to have some good documentation on how to "wrap" macros within other macros (as in the above case, for example), or to know the current state of affairs with the late escapes. I would be very happy to help write the documentation, but I'm afraid I'm not sure I fully understand hygiene and escaping well enough to give clear explanations. If anyone could also point me to some more examples, that would be fantastic (and, perhaps, after being a bit more confident, I could help write some docs ;).

[c42f]

What's the current state of this issue?

It's an unresolved problem: nested inner macros must be prepared to deal with Expr(:escape, ...) expressions arising from when the outer macro interpolates into an expression using esc and that's passed to the nested macro.

I don't think there's a clear solution in sight. The macro expander needs some information about the module scope of each Symbol in the AST so that it can look that symbol up in the correct module.

At the moment the expander gets access to this information by processing the nested Expr(:escape, ...) after all expansions. That's correct, but means nested user macros which inspect the expression must track and reproduce those nested escapes by hand which can be quite difficult. I think people rarely do this in practice.

The expander could in principle resolve symbols into GlobalRefs before the macros see them. I think this would also be correct, but has the downside that macros would see all Symbols as GlobalRefs rather than simple symbols. Presumably that would break an awful lot of things.

I believe there was a proposed solution from long ago (#6910) which did an end run around these constraints by hiding the module information in the address of the Symbols in ASTs created from macros. While this was very clever (no need for esc!?) I guess it breaks all sorts of assumptions the compiler is now making about Symbol uniqueness. Also, it's very magical.

[vtjnash]

The PR #6910 basically changes from requiring recursive macros from dealing with Expr(:escape) to renaming it Expr(:unescape). That has some nice properties, but is tangential to the issue at hand here. There's nothing to be done here now, as the current behavior is fairly set by v1.x compatibility (modulo implementation bugs where we miscompute the hygienic scope of some variable, which we should fix), though perhaps for v2.x we'll get around to finishing #6910 and cause everyone to need to update their code to replace :escape with :unescape in their newly corrected handling of recursion.

In short, the rule now for when you use esc should be very simple (and otherwise, it's likely a bug): anything passed into the function as an argument, should be wrapped in a call to esc before using it. Otherwise, don't. Unless you plan to handle hygiene yourself—since you have access to __module__ and @__MODULE__ and gensym, this is generally feasible, though often likely hard.

[c42f]

I don't think we should close this, unless we have a different issue tracking the usability problems of macros-calling-macros.

The problem is not that the current system is wrong (not exactly), but that it's very difficult for macro users to use correctly.

[vtjnash]

Having bugs and being difficult to learn aren't actionable (though certainly related). When you encounter specific bugs, please open issues for them.

[c42f]

anything passed into the function as an argument, should be wrapped in a call to esc before using it.

For simple interpolation that's fine, but ignores the case when you'd like to pattern match the argument to extract subexpressions.

The pattern matching must be aware that Expr(:escape) could occur anywhere. Anybody writing macros directly against the Expr API (by using the head field, etc) is going to miss this case.

Therefore, most macros can't be used in the AST generated by another macro.

This is the specific usability problem.

[vtjnash]

Yes, tooling like MacroTools.jl is generally recommended so that you aren't trying to deal with every case yourself.

[c42f]

MacroTools.jl doesn't seem to handle this. But I agree that pattern matching tooling with proper :escape support is one possible solution to this. We'd eventually need it to be a stdlib — without it, there's a large class of macros which will continue to be written incorrectly by default.

I think the problem is well defined and we haven't solved it. Therefore I don't know why we'd close this issue.

[tpapp]

I am also finding it difficult to understand why this was closed. Macro hygiene for the case @c42f mentioned above continues to be a tricky issue in Julia --- if this issue is considered too low level/mechanical for a discussion about that, another one should be opened.

[vtjnash]

Just because it is difficult to write a proper macro doesn't mean that anything is wrong, and it's not helpful to repeat that. There are definitely remaining bugs that need to to be fixed, but this issue has thus far not talked about them.

[ararslan]

Perhaps this should be considered a feature request then.

[c42f]

I've done my best to clearly restate this as a usability problem with the current system here: #37691