Macro expansion for old-style Expr macros

[c42f]

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.

TODO:

• Proper tests
• Needs a rebase after Low-provenance Expr->SyntaxTree conversion #22 merges
• Support for the hygienic-scope form?
• Consider whether we should also guard calling old-style macros with hasmethod() for better error messages (what if the user calls a new-style macro with the incorrect number of arguments? How do we make it so that they get a comprehensible error message in that case?)
• Preserve scope layer stack in some form? Needed for JETLS - could do separately though.
• Refine K"deferred_toplevel_eval"

[c42f]

In fixing the tests I've encountered issues with @cfunction hygiene which were obvious at the time that macro was written, but are made more obvious by the fact that Expr conversion now includes some hygiene information.

To make this systematic, I've introduced the new form K"deferred_toplevel_eval" here, but I think the semantics needs some refinement over the current implementation. Possibly this should be split into its own PR. To quote the commit message:

---

A few special forms have a kind of "deferred top level evaluation"
semantics for some of their children:

• @cfunction - the function name and types
• ccall / foreigncall - the type arguments
• @generated - the expression defining the generated function stub

For example, in @ccall f()::Int, the Int means "the symbol Int as
looked up in global scope in the module", and should fail if Int
refers to a local variable. Currently all three of these cases are
handled through different mechanisms with varying levels of hygiene
inconsistency and ability to warn about access to local variables.

To fix this problem, introduce the new K"deferred_toplevel_eval" form
which wraps an expression and preserves it as a piece of AST in the
output (rather than producing IR), but still resolves scope and hygiene.

[c42f]

The deferred toplevel eval stuff is split out into #36 now, which can merge before this.

[mlechu]

Consider whether we should also guard calling old-style macros with hasmethod() for better error messages (what if the user calls a new-style macro with the incorrect number of arguments? How do we make it so that they get a comprehensible error message in that case?)

In the case where neither new nor old matching macro-methods exist, we still get a MethodError. In the case where an old macro exists without a new one, I think it would be fair to assume that a new macro and an old macro available in the same module defined with the same name and number of (explicit) arguments should have the same functionality. We currently enforce this by being the only ones to define new-style macros; as long as we don't make it easy to define a mix of both, there shouldn't be a case where we accidentally call an old macro where we wanted a new one.

[mlechu]

Preserve scope layer stack in some form?

I'd be in favour of bringing back the current_layer macro context field and adding a field to ScopeLayer that is just a pointer to the parent scope, or maybe just the parent ID. This would quietly keep the information around and makes the hygienic-scope/escape cases slightly nicer (something like the following)

    elseif k == K"escape"
        @chk numchildren(ex) >= 1 "`escape` requires an argument"
        if isnothing(ctx.current_layer.parent)
            throw(LoweringError(ex, "`escape` node in outer context"))
        end
        outer = ctx.current_layer.parent
        newctx = MacroExpansionContext(ctx.graph, ctx.bindings, ctx.scope_layers, outer)
        return expand_forms_1(newctx, ex[1])
    elseif k == K"hygienic_scope"
        @chk numchildren(ex) >= 2 && ex[2].value isa Module "`hygienic_scope` requires an AST and a module"
        new_layer = ScopeLayer(length(ctx.scope_layers)+1, ex[2].value, ctx.current_layer, true) # param 3 is parent scope
        push!(ctx.scope_layers, new_layer)
        newctx = MacroExpansionContext(ctx.graph, ctx.bindings, ctx.scope_layers, new_layer)
        return expand_forms_1(newctx, ex[1])

[mlechu]

A small fix you may want to include in this PR is a special case for macros returning Expr(:toplevel, ...) which was added in JuliaLang/julia#53515

I implemented this for Expr here: mlechu@80ba20a. Feel free to take that change and anything else you want from that branch (only some of it is JETLS hacking; other parts I intend to upstream eventually)

[c42f]

Thanks for the hygenic scope support and extra tests! I thought testing was a bit light but tbh I couldn't think of extra cases to cover. By the way if you find it more convenient to suggest big chunks of code as a commit I'm happy to cherry pick from any branch of yours which is based on this one - just lmk which commit to pull. Looks like I have minor things to fix with the new code you added there so I'll fix those up.

I'll fiddle with current_layer so that it preserves the info you want (I don't have a good feel for what you need downstream yet or exactly how that's used so the specific advice is good).

After that, I think we're good to go with this once I've sorted out the static parameter stuff in the other PR this depends on.

[c42f]

I put a bit of extra effort into attempting something sensible with the MethodError situation. The current rule is that if any new-style method is detected, the method error referd to new-style macro arguments.

I really don't love using hasmethod() for the dispatch machinery here but I'm not exactly sure what else to do at this early stage of the JuliaLowering integration work.

In the future we could, perhaps, emit metadata from lowering which marks a macro as purely new-style or purely old-style and use that for dispatch rather than hasmethod() (perhaps including a way to override this to ease some porting situations like we're in right now with extending things like Base.@ccall before Base is able to depend on any of the new machinery)

[aviatesk]

I really don't love using hasmethod() for the dispatch machinery here but I'm not exactly sure what else to do at this early stage of the JuliaLowering integration work.

Yea, it's true that using hasmethod sounds to be hacky, but it seems like there aren't any other good alternatives.
I also considered other approaches using overlay method tables, but honestly I think they would just add complexity and make things even more confusing than using hasmethod.