Fixes for quote interpolation in Expr compat mode (#57)

Ensure that interpolating values into quoted expressions works in Expr
compat mode.

* To produce `Expr` in `CodeInfo` conversion, add an `as_Expr` attribute
  to `K"inert"`. expand_quote
* In analogy with `eval()`, add `expr_compat_mode` keyword to
  `include_string`. We may want to rethink this when we rethink how
  options to `eval()` work.
* Adapt the runtime function `interpolate_ast` to work with both `Expr`
  and `SyntaxTree` as the source expression.
* Allow `Symbol` to be interpolated into `SyntaxTree` as syntax rather
  than a `K"Value": Even in "new style macro mode", `:x` is interpreted
  as a `Symbol` rather than an AST as it's used by many packages as a
  kind of lightweight untyped enum (ie, a usage which is almost entirely
  unrelated to AST manipulation)
* Fix `SyntaxTree->Expr` conversion so that embedded Expr `K"Value"`
  means a leaf, not an embedded tree.

Author: c42f

src/eval.jl

@@ -261,7 +261,8 @@ function to_lowered_expr(mod, ex, ssa_offset=0)
     elseif k == K"return"
         Core.ReturnNode(to_lowered_expr(mod, ex[1], ssa_offset))
     elseif k == K"inert"
-        ex[1]
+        e1 = ex[1]
+        getmeta(ex, :as_Expr, false) ? QuoteNode(Expr(e1)) : e1
     elseif k == K"code_info"
         funcname = ex.is_toplevel_thunk ?
             "top-level scope" :
@@ -391,7 +392,8 @@ end
 
 Like `include`, except reads code from the given string rather than from a file.
 """
-function include_string(mod::Module, code::AbstractString, filename::AbstractString="string")
-    eval(mod, parseall(SyntaxTree, code; filename=filename))
+function include_string(mod::Module, code::AbstractString, filename::AbstractString="string";
+                        expr_compat_mode=false)
+    eval(mod, parseall(SyntaxTree, code; filename=filename); expr_compat_mode)
 end
 

src/macro_expansion.jl

@@ -57,7 +57,7 @@ function expand_quote(ctx, ex)
     # the entire expression produced by `quote` expansion. We could, but it
     # seems unnecessary for `quote` because the surface syntax is a transparent
     # representation of the expansion process. However, it's useful to add the
-    # extra srcref in a more targetted way for $ interpolations inside
+    # extra srcref in a more targeted way for $ interpolations inside
     # interpolate_ast, so we do that there.
     #
     # In principle, particular user-defined macros could opt into a similar
@@ -69,7 +69,7 @@ function expand_quote(ctx, ex)
     @ast ctx ex [K"call"
         interpolate_ast::K"Value"
         (ctx.expr_compat_mode ? Expr : SyntaxTree)::K"Value"
-        [K"inert" ex]
+        [K"inert"(meta=CompileHints(:as_Expr, ctx.expr_compat_mode)) ex]
         unquoted...
     ]
 end

src/runtime.jl

@@ -21,57 +21,106 @@ struct InterpolationContext{Graph} <: AbstractLoweringContext
     current_index::Ref{Int}
 end
 
+# Context for `Expr`-based AST interpolation in compat mode
+struct ExprInterpolationContext <: AbstractLoweringContext
+    values::Tuple
+    current_index::Ref{Int}
+end
+
+# Helper functions to make shared interpolation code which works with both
+# SyntaxTree and Expr data structures.
+_interp_kind(ex::SyntaxTree) = kind(ex)
+function _interp_kind(@nospecialize(ex))
+    return (ex isa Expr && ex.head === :quote) ? K"quote" :
+           (ex isa Expr && ex.head === :$)     ? K"$"     :
+           K"None" # Other cases irrelevant to interpolation
+end
+
+_children(ex::SyntaxTree) = children(ex)
+_children(@nospecialize(ex)) = ex isa Expr ? ex.args : ()
+
+_numchildren(ex::SyntaxTree) = numchildren(ex)
+_numchildren(@nospecialize(ex)) = ex isa Expr ? length(ex.args) : 0
+
+_syntax_list(ctx::InterpolationContext) = SyntaxList(ctx)
+_syntax_list(ctx::ExprInterpolationContext) = Any[]
+
+_interp_makenode(ctx::InterpolationContext, ex, args) = makenode(ctx, ex, ex, args)
+_interp_makenode(ctx::ExprInterpolationContext, ex, args) = Expr((ex::Expr).head, args...)
+
+_to_syntax_tree(ex::SyntaxTree) = ex
+_to_syntax_tree(@nospecialize(ex)) = expr_to_syntaxtree(ex)
+
+
 function _contains_active_interp(ex, depth)
-    k = kind(ex)
+    k = _interp_kind(ex)
     if k == K"$" && depth == 0
         return true
+    elseif _numchildren(ex) == 0
+        return false
     end
     inner_depth = k == K"quote" ? depth + 1 :
                   k == K"$"     ? depth - 1 :
                   depth
-    return any(_contains_active_interp(c, inner_depth) for c in children(ex))
+    return any(_contains_active_interp(c, inner_depth) for c in _children(ex))
 end
 
 # Produce interpolated node for `$x` syntax
-function _interpolated_value(ctx, srcref, ex)
+function _interpolated_value(ctx::InterpolationContext, srcref, ex)
     if ex isa SyntaxTree
         if !is_compatible_graph(ctx, ex)
             ex = copy_ast(ctx, ex)
         end
         append_sourceref(ctx, ex, srcref)
+    elseif ex isa Symbol
+        # Plain symbols become identifiers. This is an accomodation for
+        # compatibility to allow `:x` (a Symbol) and `:(x)` (a SyntaxTree) to
+        # be used interchangably in macros.
+        makeleaf(ctx, srcref, K"Identifier", string(ex))
     else
         makeleaf(ctx, srcref, K"Value", ex)
     end
 end
 
-function _interpolate_ast(ctx::InterpolationContext, ex, depth)
+function _interpolated_value(::ExprInterpolationContext, _, ex)
+    ex
+end
+
+function copy_ast(::ExprInterpolationContext, @nospecialize(ex))
+    @ccall(jl_copy_ast(ex::Any)::Any)
+end
+
+function _interpolate_ast(ctx, ex, depth)
     if ctx.current_index[] > length(ctx.values) || !_contains_active_interp(ex, depth)
         return ex
     end
 
     # We have an interpolation deeper in the tree somewhere - expand to an
-    # expression 
-    inner_depth = kind(ex) == K"quote" ? depth + 1 :
-                  kind(ex) == K"$"     ? depth - 1 :
+    # expression which performs the interpolation.
+    k = _interp_kind(ex)
+    inner_depth = k == K"quote" ? depth + 1 :
+                  k == K"$"     ? depth - 1 :
                   depth
-    expanded_children = SyntaxList(ctx)
-    for e in children(ex)
-        if kind(e) == K"$" && inner_depth == 0
+
+    expanded_children = _syntax_list(ctx)
+
+    for e in _children(ex)
+        if _interp_kind(e) == K"$" && inner_depth == 0
             vals = ctx.values[ctx.current_index[]]::Tuple
             ctx.current_index[] += 1
             for (i,v) in enumerate(vals)
-                srcref = numchildren(e) == 1 ? e : e[i]
+                srcref = _numchildren(e) == 1 ? e : _children(e)[i]
                 push!(expanded_children, _interpolated_value(ctx, srcref, v))
             end
         else
             push!(expanded_children, _interpolate_ast(ctx, e, inner_depth))
         end
     end
 
-    makenode(ctx, ex, head(ex), expanded_children)
+    _interp_makenode(ctx, ex, expanded_children)
 end
 
-function interpolate_ast(::Type{SyntaxTree}, ex, values...)
+function _setup_interpolation(::Type{SyntaxTree}, ex, values)
     # Construct graph for interpolation context. We inherit this from the macro
     # context where possible by detecting it using __macro_ctx__. This feels
     # hacky though.
@@ -92,30 +141,33 @@ function interpolate_ast(::Type{SyntaxTree}, ex, values...)
                                   value=Any, name_val=String, scope_layer=LayerId)
     end
     ctx = InterpolationContext(graph, values, Ref(1))
+    return ctx
+end
+
+function _setup_interpolation(::Type{Expr}, ex, values)
+    return ExprInterpolationContext(values, Ref(1))
+end
+
+function interpolate_ast(::Type{T}, ex, values...) where {T}
+    ctx = _setup_interpolation(T, ex, values)
+
     # We must copy the AST into our context to use it as the source reference
-    # of generated expressions.
+    # of generated expressions (and in the Expr case at least, to avoid mutation)
     ex1 = copy_ast(ctx, ex)
-    if kind(ex1) == K"$"
+    if _interp_kind(ex1) == K"$"
         @assert length(values) == 1
         vs = values[1]
         if length(vs) > 1
             # :($($(xs...))) where xs is more than length 1
-            throw(LoweringError(ex1, "More than one value in bare `\$` expression"))
+            throw(LoweringError(_to_syntax_tree(ex1),
+                                "More than one value in bare `\$` expression"))
         end
         _interpolated_value(ctx, ex1, only(vs))
     else
         _interpolate_ast(ctx, ex1, 0)
     end
 end
 
-function interpolate_ast(::Type{Expr}, ex, values...)
-    # TODO: Adjust `_interpolated_value` to ensure that incoming `Expr` data
-    # structures are treated as AST in Expr compat mode, rather than `K"Value"`?
-    # Or convert `ex` to `Expr` early during lowering and implement
-    # `interpolate_ast` for `Expr`?
-    Expr(interpolate_ast(SyntaxTree, ex, values...))
-end
-
 #--------------------------------------------------
 # Functions called by closure conversion
 function eval_closure_type(mod::Module, closure_type_name::Symbol, field_names, field_is_box)

src/syntax_graph.jl

@@ -583,7 +583,14 @@ function JuliaSyntax._expr_leaf_val(ex::SyntaxTree, _...)
             n
         end
     else
-        get(ex, :value, nothing)
+        val = get(ex, :value, nothing)
+        if kind(ex) == K"Value" && val isa Expr || val isa LineNumberNode
+            # Expr AST embedded in a SyntaxTree should be quoted rather than
+            # becoming part of the output AST.
+            QuoteNode(val)
+        else
+            val
+        end
     end
 end
 
@@ -774,6 +781,7 @@ end
 
 function Base.deleteat!(v::SyntaxList, inds)
     deleteat!(v.ids, inds)
+    v
 end
 
 function Base.copy(v::SyntaxList)

test/quoting.jl

@@ -95,14 +95,14 @@ end
 @test ex[2].name_val == "a"
 
 # interpolations at multiple depths
-ex = JuliaLowering.include_string(test_mod, """
+ex = JuliaLowering.include_string(test_mod, raw"""
 let
     args = (:(x),:(y))
     quote
         x = 1
         y = 2
         quote
-            f(\$\$(args...))
+            f($$(args...))
         end
     end
 end
@@ -140,23 +140,85 @@ ex2 = JuliaLowering.eval(test_mod, ex)
 @test JuliaLowering.include_string(test_mod, ":(x)") isa SyntaxTree
 
 # Double interpolation
-ex = JuliaLowering.include_string(test_mod, """
+double_interp_ex = JuliaLowering.include_string(test_mod, raw"""
 let
     args = (:(xxx),)
-    :(:(\$\$(args...)))
+    :(:($$(args...)))
 end
 """)
 Base.eval(test_mod, :(xxx = 111))
-ex2 = JuliaLowering.eval(test_mod, ex)
-@test kind(ex2) == K"Value"
-@test ex2.value == 111
+dinterp_eval = JuliaLowering.eval(test_mod, double_interp_ex)
+@test kind(dinterp_eval) == K"Value"
+@test dinterp_eval.value == 111
 
-double_interp_ex = JuliaLowering.include_string(test_mod, """
+multi_interp_ex = JuliaLowering.include_string(test_mod, raw"""
 let
     args = (:(x), :(y))
-    :(:(\$\$(args...)))
+    :(:($$(args...)))
+end
+""")
+@test_throws LoweringError JuliaLowering.eval(test_mod, multi_interp_ex)
+
+# Interpolation of SyntaxTree Identifier vs plain Symbol
+symbol_interp = JuliaLowering.include_string(test_mod, raw"""
+let
+    x = :xx    # Plain Symbol
+    y = :(yy)  # SyntaxTree K"Identifier"
+    :(f($x, $y, z))
+end
+""")
+@test symbol_interp ≈ @ast_ [K"call"
+    "f"::K"Identifier"
+    "xx"::K"Identifier"
+    "yy"::K"Identifier"
+    "z"::K"Identifier"
+]
+@test sourcetext(symbol_interp[2]) == "\$x" # No provenance for plain Symbol
+@test sourcetext(symbol_interp[3]) == "yy"
+
+# Mixing Expr into a SyntaxTree doesn't graft it onto the SyntaxTree AST but
+# treats it as a plain old value. (This is the conservative API choice and also
+# encourages ASTs to be written in the new form. However we may choose to
+# change this if necessary for compatibility.)
+expr_interp_is_value = JuliaLowering.include_string(test_mod, raw"""
+let
+    x = Expr(:call, :f, :x)
+    :(g($x))
 end
 """)
-@test_throws LoweringError JuliaLowering.eval(test_mod, double_interp_ex)
+@test expr_interp_is_value ≈ @ast_ [K"call"
+    "g"::K"Identifier"
+    Expr(:call, :f, :x)::K"Value"
+    # ^^ NB not [K"call" "f"::K"Identifier" "x"::K"Identifier"]
+]
+@test Expr(expr_interp_is_value) == Expr(:call, :g, QuoteNode(Expr(:call, :f, :x)))
+
+@testset "Interpolation in Expr compat mode" begin
+    expr_interp = JuliaLowering.include_string(test_mod, raw"""
+    let
+        x = :xx
+        :(f($x, z))
+    end
+    """, expr_compat_mode=true)
+    @test expr_interp == Expr(:call, :f, :xx, :z)
+
+    double_interp_expr = JuliaLowering.include_string(test_mod, raw"""
+    let
+        x = :xx
+        :(:(f($$x, $y)))
+    end
+    """, expr_compat_mode=true)
+    @test double_interp_expr == Expr(:quote, Expr(:call, :f, Expr(:$, :xx), Expr(:$, :y)))
+
+    # Test that ASTs are copied before they're seen by the user
+    @test JuliaLowering.include_string(test_mod, raw"""
+    exs = []
+    for i = 1:2
+        push!(exs, :(f(x,y)))
+        push!(exs[end].args, :z)
+    end
+    exs
+    """, expr_compat_mode=true) == Any[Expr(:call, :f, :x, :y, :z), Expr(:call, :f, :x, :y, :z)]
+end
 
 end