add macroexpand! function and add legacyscope kwarg (#59276)

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

Author: vtjnash

NEWS.md

@@ -60,6 +60,13 @@ New library features
 * `sort(keys(::Dict))` and `sort(values(::Dict))` now automatically collect, they previously threw ([#56978]).
 * `Base.AbstractOneTo` is added as a supertype of one-based axes, with `Base.OneTo` as its subtype ([#56902]).
 * `takestring!(::IOBuffer)` removes the content from the buffer, returning the content as a `String`.
+* The `macroexpand` (with default true) and the new `macroexpand!` (with default false)
+  functions now support a `legacyscope` boolean keyword argument to control whether to run
+  the legacy scope resolution pass over the result. The legacy scope resolution code has
+  known design bugs and will be disabled by default in a future version. Users should
+  migrate now by calling `legacyscope=false` or using `macroexpand!`. This may often require
+  fixes to the code calling `macroexpand` with `Meta.unescape` and `Meta.reescape` or by
+  updating tests to expect `hygienic-scope` or `escape` markers might appear in the result.
 
 Standard library changes
 ------------------------

base/exports.jl

@@ -837,6 +837,7 @@ export
     gensym,
     @kwdef,
     macroexpand,
+    macroexpand!,
     @macroexpand1,
     @macroexpand,
     parse,

base/expr.jl

@@ -174,11 +174,12 @@ function ==(x::DebugInfo, y::DebugInfo)
 end
 
 """
-    macroexpand(m::Module, x; recursive=true)
+    macroexpand(m::Module, x; recursive=true, legacyscope=true)
 
 Take the expression `x` and return an equivalent expression with all macros removed (expanded)
 for executing in module `m`.
 The `recursive` keyword controls whether deeper levels of nested macros are also expanded.
+The `legacyscope` keyword controls whether legacy macroscope expansion is performed.
 This is demonstrated in the example below:
 ```jldoctest; filter = r"#= .*:6 =#"
 julia> module M
@@ -198,12 +199,28 @@ julia> macroexpand(M, :(@m2()), recursive=false)
 :(#= REPL[1]:6 =# @m1)
 ```
 """
-function macroexpand(m::Module, @nospecialize(x); recursive=true)
-    if recursive
-        ccall(:jl_macroexpand, Any, (Any, Any), x, m)
-    else
-        ccall(:jl_macroexpand1, Any, (Any, Any), x, m)
-    end
+function macroexpand(m::Module, @nospecialize(x); recursive=true, legacyscope=true)
+    ccall(:jl_macroexpand, Any, (Any, Any, Cint, Cint, Cint), x, m, recursive, false, legacyscope)
+end
+
+"""
+    macroexpand!(m::Module, x; recursive=true, legacyscope=false)
+
+Take the expression `x` and return an equivalent expression with all macros removed (expanded)
+for executing in module `m`, modifying `x` in place without copying.
+The `recursive` keyword controls whether deeper levels of nested macros are also expanded.
+The `legacyscope` keyword controls whether legacy macroscope expansion is performed.
+
+This function performs macro expansion without the initial copy step, making it more efficient
+when the original expression is no longer needed. By default, macroscope expansion is disabled
+for in-place expansion as it can be called separately if needed.
+
+!!! warning
+    This function modifies the input expression `x` in place. Use `macroexpand` if you need
+    to preserve the original expression.
+"""
+function macroexpand!(m::Module, @nospecialize(x); recursive=true, legacyscope=false)
+    ccall(:jl_macroexpand, Any, (Any, Any, Cint, Cint, Cint), x, m, recursive, true, legacyscope)
 end
 
 """
@@ -250,10 +267,10 @@ With `macroexpand` the expression expands in the module given as the first argum
     The two-argument form requires at least Julia 1.11.
 """
 macro macroexpand(code)
-    return :(macroexpand($__module__, $(QuoteNode(code)), recursive=true))
+    return :(macroexpand($__module__, $(QuoteNode(code)); recursive=true, legacyscope=true))
 end
 macro macroexpand(mod, code)
-    return :(macroexpand($(esc(mod)), $(QuoteNode(code)), recursive=true))
+    return :(macroexpand($(esc(mod)), $(QuoteNode(code)); recursive=true, legacyscope=true))
 end
 
 """
@@ -262,10 +279,10 @@ end
 Non recursive version of [`@macroexpand`](@ref).
 """
 macro macroexpand1(code)
-    return :(macroexpand($__module__, $(QuoteNode(code)), recursive=false))
+    return :(macroexpand($__module__, $(QuoteNode(code)); recursive=false, legacyscope=true))
 end
 macro macroexpand1(mod, code)
-    return :(macroexpand($(esc(mod)), $(QuoteNode(code)), recursive=false))
+    return :(macroexpand($(esc(mod)), $(QuoteNode(code)); recursive=false, legacyscope=true))
 end
 
 ## misc syntax ##

doc/src/base/base.md

@@ -538,6 +538,7 @@ Meta.parse(::AbstractString)
 Meta.ParseError
 Core.QuoteNode
 Base.macroexpand
+Base.macroexpand!
 Base.@macroexpand
 Base.@macroexpand1
 Base.code_lowered

src/ast.c

@@ -1252,24 +1252,15 @@ static jl_value_t *jl_expand_macros(jl_value_t *expr, jl_module_t *inmodule, str
     return expr;
 }
 
-JL_DLLEXPORT jl_value_t *jl_macroexpand(jl_value_t *expr, jl_module_t *inmodule)
+JL_DLLEXPORT jl_value_t *jl_macroexpand(jl_value_t *expr, jl_module_t *inmodule, int recursive, int inplace, int expand_scope)
 {
     JL_TIMING(LOWERING, LOWERING);
     JL_GC_PUSH1(&expr);
-    expr = jl_copy_ast(expr);
-    expr = jl_expand_macros(expr, inmodule, NULL, 0, jl_atomic_load_acquire(&jl_world_counter), 0);
-    expr = jl_call_scm_on_ast("jl-expand-macroscope", expr, inmodule);
-    JL_GC_POP();
-    return expr;
-}
-
-JL_DLLEXPORT jl_value_t *jl_macroexpand1(jl_value_t *expr, jl_module_t *inmodule)
-{
-    JL_TIMING(LOWERING, LOWERING);
-    JL_GC_PUSH1(&expr);
-    expr = jl_copy_ast(expr);
-    expr = jl_expand_macros(expr, inmodule, NULL, 1, jl_atomic_load_acquire(&jl_world_counter), 0);
-    expr = jl_call_scm_on_ast("jl-expand-macroscope", expr, inmodule);
+    if (!inplace)
+        expr = jl_copy_ast(expr);
+    expr = jl_expand_macros(expr, inmodule, NULL, !recursive, jl_atomic_load_acquire(&jl_world_counter), 0);
+    if (expand_scope)
+        expr = jl_call_scm_on_ast("jl-expand-macroscope", expr, inmodule);
     JL_GC_POP();
     return expr;
 }

src/jl_exported_funcs.inc

@@ -294,7 +294,6 @@
     XX(jl_lseek) \
     XX(jl_lstat) \
     XX(jl_macroexpand) \
-    XX(jl_macroexpand1) \
     XX(jl_malloc) \
     XX(jl_malloc_stack) \
     XX(jl_matching_methods) \

test/syntax.jl

@@ -4555,3 +4555,50 @@ let d = Dict(:a=>1)
     foo(a::Int) = 2
     @test foo() == 1
 end
+
+# Test new macroexpand functionality - define test module at top level
+module MacroExpandTestModule
+    macro test_basic(x)
+        return :($x + 1)
+    end
+end
+
+@testset "hygienic-scope" begin
+    # Test macroexpand! (in-place expansion)
+    expr = :(MacroExpandTestModule.@test_basic(5))
+    result = macroexpand!(@__MODULE__, expr)
+    # macroexpand! returns a hygienic-scope wrapper with legacyscope=false (default)
+    @test Meta.isexpr(result, Symbol("hygienic-scope"))
+    @test result.args[1] == :(5 + 1)
+    @test result.args[2] === MacroExpandTestModule
+    @test result.args[3] isa Core.LineNumberNode
+
+    # Test legacyscope parameter
+    hygiene_expr = :(MacroExpandTestModule.@test_basic(100))
+
+    # With legacyscope=true (default for macroexpand)
+    expanded_with_scope = macroexpand(@__MODULE__, hygiene_expr; legacyscope=true)
+    @test expanded_with_scope == :($(GlobalRef(MacroExpandTestModule, :(+)))(100, 1))
+
+    # With legacyscope=false
+    expanded_no_scope = macroexpand(@__MODULE__, hygiene_expr; legacyscope=false)
+    @test Meta.isexpr(expanded_no_scope, Symbol("hygienic-scope"))
+    @test expanded_no_scope.args[1] == :(100 + 1)
+    @test expanded_no_scope.args[2] === MacroExpandTestModule
+    @test expanded_no_scope.args[3] isa Core.LineNumberNode
+
+    # Test macroexpand! with legacyscope=false (default for macroexpand!)
+    hygiene_copy = copy(hygiene_expr)
+    result_no_scope = macroexpand!(@__MODULE__, hygiene_copy; legacyscope=false)
+    @test Meta.isexpr(result_no_scope, Symbol("hygienic-scope"))
+    @test result_no_scope.args[1] == :(100 + 1)
+    @test result_no_scope.args[2] === MacroExpandTestModule
+    @test result_no_scope.args[3] isa Core.LineNumberNode
+end
+
+# Test error handling for malformed macro calls
+@testset "macroexpand error handling" begin
+    # Test with undefined macro
+    @test_throws UndefVarError macroexpand(@__MODULE__, :(@undefined_macro(x)))
+    @test_throws UndefVarError macroexpand!(@__MODULE__, :(@undefined_macro(x)))
+end