macroexpand: stop pre-running the hygiene pass

This prepares us to delete the separate hygiene pass, and make it part
of lowering resolve-scopes. This affects a few macros which expect to
run macroexpand then mangle the result more, and the result of those
macros is not already wrapped in `esc()`. The damage seems relatively
minor however, and generally a good improvement.

Author: vtjnash

doc/src/base/reflection.md

@@ -87,7 +87,7 @@ be passed instead!). For example:
 
 ```jldoctest; setup = :(using InteractiveUtils)
 julia> InteractiveUtils.macroexpand(@__MODULE__, :(@edit println("")) )
-:(InteractiveUtils.edit(println, (Base.typesof)("")))
+:($(Expr(Symbol("hygienic-scope"), :(edit($(Expr(:escape, :println)), (Base.typesof)($(Expr(:escape, ""))))), InteractiveUtils)))
 ```
 
 The functions `Base.Meta.show_sexpr` and [`dump`](@ref) are used to display S-expr style views

doc/src/manual/metaprogramming.md

@@ -535,20 +535,21 @@ this is an extremely useful tool for debugging macros):
 
 ```julia-repl sayhello2
 julia> ex = macroexpand(Main, :(@sayhello("human")) )
-:(Main.println("Hello, ", "human"))
+:($(Expr(Symbol("hygienic-scope"), :(println("Hello, ", "human")), Main)))
 
 julia> typeof(ex)
 Expr
 ```
 
-We can see that the `"human"` literal has been interpolated into the expression.
+We can see that the macro expression has been replaced by a `hygienic-scope` expression in
+its place and the `"human"` literal has been interpolated into the expression.
 
 There also exists a macro [`@macroexpand`](@ref) that is perhaps a bit more convenient than the `macroexpand` function:
 
 
 ```jldoctest sayhello2
 julia> @macroexpand @sayhello "human"
-:(println("Hello, ", "human"))
+:($(Expr(Symbol("hygienic-scope"), :(println("Hello, ", "human")), Main)))
 ```
 
 ### Hold up: why macros?
@@ -579,7 +580,7 @@ julia> typeof(ex)
 Expr
 
 julia> ex
-:(println("I execute at runtime. The argument is: ", $(Expr(:copyast, :($(QuoteNode(:((1, 2, 3)))))))))
+:($(Expr(Symbol("hygienic-scope"), :(println("I execute at runtime. The argument is: ", $(Expr(:copyast, :($(QuoteNode(:((1, 2, 3))))))))), Main)))
 
 julia> eval(ex)
 I execute at runtime. The argument is: (1, 2, 3)
@@ -728,18 +729,18 @@ of a macro expansion with the aptly named [`@macroexpand`](@ref) macro:
 
 ```julia-repl assert2
 julia> @macroexpand @assert a == b
-:(if Main.a == Main.b
-        Main.nothing
-    else
-        Main.throw(Main.AssertionError("a == b"))
-    end)
+:($(Expr(Symbol("hygienic-scope"), :(if $(Expr(:escape, :(a == b)))
+      nothing
+  else
+      throw(AssertionError("a == b"))
+  end), Base)))
 
 julia> @macroexpand @assert a==b "a should equal b!"
-:(if Main.a == Main.b
-        Main.nothing
-    else
-        Main.throw(Main.AssertionError("a should equal b!"))
-    end)
+:($(Expr(Symbol("hygienic-scope"), :(if $(Expr(:escape, :(a == b)))
+      nothing
+  else
+      throw(AssertionError("a should equal b!"))
+  end), Base)))
 ```
 
 There is yet another case that the actual `@assert` macro handles: what if, in addition to printing
