add macroexpand! function and add legacyscope kwarg

[vtjnash]

Setting legacyscope=false is intended to make it much easier to debug and test macro expansion, since it no longer runs a buggy symbol mangling pass automatically. Adding the mutating version (macroexpand!) is mainly a handy way to opt in to the new legacyscope=true, without needing to spell that out.

More background: the macroexpand.scm pass design is buggy, so we'd like to stop using in the future. Currently changing the default causes visible breakage to a lot of buggy packages tests, so for now just provide the option to skip the legacy scope resolution. This is a continuation of #49793 and a prerequisite for eventually replacing the flisp code with JuliaLowering (once we can deprecate this parameter).

Implement in-place macro expansion with macroexpand! (no corresponding @macroexpand!) that avoids copying AST nodes when the original expression is no longer needed anyways. But more importantly, add a legacyscope::Bool keyword argument to the functions that allows opting out of the legacy scope mangling.

Changes:

• Consolidate jl_macroexpand C functions with added parameters for recursive, inplace, and the (legacy) expand_scope control.
• Add macroexpand! Julia function with legacyscope=false default.
• Update macroexpand to have legacyscope (default true) for backward compatibility, until v2 or earlier.

Added to backporting so that new code can start to be written with legacyscope=false. Not entirely a new feature, since this is just adding the ability to disable an old (long deprecated) feature.

🤖 Generated with Claude

[oscardssmith]

How does this interact with the incoming JuliaLowering changes?

[mlechu]

I'm not sure we need the legacyscope parameter. The current macro compatibility experiments in JuliaLowering bypass macroexpand.scm and jl_expand_macros by just calling the macro-methods directly, determining if there's an applicable legacy macro by checking the signature. Most of the macro-related ast.c code is rewritten in JuliaLowering (existing code takes Expr), but it's not that much logic.

[JeffBezanson]

If this needs to add macroexpand! we can't technically backport it. Is this really needed that badly?

[vtjnash]

I'm not sure we need the legacyscope parameter. The current macro compatibility experiments in JuliaLowering bypass macroexpand.scm and jl_expand_macros by just calling the macro-methods directly

This has nothing to do with macro compatibility, but with many users that rely on the buggy symbol mangling pass behavior of macroexpand. Most of the PkgEval results are gone now, but some of the summary of the failure last time I tried to delete macroexpand.scm is still captured in #49793

I also think it might be a mistake for JuliaLowering to contain that code, since technically the macroexpand step has about as much to do with lowering as they do with parsing or inference. All of those are sequential stages that produce or accept input from a different stage, but that doesn't mean those stages should be entangled.

@JeffBezanson I did not say this needs to add macroexpand!, but that it is here as a helper to make it easier to convert previously buggy uses of macroexpand into both more efficient and more correct uses within macros.

[JeffBezanson]

Adding the function is ok, but ideally we wouldn't add the export until the next version.

Also I think it's confusing for macroexpand and macroexpand! to have different default values for an argument. When mutating and non-mutating versions of a function exist I tend to assume they are the same except for mutation.

[mlechu]

I see; I thought deleting the call to the symbol mangler (and "prerequisite") meant that JuliaLowering was supposed to be using the code and handling hygiene on its own. This change makes sense to make, though I'm still confused about where the hygienic-scopes are handled—shouldn't those be consumed in resolve-scopes?

users that rely on the buggy symbol mangling pass behavior

Let me know if you run a new pkgeval identifying them, as I'll need to figure this out soon.

I also think it might be a mistake for JuliaLowering to contain that code, since technically the macroexpand step has about as much to do with lowering as they do with parsing or inference. All of those are sequential stages that produce or accept input from a different stage, but that doesn't mean those stages should be entangled.

Expanding macros is a distinct step, but assuming all that code is going to end up in the julia repository at some point, I don't see any issue. (Assuming not, I see several much more painful ones.)

[vtjnash]

though I'm still confused about where the hygienic-scopes are handled—shouldn't those be consumed in resolve-scopes

They should, yes, but currently they are handled by a pre-processor (macroexpand.scm) which tries to guess (badly) what resolve-scopes would do with them.

So the question before merging is what the current default should be for the macroexpand! legacy scope parameter. Then easy to run a PkgEval again with the legacyscope default switched.

[vtjnash]

Alright, let's try this, then address any post-merge review feedback later in the release cycle