Skip to content

Commit 296cca2

Browse files
authored
Bypass SyntaxNode in JuliaLowering; fix kw bug (#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.
1 parent cb2ceb9 commit 296cca2

File tree

12 files changed

+220
-107
lines changed

12 files changed

+220
-107
lines changed

‎JuliaLowering/src/compat.jl‎

Lines changed: 12 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -34,9 +34,7 @@ end
3434
graph =syntax_graph(ctx)
3535
toplevel_src =ifisnothing(lnn)
3636
# Provenance sinkhole for all nodes until we hit a linenode
37-
dummy_src =SourceRef(
38-
SourceFile("No source for expression"),
39-
1, JS.GreenNode(K"None", 0))
37+
dummy_src =SourceRef(SourceFile("No source for expression"), 1, 0)
4038
_insert_tree_node(graph, K"None", dummy_src)
4139
else
4240
lnn
@@ -46,15 +44,14 @@ end
4644
return out
4745
end
4846

49-
function_expr_replace!(@nospecialize(e), replace_pred::Function, replacer!::Function,
47+
function_expr_replace(@nospecialize(e), replace_pred::Function, replacer::Function,
5048
recurse_pred=(@nospecialize e)->true)
5149
ifreplace_pred(e)
52-
replacer!(e)
53-
end
54-
if e isa Expr &&recurse_pred(e)
55-
for a in e.args
56-
_expr_replace!(a, replace_pred, replacer!, recurse_pred)
57-
end
50+
replacer(e)
51+
elseif e isa Expr &&recurse_pred(e)
52+
Expr(e.head, Any[_expr_replace(a, replace_pred, replacer, recurse_pred) for a in e.args]...)
53+
else
54+
e
5855
end
5956
end
6057

@@ -366,7 +363,8 @@ function _insert_convert_expr(@nospecialize(e), graph::SyntaxGraph, src::SourceA
366363
lam_eqs = Any[]
367364
for a in a1.args
368365
a isa LineNumberNode &&continue
369-
a isa Expr && a.head === :(=) ?push!(lam_eqs, a) :push!(lam_args, a)
366+
a isa Expr && a.head === :(=) ?
367+
push!(lam_eqs, Expr(:kw, a.args...)) :push!(lam_args, a)
370368
end
371369
!isempty(lam_eqs) &&push!(lam_args, Expr(:parameters, lam_eqs...))
372370
child_exprs[1] =a1_esc(Expr(:tuple, lam_args...))
@@ -436,11 +434,9 @@ function _insert_convert_expr(@nospecialize(e), graph::SyntaxGraph, src::SourceA
436434
child_exprs = child_exprs[2:end]
437435
#TODO handle docstrings after refactor
438436
elseif (e.head ===:using|| e.head ===:import)
439-
_expr_replace!(e,
440-
(e)->(e isa Expr && e.head === :.),
441-
(e)->(e.head =:importpath))
442-
elseif e.head ===:kw
443-
st_k =K"="
437+
e2 =_expr_replace(e, (e)->(e isa Expr && e.head === :.),
438+
(e)->Expr(:importpath, e.args...))
439+
child_exprs = e2.args
444440
elseif e.head in (:local, :global) && nargs >1
445441
# Possible normalization
446442
# child_exprs = Any[Expr(:tuple, child_exprs...)]

‎JuliaLowering/src/desugaring.jl‎

Lines changed: 46 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ function check_no_parameters(ex::SyntaxTree, msg)
8686
end
8787

8888
functioncheck_no_assignment(exs, msg="misplaced assignment statement in `[ ... ]`")
89-
i =findfirst(kind(e) ==K"="for e in exs)
89+
i =findfirst(kind(e) ==K"="||kind(e) ==K"kw"for e in exs)
9090
if!isnothing(i)
9191
throw(LoweringError(exs[i], msg))
9292
end
@@ -532,17 +532,17 @@ end
532532

533533
#-------------------------------------------------------------------------------
534534
# Expansion of array indexing
535-
function_arg_to_temp(ctx, stmts, ex, eq_is_kw=false)
535+
function_arg_to_temp(ctx, stmts, ex)
536536
k =kind(ex)
537537
ifis_effect_free(ex)
538538
ex
539539
elseif k ==K"..."
540540
@ast ctx ex [k _arg_to_temp(ctx, stmts, ex[1])]
541-
elseif k ==K"="&& eq_is_kw
542-
@ast ctx ex [K"=" ex[1] _arg_to_temp(ctx, stmts, ex[2], false)]
541+
elseif k ==K"kw"
542+
@ast ctx ex [K"kw" ex[1] _arg_to_temp(ctx, stmts, ex[2])]
543543
elseif k ==K"parameters"
544544
mapchildren(ctx, ex) do e
545-
_arg_to_temp(ctx, stmts, e, true)
545+
_arg_to_temp(ctx, stmts, e)
546546
end
547547
else
548548
emit_assign_tmp(stmts, ctx, ex)
@@ -565,9 +565,8 @@ function remove_argument_side_effects(ctx, stmts, ex)
565565
emit_assign_tmp(stmts, ctx, ex)
566566
else
567567
args =SyntaxList(ctx)
568-
eq_is_kw = ((k ==K"call"|| k ==K"dotcall") &&is_prefix_call(ex)) || k ==K"ref"
569568
for (i,e) inenumerate(children(ex))
570-
push!(args, _arg_to_temp(ctx, stmts, e, eq_is_kw && i >1))
569+
push!(args, _arg_to_temp(ctx, stmts, e))
571570
end
572571
#TODO: Copy attributes?
573572
@ast ctx ex [k args...]
@@ -1613,7 +1612,7 @@ function _merge_named_tuple(ctx, srcref, old, new)
16131612
end
16141613
end
16151614

1616-
functionexpand_named_tuple(ctx, ex, kws;
1615+
functionexpand_named_tuple(ctx, ex, kws, eq_is_kw;
16171616
field_name="named tuple field",
16181617
element_name="named tuple element")
16191618
name_strs =Set{String}()
@@ -1628,7 +1627,8 @@ function expand_named_tuple(ctx, ex, kws;
16281627
# x ==> x = x
16291628
name =to_symbol(ctx, kw)
16301629
value = kw
1631-
elseif k ==K"="
1630+
elseif k ==K"kw"|| (eq_is_kw && k ==K"=")
1631+
# syntax TODO: This should parse to K"kw"
16321632
# x = a
16331633
ifkind(kw[1]) !=K"Identifier"&&kind(kw[1]) !=K"Placeholder"
16341634
throw(LoweringError(kw[1], "invalid $field_name name"))
@@ -1696,7 +1696,7 @@ end
16961696
functionexpand_kw_call(ctx, srcref, farg, args, kws)
16971697
@ast ctx srcref [K"block"
16981698
func := farg
1699-
kw_container :=expand_named_tuple(ctx, srcref, kws;
1699+
kw_container :=expand_named_tuple(ctx, srcref, kws, false;
17001700
field_name="keyword argument",
17011701
element_name="keyword argument")
17021702
ifall(kind(kw) ==K"..."for kw in kws)
@@ -1869,7 +1869,7 @@ function remove_kw_args!(ctx, args::SyntaxList)
18691869
for i in1:length(args)
18701870
arg = args[i]
18711871
k =kind(arg)
1872-
if k ==K"="
1872+
if k ==K"kw"
18731873
ifisnothing(kws)
18741874
kws =SyntaxList(ctx)
18751875
end
@@ -2277,7 +2277,7 @@ end
22772277
functionexpand_function_arg(ctx, body_stmts, arg, is_last_arg, is_kw, arg_id)
22782278
ex = arg
22792279

2280-
ifkind(ex) ==K"="
2280+
ifkind(ex) ==K"kw"
22812281
default = ex[2]
22822282
ex = ex[1]
22832283
else
@@ -2858,8 +2858,10 @@ function keyword_function_defs(ctx, srcref, callex_srcref, name_str, typevar_nam
28582858
kwcall_body_tail
28592859
]
28602860
else
2861-
scope_nest(ctx, kw_names, kw_values, kwcall_body_tail)
2861+
scope_nest(ctx, has_kw_slurp ? kw_names[1:end-1] : kw_names,
2862+
kw_values, kwcall_body_tail)
28622863
end
2864+
28632865
main_kwcall_typevars =trim_used_typevars(ctx, kwcall_arg_types, typevar_names, typevar_stmts)
28642866
push!(kwcall_method_defs,
28652867
method_def_expr(ctx, srcref, callex_srcref, kwcall_mtable,
@@ -3233,6 +3235,29 @@ end
32333235

32343236
#-------------------------------------------------------------------------------
32353237
# Anon function syntax
3238+
functionexpand_arrow_args(ctx, arglist)
3239+
k =kind(arglist)
3240+
# The arglist can sometimes be parsed as a block, or something else, and
3241+
# fixing this is extremely awkward when nested inside `where`. See
3242+
# https://github.com/JuliaLang/JuliaSyntax.jl/pull/522
3243+
if k ==K"block"
3244+
@chknumchildren(arglist) ==2
3245+
kw = arglist[2]
3246+
ifkind(kw) ===K"="
3247+
kw =@ast ctx kw [K"kw"children(kw)...]
3248+
end
3249+
arglist =@ast ctx arglist [K"tuple"
3250+
arglist[1]
3251+
[K"parameters" kw]
3252+
]
3253+
elseif k !=K"tuple"
3254+
arglist =@ast ctx arglist [K"tuple" arglist]
3255+
end
3256+
returnmapchildren(ctx, arglist) do a
3257+
kind(a) ===K"="?@ast(ctx, a, [K"kw"children(a)...]) : a
3258+
end
3259+
end
3260+
32363261
functionexpand_arrow_arglist(ctx, arglist, arrowname)
32373262
k =kind(arglist)
32383263
if k ==K"where"
@@ -3241,23 +3266,9 @@ function expand_arrow_arglist(ctx, arglist, arrowname)
32413266
arglist[2]
32423267
]
32433268
else
3244-
# The arglist can sometimes be parsed as a block, or something else, and
3245-
# fixing this is extremely awkward when nested inside `where`. See
3246-
# https://github.com/JuliaLang/JuliaSyntax.jl/pull/522
3247-
if k ==K"block"
3248-
@chknumchildren(arglist) ==2
3249-
arglist =@ast ctx arglist [K"tuple"
3250-
arglist[1]
3251-
[K"parameters" arglist[2]]
3252-
]
3253-
elseif k !=K"tuple"
3254-
arglist =@ast ctx arglist [K"tuple"
3255-
arglist[1]
3256-
]
3257-
end
32583269
@ast ctx arglist [K"call"
32593270
arrowname::K"Placeholder"
3260-
children(arglist)...
3271+
children(expand_arrow_args(ctx, arglist))...
32613272
]
32623273
end
32633274
end
@@ -3280,7 +3291,7 @@ function expand_opaque_closure(ctx, ex)
32803291
func_expr = ex[5]
32813292
@chkkind(func_expr) ==K"->"
32823293
@chknumchildren(func_expr) ==2
3283-
args = func_expr[1]
3294+
args =expand_arrow_args(ctx, func_expr[1])
32843295
@chkkind(args) ==K"tuple"
32853296
check_no_parameters(ex, args)
32863297

@@ -3829,7 +3840,7 @@ function _rewrite_ctor_new_calls(ctx, ex, struct_name, global_struct_name, ctor_
38293840
)
38303841
end
38313842
# Rewrite a call to new()
3832-
kw_arg_i =findfirst(e->(k =kind(e); k ==K"="|| k ==K"parameters"), children(ex))
3843+
kw_arg_i =findfirst(e->(k =kind(e); k ==K"kw"|| k ==K"parameters"), children(ex))
38333844
if!isnothing(kw_arg_i)
38343845
throw(LoweringError(ex[kw_arg_i], "`new` does not accept keyword arguments"))
38353846
end
@@ -4147,7 +4158,7 @@ function expand_struct_def(ctx, ex, docs)
41474158
struct_name
41484159
isnothing(docs) ?nothing_(ctx, ex) : docs[1]
41494160
::K"SourceLocation"(ex)
4150-
[K"="
4161+
[K"kw"
41514162
"field_docs"::K"Identifier"
41524163
[K"call""svec"::K"core" field_docs...]
41534164
]
@@ -4194,7 +4205,7 @@ end
41944205
functionexpand_curly(ctx, ex)
41954206
@assertkind(ex) ==K"curly"
41964207
check_no_parameters(ex, "unexpected semicolon in type parameter list")
4197-
check_no_assignment(children(ex), "misplace assignment in type parameter list")
4208+
check_no_assignment(children(ex), "misplaced assignment in type parameter list")
41984209

41994210
typevar_stmts =SyntaxList(ctx)
42004211
type_args =SyntaxList(ctx)
@@ -4512,9 +4523,9 @@ function expand_forms_2(ctx::DesugaringContext, ex::SyntaxTree, docs=nothing)
45124523
ifnumchildren(ex) >1
45134524
throw(LoweringError(ex[end], "unexpected semicolon in tuple - use `,` to separate tuple elements"))
45144525
end
4515-
expand_forms_2(ctx, expand_named_tuple(ctx, ex, children(ex[1])))
4526+
expand_forms_2(ctx, expand_named_tuple(ctx, ex, children(ex[1]), true))
45164527
elseifany_assignment(children(ex))
4517-
expand_forms_2(ctx, expand_named_tuple(ctx, ex, children(ex)))
4528+
expand_forms_2(ctx, expand_named_tuple(ctx, ex, children(ex), true))
45184529
else
45194530
expand_forms_2(ctx, @ast ctx ex [K"call"
45204531
"tuple"::K"core"
@@ -4558,7 +4569,7 @@ function expand_forms_2(ctx::DesugaringContext, ex::SyntaxTree, docs=nothing)
45584569
ctx.mod ::K"Value"
45594570
[K"inert" ex]
45604571
[K"parameters"
4561-
[K"="
4572+
[K"kw"
45624573
"expr_compat_mode"::K"Identifier"
45634574
ctx.expr_compat_mode::K"Bool"
45644575
]

‎JuliaLowering/src/hooks.jl‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ function core_lowering_hook(@nospecialize(code), mod::Module,
1515
file = file isa Ptr{UInt8} ?unsafe_string(file) : file
1616
line =!(line isa Int) ?Int(line) : line
1717

18-
local st0=nothing
18+
local st0, st1 =nothing,nothing
1919
try
2020
st0 = code isa Expr ?expr_to_syntaxtree(code, LineNumberNode(line, file)) : code
2121
ifkind(st0) inKSet"toplevel module"
@@ -32,7 +32,7 @@ function core_lowering_hook(@nospecialize(code), mod::Module,
3232
ex =to_lowered_expr(st5)
3333
return Core.svec(ex, st5, ctx5)
3434
catch exc
35-
@info("JuliaLowering threw given input:", code=code, st0=st0, file=file, line=line, mod=mod)
35+
@info("JuliaLowering threw given input:", code=code, st0=st0, st1=st1, file=file, line=line, mod=mod)
3636
rethrow(exc)
3737

3838
#TODO: Re-enable flisp fallback once we're done collecting errors

‎JuliaLowering/src/macro_expansion.jl‎

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

0 commit comments

Comments
(0)