@@ -777,15 +778,22 @@ of expressions inside the macro body.
 ### Hygiene
 
 An issue that arises in more complex macros is that of [hygiene](https://en.wikipedia.org/wiki/Hygienic_macro).
-In short, macros must ensure that the variables they introduce in their returned expressions do
-not accidentally clash with existing variables in the surrounding code they expand into. Conversely,
-the expressions that are passed into a macro as arguments are often *expected* to evaluate in
-the context of the surrounding code, interacting with and modifying the existing variables. Another
-concern arises from the fact that a macro may be called in a different module from where it was
-defined. In this case we need to ensure that all global variables are resolved to the correct
-module. Julia already has a major advantage over languages with textual macro expansion (like
-C) in that it only needs to consider the returned expression. All the other variables (such as
-`msg` in `@assert` above) follow the [normal scoping block behavior](@ref scope-of-variables).
+We saw this a bit earlier with the hygienic-scope expression in the result of macroexpand
+that demarcated where our expression came from as a result of interpolation of the expanded
+macro into the rest of the expression.
+
+This already gives Julia a major advantage over languages with textual macro expansion (like
+C) in that it only needs to consider the returned expression, and it cannot change the
+parsing of the expression surrounding it. Thus, all the other variables (such as `msg` in
+`@assert` above) follow the [normal scoping block behavior](@ref scope-of-variables).
+
+Hygiene markers exists because macros must ensure that the variables they introduce in their
+returned expressions do not accidentally clash with existing variables in the surrounding
+code they expand into. Conversely, the expressions that are passed into a macro as arguments
+are often *expected* to evaluate in the context of the surrounding code, interacting with
+and modifying the existing variables. Another concern arises from the fact that a macro may
+be called in a different module from where it was defined. In this case we need to ensure
+that all global variables are resolved to the correct module.
 
 To demonstrate these issues, let us consider writing a `@time` macro that takes an expression
 as its argument, records the time, evaluates the expression, records the time again, prints the

src/ast.c

@@ -1255,7 +1255,6 @@ JL_DLLEXPORT jl_value_t *jl_macroexpand(jl_value_t *expr, jl_module_t *inmodule)
     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;
 }
@@ -1266,7 +1265,6 @@ JL_DLLEXPORT jl_value_t *jl_macroexpand1(jl_value_t *expr, jl_module_t *inmodule
     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);
     JL_GC_POP();
     return expr;
 }

src/jlfrontend.scm

@@ -194,10 +194,6 @@
 (define (jl-expand-to-thunk-stmt expr file line)
   (expand-to-thunk-stmt- expr file line))
 
