Skip to content

Commit

Permalink
Resolve varargs calls recursively
Browse files Browse the repository at this point in the history
These were being "hidden" inside a Core._apply call.
This revealed some other cases that require one runs in Compiled mode.
  • Loading branch information
timholy committed Mar 12, 2019
1 parent 7d8d038 commit c5c3ec8
Show file tree
Hide file tree
Showing 5 changed files with 42 additions and 6 deletions.
2 changes: 2 additions & 0 deletions src/JuliaInterpreter.jl
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ function set_compiled_methods()
push!(compiled_methods, which(flush, Tuple{IOStream}))
push!(compiled_methods, which(disable_sigint, Tuple{Function}))
push!(compiled_methods, which(reenable_sigint, Tuple{Function}))
# Signal-handling in the `print` dispatch hierarchy
push!(compiled_methods, which(Base.unsafe_write, Tuple{Base.LibuvStream, Ptr{UInt8}, UInt}))
end

function __init__()
Expand Down
22 changes: 19 additions & 3 deletions src/generate_builtins.jl
Original file line number Diff line number Diff line change
Expand Up @@ -82,15 +82,16 @@ function getargs(args, frame)
end
\"\"\"
ret = maybe_evaluate_builtin(frame, call_expr)
ret = maybe_evaluate_builtin(frame, call_expr, expand::Bool)
If `call_expr` is to a builtin function, evaluate it, returning the result inside a `Some` wrapper.
Otherwise, return `call_expr`.
If `expand` is true, `Core._apply` calls will be resolved as a call to the applied function.
\"\"\"
function maybe_evaluate_builtin(frame, call_expr)
function maybe_evaluate_builtin(frame, call_expr, expand::Bool)
# By having each call appearing statically in the "switch" block below,
# each gets call-site optimized.
args = call_expr.args
nargs = length(args) - 1
fex = args[1]
Expand Down Expand Up @@ -122,9 +123,24 @@ function maybe_evaluate_builtin(frame, call_expr)
"""
$head f === tuple
return Some{Any}(ntuple(i->@lookup(frame, args[i+1]), length(args)-1))
""")
continue
elseif f === Core._apply
# Resolve varargs calls
print(io,
"""
$head f === Core._apply
argswrapped = getargs(args, frame)
if !expand
return Some{Any}(Core._apply(getargs(args, frame)...))
end
argsflat = Base.append_any((argswrapped[1],), argswrapped[2:end]...)
new_expr = Expr(:call, map(x->isa(x, Symbol) || isa(x, Expr) || isa(x, QuoteNode) ? QuoteNode(x) : x, argsflat)...)
return new_expr
""")
continue
end

id = findfirst(isequal(f), Core.Compiler.T_FFUNC_KEY)
fcall = generate_fcall(f, Core.Compiler.T_FFUNC_VAL, id)
print(io,
Expand Down
5 changes: 3 additions & 2 deletions src/interpret.jl
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ function evaluate_call_compiled!(::Compiled, frame::Frame, call_expr::Expr; ente
pc = frame.pc
ret = bypass_builtins(frame, call_expr, pc)
isa(ret, Some{Any}) && return ret.value
ret = maybe_evaluate_builtin(frame, call_expr)
ret = maybe_evaluate_builtin(frame, call_expr, false)
isa(ret, Some{Any}) && return ret.value
fargs = collect_args(frame, call_expr)
f = fargs[1]
Expand All @@ -190,8 +190,9 @@ function evaluate_call_recurse!(@nospecialize(recurse), frame::Frame, call_expr:
pc = frame.pc
ret = bypass_builtins(frame, call_expr, pc)
isa(ret, Some{Any}) && return ret.value
ret = maybe_evaluate_builtin(frame, call_expr)
ret = maybe_evaluate_builtin(frame, call_expr, true)
isa(ret, Some{Any}) && return ret.value
call_expr = ret
fargs = collect_args(frame, call_expr)
if (f = fargs[1]) === Core.eval
return Core.eval(fargs[2], fargs[3]) # not a builtin, but worth treating specially
Expand Down
2 changes: 1 addition & 1 deletion src/precompile.jl
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
function _precompile_()
ccall(:jl_generating_output, Cint, ()) == 1 || return nothing
precompile(Tuple{typeof(maybe_evaluate_builtin), Frame, Expr})
precompile(Tuple{typeof(maybe_evaluate_builtin), Frame, Expr, Bool})
precompile(Tuple{typeof(getargs), Vector{Any}, Frame})
precompile(Tuple{typeof(get_call_framecode), Vector{Any}, FrameCode, Int})
for f in (evaluate_call!,
Expand Down
17 changes: 17 additions & 0 deletions test/debug.jl
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,23 @@ end
@test get_return(frame) == 3
end

@testset "Varargs" begin
f_va_inner(x) = x + 1
f_va_outer(args...) = f_va_inner(args...)
frame = fr = JuliaInterpreter.enter_call(f_va_outer, 1)
# depending on whether this is in or out of a @testset, the first statement may differ
stmt1 = fr.framecode.src.code[1]
if isexpr(stmt1, :call) && @lookup(frame, stmt1.args[1]) === getfield
fr, pc = debug_command(fr, "se")
end
fr, pc = debug_command(fr, "s")
fr, pc = debug_command(fr, "n")
@test root(fr) !== fr
fr, pc = debug_command(fr, "finish")
@test debug_command(fr, "finish") === nothing
@test get_return(frame) === 2
end

@testset "Exceptions" begin
# Don't break on caught exceptions
err_caught = Any[nothing]
Expand Down

2 comments on commit c5c3ec8

@KristofferC
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Argh, I was actually thinking about this exact thing when looking at the stacktrace in #113 (comment) and saw that plot was directly called with compiled mode. And then somehow I pushed it aside in my head. Great that you managed to find it.

@timholy
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We might need to use similar treatment for Core._apply_latest, Core._apply_pure, and possibly others. I just haven't had time to figure what they do so I can mimic them. TBH I'm still not entirely sure about the logic behind Core._apply, it's just this is what it seems to do.

Please sign in to comment.