Skip to content

Commit

Permalink
In errorshow UI, filter out frames for _include implementation
Browse files Browse the repository at this point in the history
This simplifies stack traces for include() which were made much more
complex by moving jl_parse_eval_all to the Julia layer.
  • Loading branch information
c42f committed Apr 16, 2020
1 parent f0f535f commit 5add326
Show file tree
Hide file tree
Showing 3 changed files with 52 additions and 11 deletions.
35 changes: 34 additions & 1 deletion base/errorshow.jl
Original file line number Diff line number Diff line change
Expand Up @@ -750,6 +750,39 @@ function is_kw_sorter_name(name::Symbol)
return !startswith(sn, '#') && endswith(sn, "##kw")
end

# For improved user experience, filter out frames for include() implementation
# - see #33065. See also #35371 for extended discussion of internal frames.
function _simplify_include_frames(trace)
kept_frames = trues(length(trace))
i = length(trace)
first_ignored = nothing
while i >= 1
frame,_ = trace[i]
mod = parentmodule(frame)
if isnothing(first_ignored)
if mod in (Base,Core) && frame.func === :_include
# Hide include() machinery by default
first_ignored = i
end
else
# Hack: allow `mod==nothing` as a workaround for inlined functions.
# TODO: Fix this by improving debug info.
if mod in (Base,Core,nothing) && 1+first_ignored-i <= 5
if frame.func == :eval
@debug "Ignoring frames" removed=trace[i:first_ignored]
kept_frames[i:first_ignored] .= false
first_ignored = nothing
end
else
# Bail out
first_ignored = nothing
end
end
i -= 1
end
trace[kept_frames]
end

function process_backtrace(t::Vector, limit::Int=typemax(Int); skipC = true)
n = 0
last_frame = StackTraces.UNKNOWN
Expand Down Expand Up @@ -790,7 +823,7 @@ function process_backtrace(t::Vector, limit::Int=typemax(Int); skipC = true)
if n > 0
push!(ret, (last_frame, n))
end
return ret
return _simplify_include_frames(ret)
end

function show_exception_stack(io::IO, stack::Vector)
Expand Down
1 change: 1 addition & 0 deletions base/loading.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1149,6 +1149,7 @@ Base.include # defined in Base.jl

# Full include() implementation which is used after bootstrap
function _include(mapexpr::Function, mod::Module, _path::AbstractString)
@_noinline_meta # Workaround for module availability in _simplify_include_frames
path, prev = _include_dependency(mod, _path)
for callback in include_callbacks # to preserve order, must come before Core.include
invokelatest(callback, mod, path)
Expand Down
27 changes: 17 additions & 10 deletions base/stacktraces.jl
Original file line number Diff line number Diff line change
Expand Up @@ -268,22 +268,29 @@ function show(io::IO, frame::StackFrame; full_path::Bool=false)
end
end

function Base.parentmodule(frame::StackFrame)
if frame.linfo isa MethodInstance
def = frame.linfo.def
if def isa Module
return def
else
@assert def isa Method
return def.module
end
else
# Bug: currently the module is not available for inlined frames and
# frames arising from the interpreter.
nothing
end
end

"""
from(frame::StackFrame, filter_mod::Module) -> Bool
Returns whether the `frame` is from the provided `Module`
"""
function from(frame::StackFrame, m::Module)
finfo = frame.linfo
result = false

if finfo isa MethodInstance
frame_m = finfo.def
isa(frame_m, Method) && (frame_m = frame_m.module)
result = nameof(frame_m) === nameof(m)
end

return result
nameof(parentmodule(frame)) == nameof(m)
end

end

0 comments on commit 5add326

Please sign in to comment.