-(define (jl-expand-macroscope expr)
-  (error-wrap (lambda ()
-                (julia-expand-macroscope expr))))
-
 ;; construct default definitions of `eval` for non-bare modules
 ;; called by jl_eval_module_expr
 (define (module-default-defs name file line)

test/syntax.jl

@@ -1557,7 +1557,7 @@ let ex = Meta.parse("@test27521(2) do y; y; end")
     fex = Expr(:(->), Expr(:tuple, :y), Expr(:block, LineNumberNode(1,:none), :y))
     @test ex == Expr(:do, Expr(:macrocall, Symbol("@test27521"), LineNumberNode(1,:none), 2),
                      fex)
-    @test macroexpand(@__MODULE__, ex) == Expr(:tuple, fex, 2)
+    @test macroexpand(@__MODULE__, ex).args[1] == Expr(:tuple, esc(fex), 2)
 end
 
 # issue #43018
@@ -3530,110 +3530,4 @@ end
 @test eval(:(if false
              elseif false || (()->true)()
                  42
-             end)) == 42
-
-macro _macroexpand(x, m=__module__)
-    :($__source__; macroexpand($m, Expr(:var"hygienic-scope", $(esc(Expr(:quote, x))), $m)))
-end
-
-@testset "unescaping in :global expressions" begin
-    m = @__MODULE__
-    @test @_macroexpand(global x::T) == :(global x::$(GlobalRef(m, :T)))
-    @test @_macroexpand(global (x, $(esc(:y)))) == :(global (x, y))
-    @test @_macroexpand(global (x::S, $(esc(:y))::$(esc(:T)))) ==
-        :(global (x::$(GlobalRef(m, :S)), y::T))
-    @test @_macroexpand(global (; x, $(esc(:y)))) == :(global (; x, y))
-    @test @_macroexpand(global (; x::S, $(esc(:y))::$(esc(:T)))) ==
-        :(global (; x::$(GlobalRef(m, :S)), y::T))
-
-    @test @_macroexpand(global x::T = a) == :(global x::$(GlobalRef(m, :T)) = $(GlobalRef(m, :a)))
-    @test @_macroexpand(global (x, $(esc(:y))) = a) == :(global (x, y) = $(GlobalRef(m, :a)))
-    @test @_macroexpand(global (x::S, $(esc(:y))::$(esc(:T))) = a) ==
-        :(global (x::$(GlobalRef(m, :S)), y::T) = $(GlobalRef(m, :a)))
-    @test @_macroexpand(global (; x, $(esc(:y))) = a) == :(global (; x, y) = $(GlobalRef(m, :a)))
-    @test @_macroexpand(global (; x::S, $(esc(:y))::$(esc(:T))) = a) ==
-        :(global (; x::$(GlobalRef(m, :S)), y::T) = $(GlobalRef(m, :a)))
-end
-
-# issue #49920
-let line1 = (quote end).args[1],
-    line2 = (quote end).args[1],
-    line3 = (quote end).args[1]
-    @test 1 === eval(Meta.lower(Main, Expr(:block, line1, 1, line2, line3)))
-end
-
-# issue #49984
-macro z49984(s); :(let a; $(esc(s)); end); end
-@test let a = 1; @z49984(a) === 1; end
-
-# issues #37783, #39929, #42552, #43379, and #48332
-let x = 1 => 2
-    @test_throws ErrorException @eval a => b = 2
-    @test_throws "function Base.=> must be explicitly imported to be extended" @eval a => b = 2
-end
-
-# Splatting in non-final default value (Ref #50518)
-for expr in (quote
-    function g1(a=(1,2)..., b...=3)
-        b
-    end
-end,quote
-    function g2(a=(1,2)..., b=3, c=4)
-        (b, c)
-    end
-end,quote
-    function g3(a=(1,2)..., b=3, c...=4)
-        (b, c)
-    end
-end)
-    let exc = try eval(expr); catch exc; exc end
-        @test isa(exc, ErrorException)
-        @test startswith(exc.msg, "syntax: invalid \"...\" in non-final positional argument default value")
-    end
-end
-
-# Test that bad lowering does not segfault (ref #50518)
-@test_throws ErrorException("syntax: Attempted to use slot marked unused") @eval function funused50518(::Float64)
-    $(Symbol("#unused#"))
-end
-
-@testset "public keyword" begin
-    p(str) = Base.remove_linenums!(Meta.parse(str))
-    # tests ported from JuliaSyntax.jl
-    @test p("function f(public)\n    public + 3\nend") == Expr(:function, Expr(:call, :f, :public), Expr(:block, Expr(:call, :+, :public, 3)))
-    @test p("public A, B") == Expr(:public, :A, :B)
-    @test p("if true \n public *= 4 \n end") == Expr(:if, true, Expr(:block, Expr(:*=, :public, 4)))
-    @test p("module Mod\n public A, B \n end") == Expr(:module, true, :Mod, Expr(:block, Expr(:public, :A, :B)))
-    @test p("module Mod2\n a = 3; b = 6; public a, b\n end") == Expr(:module, true, :Mod2, Expr(:block, Expr(:(=), :a, 3), Expr(:(=), :b, 6), Expr(:public, :a, :b)))
-    @test p("a = 3; b = 6; public a, b") == Expr(:toplevel, Expr(:(=), :a, 3), Expr(:(=), :b, 6), Expr(:public, :a, :b))
-    @test_throws Meta.ParseError p("begin \n public A, B \n end")
-    @test_throws Meta.ParseError p("if true \n public A, B \n end")
-    @test_throws Meta.ParseError p("public export=true foo, bar")
-    @test_throws Meta.ParseError p("public experimental=true foo, bar")
-    @test p("public(x::String) = false") == Expr(:(=), Expr(:call, :public, Expr(:(::), :x, :String)), Expr(:block, false))
-    @test p("module M; export @a; end") == Expr(:module, true, :M, Expr(:block, Expr(:export, :var"@a")))
-    @test p("module M; public @a; end") == Expr(:module, true, :M, Expr(:block, Expr(:public, :var"@a")))
-    @test p("module M; export ⤈; end") == Expr(:module, true, :M, Expr(:block, Expr(:export, :⤈)))
-    @test p("module M; public ⤈; end") == Expr(:module, true, :M, Expr(:block, Expr(:public, :⤈)))
-    @test p("public = 4") == Expr(:(=), :public, 4)
-    @test p("public[7] = 5") == Expr(:(=), Expr(:ref, :public, 7), 5)
-    @test p("public() = 6") == Expr(:(=), Expr(:call, :public), Expr(:block, 6))
-end
-
-@testset "removing argument sideeffects" begin
-    # Allow let blocks in broadcasted LHSes, but only evaluate them once:
-    execs = 0
-    array = [1]
-    let x = array; execs += 1; x; end .+= 2
-    @test array == [3]
-    @test execs == 1
-    let; execs += 1; array; end .= 4
-    @test array == [4]
-    @test execs == 2
-    let x = array; execs += 1; x; end::Vector{Int} .+= 2
-    @test array == [6]
-    @test execs == 3
-    let; execs += 1; array; end::Vector{Int} .= 7
-    @test array == [7]
-    @test execs == 4
-end
+             end)) == 42