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 May 12, 2020
1 parent f60aefe commit 3c131bf
Show file tree
Hide file tree
Showing 4 changed files with 64 additions and 11 deletions.
37 changes: 36 additions & 1 deletion base/errorshow.jl
Original file line number Diff line number Diff line change
Expand Up @@ -681,6 +681,41 @@ 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)
i = length(trace)
kept_frames = trues(i)
first_ignored = nothing
while i >= 1
frame, _ = trace[i]
mod = parentmodule(frame)
if isnothing(first_ignored)
if mod === Base && 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
kept_frames[i:first_ignored] .= false
first_ignored = nothing
end
else
# Bail out to avoid hiding frames in unexpected circumstances
first_ignored = nothing
end
end
i -= 1
end
if !isnothing(first_ignored)
kept_frames[i:first_ignored] .= false
end
return 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 @@ -721,7 +756,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
26 changes: 16 additions & 10 deletions base/stacktraces.jl
Original file line number Diff line number Diff line change
Expand Up @@ -268,22 +268,28 @@ 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
return (def::Method).module
end
else
# The module is not always available (common reasons include 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
return parentmodule(frame) === m
end

end
11 changes: 11 additions & 0 deletions test/errorshow.jl
Original file line number Diff line number Diff line change
Expand Up @@ -704,3 +704,14 @@ for (func,str) in ((TestMethodShadow.:+,":+"), (TestMethodShadow.:(==),":(==)"),
end::MethodError
@test occursin("You may have intended to import Base.$str", sprint(Base.showerror, ex))
end

# Test that implementation detail of include() is hidden from the user by default
let bt = try
include("testhelpers/include_error.jl")
catch
catch_backtrace()
end
bt_str = sprint(Base.show_backtrace, bt)
@test occursin(" include(", bt_str)
@test !occursin(" _include(", bt_str)
end
1 change: 1 addition & 0 deletions test/testhelpers/include_error.jl
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
error("Expected exception while running include")

0 comments on commit 3c131bf

Please sign in to comment.