Rewrite macros to use automatic hygiene

Manual `gensym` code often contains a lot of mistakes, either because the
user uses something like `.` or `+`, or because it combines code from
multiple modules (gensym is only unique within a pre-compile unit). This
replaces most uses for macro local variables with proper scope markers.
I did not rewrite `_lift_one_interp_helper` or `replace_ref_begin_end_`
however, since, while possible by adding `esc` to every argument that
has not used a gensym value, if any other argument did is a value, I
worried that could lead to macroexpand.scm making more new mistakes so I
left if for a separate PR. Better yet, we could make a unhygienic-scope
and unescape pair for marking the inverse/dual of the usual operations
(marking a symbol as unescaped within a region of unhygienic (escaped)
code to make these various uses easier to implement.

But rewrite `replace_ref_begin_end_` to respect argument order and
evaluation count (similar to its julia-syntax.scm counterpart) and
scoping (not adding `let` unpredictably).

Author: vtjnash

base/boot.jl

@@ -746,7 +746,7 @@ end
 
 # module providing the IR object model
 # excluding types already exported by Core (GlobalRef, QuoteNode, Expr, LineNumberNode)
-# any type beyond these is self-quoting (see also Base.is_ast_node)
+# any type beyond these is self-quoting (see also Base.isa_ast_node)
 module IR
 
 export CodeInfo, MethodInstance, CodeInstance, GotoNode, GotoIfNot, ReturnNode,

base/cartesian.jl

@@ -36,15 +36,14 @@ If you want just a post-expression, supply [`nothing`](@ref) for the pre-express
 parentheses and semicolons, you can supply multi-statement expressions.
 """
 macro nloops(N, itersym, rangeexpr, args...)
-    _nloops(N, itersym, rangeexpr, args...)
+    _nloops(N, itersym, true, rangeexpr, args...)
 end
 
-function _nloops(N::Int, itersym::Symbol, arraysym::Symbol, args::Expr...)
-    @gensym d
-    _nloops(N, itersym, :($d->Base.axes($arraysym, $d)), args...)
+function _nloops(N::Int, itersym::Symbol, esc_rng::Bool, arraysym::Symbol, args::Expr...)
+    _nloops(N, itersym, false, :(d->axes($(esc(arraysym)), d)), args...)
 end
 
-function _nloops(N::Int, itersym::Symbol, rangeexpr::Expr, args::Expr...)
+function _nloops(N::Int, itersym::Symbol, esc_rng::Bool, rangeexpr::Expr, args::Expr...)
     if rangeexpr.head !== :->
         throw(ArgumentError("second argument must be an anonymous function expression to compute the range"))
     end
@@ -55,11 +54,13 @@ function _nloops(N::Int, itersym::Symbol, rangeexpr::Expr, args::Expr...)
     ex = Expr(:escape, body)
     for dim = 1:N
         itervar = inlineanonymous(itersym, dim)
+        itervar = esc(itervar)
         rng = inlineanonymous(rangeexpr, dim)
+        esc_rng && (rng = esc(rng))
         preexpr = length(args) > 1 ? inlineanonymous(args[1], dim) : (:(nothing))
         postexpr = length(args) > 2 ? inlineanonymous(args[2], dim) : (:(nothing))
         ex = quote
-            for $(esc(itervar)) = $(esc(rng))
+            for $itervar = $rng
                 $(esc(preexpr))
                 $ex
                 $(esc(postexpr))

base/experimental.jl

@@ -41,7 +41,7 @@ within this scope, even if the compiler can't prove this to be the case.
     Experimental API. Subject to change without deprecation.
 """
 macro aliasscope(body)
-    sym = gensym()
+    sym = :aliasscope_result
     quote
         $(Expr(:aliasscope))
         $sym = $(esc(body))

base/meta.jl

@@ -628,6 +628,21 @@ function unescape(@nospecialize ex)
     return ex
 end
 
+"""
+    Meta.reescape(unescaped_expr, original_expr)
+
+Re-wrap `unescaped_expr` with the same level of escaping as `original_expr` had.
+This is the inverse operation of [`unescape`](@ref) - if the original expression
+was escaped, the unescaped expression is wrapped in `:escape` again.
+"""
+function reescape(@nospecialize(unescaped_expr), @nospecialize(original_expr))
+    if isexpr(original_expr, :escape) || isexpr(original_expr, :var"hygienic-scope")
+        return reescape(Expr(:escape, unescaped_expr), original_expr.args[1])
+    else
+        return unescaped_expr
+    end
+end
+
 """
     Meta.uncurly(expr)
 

base/reflection.jl

@@ -1305,19 +1305,19 @@ macro invoke(ex)
     f, args, kwargs = destructure_callex(topmod, ex)
     types = Expr(:curly, :Tuple)
     out = Expr(:call, GlobalRef(Core, :invoke))
-    isempty(kwargs) || push!(out.args, Expr(:parameters, kwargs...))
-    push!(out.args, f)
+    isempty(kwargs) || push!(out.args, Expr(:parameters, Any[esc(kw) for kw in kwargs]...))
+    push!(out.args, esc(f))
     push!(out.args, types)
     for arg in args
         if isexpr(arg, :(::))
-            push!(out.args, arg.args[1])
-            push!(types.args, arg.args[2])
+            push!(out.args, esc(arg.args[1]))
+            push!(types.args, esc(arg.args[2]))
         else
-            push!(out.args, arg)
-            push!(types.args, Expr(:call, GlobalRef(Core, :Typeof), arg))
+            push!(out.args, esc(arg))
+            push!(types.args, Expr(:call, GlobalRef(Core, :Typeof), esc(arg)))
         end
     end
-    return esc(out)
+    return out
 end
 
 getglobalref(gr::GlobalRef, world::UInt) = ccall(:jl_eval_globalref, Any, (Any, UInt), gr, world)
@@ -1367,42 +1367,42 @@ macro invokelatest(ex)
 
     if !isa(f, GlobalRef)
         out_f = Expr(:call, GlobalRef(Base, :invokelatest))
-        isempty(kwargs) || push!(out_f.args, Expr(:parameters, kwargs...))
+        isempty(kwargs) || push!(out_f.args, Expr(:parameters, Any[esc(kw) for kw in kwargs]...))
 
         if isexpr(f, :(.))
-            s = gensym()
+            s = :s
             check = quote
-                $s = $(f.args[1])
+                $s = $(esc(f.args[1]))
                 isa($s, Module)
             end
-            push!(out_f.args, Expr(:(.), s, f.args[2]))
+            push!(out_f.args, Expr(:(.), s, esc(f.args[2])))
         else
-            push!(out_f.args, f)
+            push!(out_f.args, esc(f))
         end
-        append!(out_f.args, args)
+        append!(out_f.args, Any[esc(arg) for arg in args])
 
         if @isdefined(s)
-            f = :(GlobalRef($s, $(f.args[2])))
-        elseif !isa(f, Symbol)
-            return esc(out_f)
+            f = :(GlobalRef($s, $(esc(f.args[2]))))
+        elseif isa(f, Symbol)
+            check = esc(:($(Expr(:isglobal, f))))
         else
-            check = :($(Expr(:isglobal, f)))
+            return out_f
         end
     end
 
     out_gr = Expr(:call, GlobalRef(Base, :invokelatest_gr))
-    isempty(kwargs) || push!(out_gr.args, Expr(:parameters, kwargs...))
+    isempty(kwargs) || push!(out_gr.args, Expr(:parameters, Any[esc(kw) for kw in kwargs]...))
     push!(out_gr.args, isa(f, GlobalRef) ? QuoteNode(f) :
                        isa(f, Symbol) ? QuoteNode(GlobalRef(__module__, f)) :
                        f)
-    append!(out_gr.args, args)
+    append!(out_gr.args, Any[esc(arg) for arg in args])
 
     if isa(f, GlobalRef)
-        return esc(out_gr)
+        return out_gr
     end
 
     # f::Symbol
-    return esc(:($check ? $out_gr : $out_f))
+    return :($check ? $out_gr : $out_f)
 end
 
 function destructure_callex(topmod::Module, @nospecialize(ex))

base/show.jl

@@ -1,7 +1,7 @@
 # This file is a part of Julia. License is MIT: https://julialang.org/license
 
 using .Compiler: has_typevar
-using .Meta: isidentifier, isoperator, isunaryoperator, isbinaryoperator, ispostfixoperator, 
+using .Meta: isidentifier, isoperator, isunaryoperator, isbinaryoperator, ispostfixoperator,
             is_id_start_char, is_id_char, _isoperator, is_syntactic_operator, is_valid_identifier
 
 function show(io::IO, ::MIME"text/plain", u::UndefInitializer)

base/simdloop.jl

@@ -60,22 +60,22 @@ function compile(x, ivdep)
     check_body!(x)
 
     var,range = parse_iteration_space(x.args[1])
-    r = gensym("r") # Range value
-    j = gensym("i") # Iteration variable for outer loop
-    n = gensym("n") # Trip count for inner loop
-    i = gensym("i") # Trip index for inner loop
-    quote
+    # r: Range value
+    # j: Iteration variable for outer loop
+    # n: Trip count for inner loop
+    # i: Trip index for inner loop
+    return quote
         # Evaluate range value once, to enhance type and data flow analysis by optimizers.
-        let $r = $range
-            for $j in Base.simd_outer_range($r)
-                let $n = Base.simd_inner_length($r,$j)
-                    if zero($n) < $n
+        let r = $(esc(range))
+            for j in Base.simd_outer_range(r)
+                let n = Base.simd_inner_length(r,j)
+                    if zero(n) < n
                         # Lower loop in way that seems to work best for LLVM 3.3 vectorizer.
-                        let $i = zero($n)
-                            while $i < $n
-                                local $var = Base.simd_index($r,$j,$i)
-                                $(x.args[2])        # Body of loop
-                                $i += 1
+                        let i = zero(n)
+                            while i < n
+                                local $(esc(var)) = Base.simd_index(r,j,i)
+                                $(esc(x.args[2]))        # Body of loop
+                                i += 1
                                 $(Expr(:loopinfo, Symbol("julia.simdloop"), ivdep))  # Mark loop as SIMD loop
                             end
                         end
@@ -125,12 +125,12 @@ either case, your inner loop should have the following properties to allow vecto
 * No iteration ever waits on a previous iteration to make forward progress.
 """
 macro simd(forloop)
-    esc(compile(forloop, nothing))
+    compile(forloop, nothing)
 end
 
 macro simd(ivdep, forloop)
     if ivdep === :ivdep
-        esc(compile(forloop, Symbol("julia.ivdep")))
+        compile(forloop, Symbol("julia.ivdep"))
     else
         throw(SimdError("Only ivdep is valid as the first argument to @simd"))
     end

base/some.jl

@@ -152,8 +152,9 @@ macro something(args...)
     which is why we need the last argument first
     when building the final expression.
     =#
-    for arg in reverse(args)
-        val = gensym()
+    for i in reverse(eachindex(args))
+        arg  = args[i]
+        val = Cartesian.inlineanonymous(:val, i)
         expr = quote
             $val = $(esc(arg))
             if !isnothing($val)

base/views.jl

@@ -29,7 +29,8 @@ function replace_ref_begin_end_!(ex, withex)
     elseif isa(ex,Expr)
         if ex.head === :ref
             ex.args[1], used_withex = replace_ref_begin_end_!(ex.args[1], withex)
-            S = isa(ex.args[1],Symbol) ? ex.args[1]::Symbol : gensym(:S) # temp var to cache ex.args[1] if needed
+            S = gensym(:S) # temp var to cache ex.args[1] if needed. if S is a global or expression, then it has side effects to use
+            assignments = []
             used_S = false # whether we actually need S
             # new :ref, so redefine withex
             nargs = length(ex.args)-1
@@ -39,29 +40,62 @@ function replace_ref_begin_end_!(ex, withex)
                 # replace with lastindex(S)
                 ex.args[2], used_S = replace_ref_begin_end_!(ex.args[2], (:($firstindex($S)),:($lastindex($S))))
             else
-                n = 1
+                ni = 1
+                nx = 0
                 J = lastindex(ex.args)
+                need_temps = false # whether any arg needs temporaries
+
+                # First pass: determine if any argument will needs temporaries
+                for j = 2:J
+                    exj = ex.args[j]
+                    if isexpr(exj, :...)
+                        need_temps = true
+                        break
+                    end
+                end
+
+                # Second pass: if any need temps, create temps for all args
+                temp_vars = Tuple{Int,Symbol}[]
                 for j = 2:J
+                    n = nx === 0 ? ni : :($nx + $ni)
                     exj, used = replace_ref_begin_end_!(ex.args[j], (:($firstindex($S,$n)),:($lastindex($S,$n))))
                     used_S |= used
                     ex.args[j] = exj
-                    if isa(exj,Expr) && exj.head === :...
-                        # splatted object
-                        exjs = exj.args[1]
-                        n = :($n + length($exjs))
-                    elseif isa(n, Expr)
-                        # previous expression splatted
-                        n = :($n + 1)
-                    else
-                        # an integer
-                        n += 1
+                    ni += 1
+                    if need_temps
+                        isva = isexpr(exj, :...) # implied need_temps
+                        if isva
+                            exj = exj.args[1]
+                        end
+                        if isa_ast_node(exj) # create temp to preserve evaluation order and count in case `used` gets set later
+                            exj = gensym(:arg)
+                            push!(temp_vars, (j, exj))
+                        end
+                        if isva
+                            ni -= 1
+                            nx = nx === 0 ? :(length($exj)) : :($nx + length($exj))
+                        end
+                    end
+                end
+
+                # Third pass: if `used`, need to actually make those temp assignments now
+                if used_S
+                    for (j, temp_var) in temp_vars
+                        exj = ex.args[j]
+                        isva = isexpr(exj, :...) # implied need_temps
+                        if isva
+                            exj = exj.args[1]
+                        end
+                        push!(assignments, :(local $temp_var = $exj))
+                        ex.args[j] = isva ? Expr(:..., temp_var) : temp_var
                     end
                 end
             end
-            if used_S && S !== ex.args[1]
+
+            if used_S
                 S0 = ex.args[1]
                 ex.args[1] = S
-                ex = Expr(:let, :($S = $S0), ex)
+                ex = :(local $S = $S0; $(assignments...); $ex)
             end
         else
             # recursive search
@@ -131,11 +165,13 @@ macro view(ex)
     #      `view(A, idx) = xxx` in cases such as `@view(A[idx]) = xxx.`
     if Meta.isexpr(ex, :ref)
         ex = Expr(:call, view, ex.args...)
-    elseif Meta.isexpr(ex, :let) && (arg2 = ex.args[2]; Meta.isexpr(arg2, :ref))
+    elseif Meta.isexpr(ex, :block)
+        arg2 = ex.args[end]
+        Meta.isexpr(arg2, :ref) || error("unsupported replace_ref_begin_end result")
         # ex replaced by let ...; foo[...]; end
-        ex.args[2] = Expr(:call, view, arg2.args...)
+        ex.args[end] = Expr(:call, view, arg2.args...)
     else
-        error("invalid expression")
+        error("unsupported replace_ref_begin_end result")
     end
     return esc(ex)
 end
@@ -176,10 +212,7 @@ function _views(ex::Expr)
 
             # temp vars to avoid recomputing a and i,
             # which will be assigned in a let block:
-            a = gensym(:a)
-            i = let lhs=lhs     # #15276
-                [gensym(:i) for k = 1:length(lhs.args)-1]
-            end
+            i = Symbol[Symbol(:i, k) for k = 1:length(lhs.args)-1]
 
             # for splatted indices like a[i, j...], we need to
             # splat the corresponding temp var.
@@ -194,14 +227,15 @@ function _views(ex::Expr)
                 end
             end
 
-            Expr(:let,
-                 Expr(:block,
-                      :($a = $(_views(lhs.args[1]))),
-                      Any[:($(i[k]) = $(_views(lhs.args[k+1]))) for k=1:length(i)]...),
-                 Expr(first(h) == '.' ? :(.=) : :(=), :($a[$(I...)]),
-                      Expr(:call, Symbol(h[1:end-1]),
-                           :($maybeview($a, $(I...))),
-                           mapany(_views, ex.args[2:end])...)))
+            Expr(:var"hygienic-scope", # assign a and i to the macro's scope
+                 Expr(:let,
+                      Expr(:block,
+                           :(a = $(esc(_views(lhs.args[1])))),
+                           Any[:($(i[k]) = $(esc(_views(lhs.args[k+1])))) for k=1:length(i)]...),
+                      Expr(first(h) == '.' ? :(.=) : :(=), :(a[$(I...)]),
+                           Expr(:call, esc(Symbol(h[1:end-1])),
+                                :($maybeview(a, $(I...))),
+                                mapany(e -> esc(_views(e)), ex.args[2:end])...))), Base)
         else
             exprarray(ex.head, mapany(_views, ex.args))
         end

doc/src/devdocs/ast.md

@@ -236,9 +236,9 @@ These expressions are represented as `LineNumberNode`s in Julia.
 ### Macros
 
 Macro hygiene is represented through the expression head pair `escape` and `hygienic-scope`.
-The result of a macro expansion is automatically wrapped in `(hygienic-scope block module)`,
+The result of a macro expansion is automatically wrapped in `(hygienic-scope block module [lno])`,
 to represent the result of the new scope. The user can insert `(escape block)` inside
-to interpolate code from the caller.
+to interpolate code from the caller. The lno is the `__source__` argument of the macro, if included.
 
 
 ## Lowered form