Bypass SyntaxNode in JuliaLowering; fix kw bug (JuliaLang#60162)

The `kw` form isn't produced in `SyntaxNode`/`SyntaxTree` like it is in Expr,
which simplifies the parser, but causes problems once we start start caring
about the semantics of these trees. Parsing `"f(a=1)"` will always produce an
`Expr(:call, :f, Expr(:kw, :a, 1))`, but the equivalent SyntaxNode
representation `(call f (= a 1))` makes `Expr(:call, :f, Expr(:(=), :a, 1))`
unrepresentable, even though that's valid syntax a macro could produce.

To fix this, we need to convert `=` to `kw` within certain forms before
macro expansion.

Author: mlechu

JuliaLowering/src/compat.jl

@@ -34,9 +34,7 @@ end
     graph = syntax_graph(ctx)
     toplevel_src = if isnothing(lnn)
         # Provenance sinkhole for all nodes until we hit a linenode
-        dummy_src = SourceRef(
-            SourceFile("No source for expression"),
-            1, JS.GreenNode(K"None", 0))
+        dummy_src = SourceRef(SourceFile("No source for expression"), 1, 0)
         _insert_tree_node(graph, K"None", dummy_src)
     else
         lnn
@@ -46,15 +44,14 @@ end
     return out
 end
 
-function _expr_replace!(@nospecialize(e), replace_pred::Function, replacer!::Function,
+function _expr_replace(@nospecialize(e), replace_pred::Function, replacer::Function,
                         recurse_pred=(@nospecialize e)->true)
     if replace_pred(e)
-        replacer!(e)
-    end
-    if e isa Expr && recurse_pred(e)
-        for a in e.args
-            _expr_replace!(a, replace_pred, replacer!, recurse_pred)
-        end
+        replacer(e)
+    elseif e isa Expr && recurse_pred(e)
+        Expr(e.head, Any[_expr_replace(a, replace_pred, replacer, recurse_pred) for a in e.args]...)
+    else
+        e
     end
 end
 
@@ -366,7 +363,8 @@ function _insert_convert_expr(@nospecialize(e), graph::SyntaxGraph, src::SourceA
             lam_eqs = Any[]
             for a in a1.args
                 a isa LineNumberNode && continue
-                a isa Expr && a.head === :(=) ? push!(lam_eqs, a) : push!(lam_args, a)
+                a isa Expr && a.head === :(=) ?
+                    push!(lam_eqs, Expr(:kw, a.args...)) : push!(lam_args, a)
             end
             !isempty(lam_eqs) && push!(lam_args, Expr(:parameters, lam_eqs...))
             child_exprs[1] = a1_esc(Expr(:tuple, lam_args...))
@@ -436,11 +434,9 @@ function _insert_convert_expr(@nospecialize(e), graph::SyntaxGraph, src::SourceA
         child_exprs = child_exprs[2:end]
         # TODO handle docstrings after refactor
     elseif (e.head === :using || e.head === :import)
-        _expr_replace!(e,
-                       (e)->(e isa Expr && e.head === :.),
-                       (e)->(e.head = :importpath))
-    elseif e.head === :kw
-        st_k = K"="
+        e2 = _expr_replace(e, (e)->(e isa Expr && e.head === :.),
+                           (e)->Expr(:importpath, e.args...))
+        child_exprs = e2.args
     elseif e.head in (:local, :global) && nargs > 1
         # Possible normalization
         # child_exprs = Any[Expr(:tuple, child_exprs...)]

JuliaLowering/src/desugaring.jl

@@ -86,7 +86,7 @@ function check_no_parameters(ex::SyntaxTree, msg)
 end
 
 function check_no_assignment(exs, msg="misplaced assignment statement in `[ ... ]`")
-    i = findfirst(kind(e) == K"=" for e in exs)
+    i = findfirst(kind(e) == K"=" || kind(e) == K"kw" for e in exs)
     if !isnothing(i)
         throw(LoweringError(exs[i], msg))
     end
@@ -532,17 +532,17 @@ end
 
 #-------------------------------------------------------------------------------
 # Expansion of array indexing
-function _arg_to_temp(ctx, stmts, ex, eq_is_kw=false)
+function _arg_to_temp(ctx, stmts, ex)
     k = kind(ex)
     if is_effect_free(ex)
         ex
     elseif k == K"..."
         @ast ctx ex [k _arg_to_temp(ctx, stmts, ex[1])]
-    elseif k == K"=" && eq_is_kw
-        @ast ctx ex [K"=" ex[1] _arg_to_temp(ctx, stmts, ex[2], false)]
+    elseif k == K"kw"
+        @ast ctx ex [K"kw" ex[1] _arg_to_temp(ctx, stmts, ex[2])]
     elseif k == K"parameters"
         mapchildren(ctx, ex) do e
-            _arg_to_temp(ctx, stmts, e, true)
+            _arg_to_temp(ctx, stmts, e)
         end
     else
         emit_assign_tmp(stmts, ctx, ex)
@@ -565,9 +565,8 @@ function remove_argument_side_effects(ctx, stmts, ex)
             emit_assign_tmp(stmts, ctx, ex)
         else
             args = SyntaxList(ctx)
-            eq_is_kw = ((k == K"call" || k == K"dotcall") && is_prefix_call(ex)) || k == K"ref"
             for (i,e) in enumerate(children(ex))
-                push!(args, _arg_to_temp(ctx, stmts, e, eq_is_kw && i > 1))
+                push!(args, _arg_to_temp(ctx, stmts, e))
             end
             # TODO: Copy attributes?
             @ast ctx ex [k args...]
@@ -1613,7 +1612,7 @@ function _merge_named_tuple(ctx, srcref, old, new)
     end
 end
 
-function expand_named_tuple(ctx, ex, kws;
+function expand_named_tuple(ctx, ex, kws, eq_is_kw;
                             field_name="named tuple field",
                             element_name="named tuple element")
     name_strs = Set{String}()
@@ -1628,7 +1627,8 @@ function expand_named_tuple(ctx, ex, kws;
             # x  ==>  x = x
             name = to_symbol(ctx, kw)
             value = kw
-        elseif k == K"="
+        elseif k == K"kw" || (eq_is_kw && k == K"=")
+            # syntax TODO: This should parse to K"kw"
             # x = a
             if kind(kw[1]) != K"Identifier" && kind(kw[1]) != K"Placeholder"
                 throw(LoweringError(kw[1], "invalid $field_name name"))
@@ -1696,7 +1696,7 @@ end
 function expand_kw_call(ctx, srcref, farg, args, kws)
     @ast ctx srcref [K"block"
         func := farg
-        kw_container := expand_named_tuple(ctx, srcref, kws;
+        kw_container := expand_named_tuple(ctx, srcref, kws, false;
                                            field_name="keyword argument",
                                            element_name="keyword argument")
         if all(kind(kw) == K"..." for kw in kws)
@@ -1869,7 +1869,7 @@ function remove_kw_args!(ctx, args::SyntaxList)
     for i in 1:length(args)
         arg = args[i]
         k = kind(arg)
-        if k == K"="
+        if k == K"kw"
             if isnothing(kws)
                 kws = SyntaxList(ctx)
             end
@@ -2277,7 +2277,7 @@ end
 function expand_function_arg(ctx, body_stmts, arg, is_last_arg, is_kw, arg_id)
     ex = arg
 
-    if kind(ex) == K"="
+    if kind(ex) == K"kw"
         default = ex[2]
         ex = ex[1]
     else
@@ -2858,8 +2858,10 @@ function keyword_function_defs(ctx, srcref, callex_srcref, name_str, typevar_nam
             kwcall_body_tail
         ]
     else
-        scope_nest(ctx, kw_names, kw_values, kwcall_body_tail)
+        scope_nest(ctx, has_kw_slurp ? kw_names[1:end-1] : kw_names,
+                   kw_values, kwcall_body_tail)
     end
+
     main_kwcall_typevars = trim_used_typevars(ctx, kwcall_arg_types, typevar_names, typevar_stmts)
     push!(kwcall_method_defs,
           method_def_expr(ctx, srcref, callex_srcref, kwcall_mtable,
@@ -3233,6 +3235,29 @@ end
 
 #-------------------------------------------------------------------------------
 # Anon function syntax
+function expand_arrow_args(ctx, arglist)
+    k = kind(arglist)
+    # The arglist can sometimes be parsed as a block, or something else, and
+    # fixing this is extremely awkward when nested inside `where`. See
+    # https://github.com/JuliaLang/JuliaSyntax.jl/pull/522
+    if k == K"block"
+        @chk numchildren(arglist) == 2
+        kw = arglist[2]
+        if kind(kw) === K"="
+            kw = @ast ctx kw [K"kw" children(kw)...]
+        end
+        arglist = @ast ctx arglist [K"tuple"
+            arglist[1]
+            [K"parameters" kw]
+        ]
+    elseif k != K"tuple"
+        arglist = @ast ctx arglist [K"tuple" arglist]
+    end
+    return mapchildren(ctx, arglist) do a
+        kind(a) === K"=" ? @ast(ctx, a, [K"kw" children(a)...]) : a
+    end
+end
+
 function expand_arrow_arglist(ctx, arglist, arrowname)
     k = kind(arglist)
     if k == K"where"
@@ -3241,23 +3266,9 @@ function expand_arrow_arglist(ctx, arglist, arrowname)
             arglist[2]
         ]
     else
-        # The arglist can sometimes be parsed as a block, or something else, and
-        # fixing this is extremely awkward when nested inside `where`. See
-        # https://github.com/JuliaLang/JuliaSyntax.jl/pull/522
-        if k == K"block"
-            @chk numchildren(arglist) == 2
-            arglist = @ast ctx arglist [K"tuple"
-                arglist[1]
-                [K"parameters" arglist[2]]
-            ]
-        elseif k != K"tuple"
-            arglist = @ast ctx arglist [K"tuple"
-                arglist[1]
-            ]
-        end
         @ast ctx arglist [K"call"
             arrowname::K"Placeholder"
-            children(arglist)...
+            children(expand_arrow_args(ctx, arglist))...
         ]
     end
 end
@@ -3280,7 +3291,7 @@ function expand_opaque_closure(ctx, ex)
     func_expr = ex[5]
     @chk kind(func_expr) == K"->"
     @chk numchildren(func_expr) == 2
-    args = func_expr[1]
+    args = expand_arrow_args(ctx, func_expr[1])
     @chk kind(args) == K"tuple"
     check_no_parameters(ex, args)
 
@@ -3829,7 +3840,7 @@ function _rewrite_ctor_new_calls(ctx, ex, struct_name, global_struct_name, ctor_
         )
     end
     # Rewrite a call to new()
-    kw_arg_i = findfirst(e->(k = kind(e); k == K"=" || k == K"parameters"), children(ex))
+    kw_arg_i = findfirst(e->(k = kind(e); k == K"kw" || k == K"parameters"), children(ex))
     if !isnothing(kw_arg_i)
         throw(LoweringError(ex[kw_arg_i], "`new` does not accept keyword arguments"))
     end
@@ -4147,7 +4158,7 @@ function expand_struct_def(ctx, ex, docs)
                 struct_name
                 isnothing(docs) ? nothing_(ctx, ex) : docs[1]
                 ::K"SourceLocation"(ex)
-                [K"="
+                [K"kw"
                     "field_docs"::K"Identifier"
                     [K"call" "svec"::K"core" field_docs...]
                 ]
@@ -4194,7 +4205,7 @@ end
 function expand_curly(ctx, ex)
     @assert kind(ex) == K"curly"
     check_no_parameters(ex, "unexpected semicolon in type parameter list")
-    check_no_assignment(children(ex), "misplace assignment in type parameter list")
+    check_no_assignment(children(ex), "misplaced assignment in type parameter list")
 
     typevar_stmts = SyntaxList(ctx)
     type_args = SyntaxList(ctx)
@@ -4512,9 +4523,9 @@ function expand_forms_2(ctx::DesugaringContext, ex::SyntaxTree, docs=nothing)
             if numchildren(ex) > 1
                 throw(LoweringError(ex[end], "unexpected semicolon in tuple - use `,` to separate tuple elements"))
             end
-            expand_forms_2(ctx, expand_named_tuple(ctx, ex, children(ex[1])))
+            expand_forms_2(ctx, expand_named_tuple(ctx, ex, children(ex[1]), true))
         elseif any_assignment(children(ex))
-            expand_forms_2(ctx, expand_named_tuple(ctx, ex, children(ex)))
+            expand_forms_2(ctx, expand_named_tuple(ctx, ex, children(ex), true))
         else
             expand_forms_2(ctx, @ast ctx ex [K"call"
                 "tuple"::K"core"
@@ -4558,7 +4569,7 @@ function expand_forms_2(ctx::DesugaringContext, ex::SyntaxTree, docs=nothing)
                 ctx.mod               ::K"Value"
                 [K"inert" ex]
                 [K"parameters"
-                    [K"="
+                    [K"kw"
                         "expr_compat_mode"::K"Identifier"
                         ctx.expr_compat_mode::K"Bool"
                     ]

JuliaLowering/src/hooks.jl

@@ -15,7 +15,7 @@ function core_lowering_hook(@nospecialize(code), mod::Module,
     file = file isa Ptr{UInt8} ? unsafe_string(file) : file
     line = !(line isa Int) ? Int(line) : line
 
-    local st0 = nothing
+    local st0, st1 = nothing, nothing
     try
         st0 = code isa Expr ? expr_to_syntaxtree(code, LineNumberNode(line, file)) : code
         if kind(st0) in KSet"toplevel module"
@@ -32,7 +32,7 @@ function core_lowering_hook(@nospecialize(code), mod::Module,
         ex = to_lowered_expr(st5)
         return Core.svec(ex, st5, ctx5)
     catch exc
-        @info("JuliaLowering threw given input:", code=code, st0=st0, file=file, line=line, mod=mod)
+        @info("JuliaLowering threw given input:", code=code, st0=st0, st1=st1, file=file, line=line, mod=mod)
         rethrow(exc)
 
         # TODO: Re-enable flisp fallback once we're done collecting errors

JuliaLowering/src/macro_expansion.jl

@@ -471,8 +471,9 @@ function expand_forms_1(ctx::MacroExpansionContext, ex::SyntaxTree)
         # TODO: Upstream should set a general flag for detecting parenthesized
         # expressions so we don't need to dig into `green_tree` here. Ugh!
         plain_symbol = has_flags(ex, JuliaSyntax.COLON_QUOTE) &&
-                       kind(ex[1]) == K"Identifier" &&
-                       (sr = sourceref(ex); sr isa SourceRef && kind(sr.green_tree[2]) != K"parens")
+            kind(ex[1]) == K"Identifier" && (
+                prov = flattened_provenance(ex);
+                length(prov) >= 1 && kind(prov[end][end]) != K"parens")
         if plain_symbol
             # As a compromise for compatibility, we treat non-parenthesized
             # colon quoted identifiers like `:x` as plain Symbol literals