Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

hide internally created methods in stacktraces that occur as a result of argument forwarding #49102

Merged
merged 4 commits into from
Mar 23, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 70 additions & 1 deletion base/errorshow.jl
Original file line number Diff line number Diff line change
Expand Up @@ -823,6 +823,73 @@ function _simplify_include_frames(trace)
return trace[kept_frames]
end

# Collapse frames that have the same location (in some cases)
function _collapse_repeated_frames(trace)
kept_frames = trues(length(trace))
last_frame = nothing
for i in 1:length(trace)
frame::StackFrame, _ = trace[i]
if last_frame !== nothing && frame.file == last_frame.file && frame.line == last_frame.line
#=
Handles this case:

f(g, a; kw...) = error();
@inline f(a; kw...) = f(identity, a; kw...);
f(1)

which otherwise ends up as:

[4] #f#4 <-- useless
@ ./REPL[2]:1 [inlined]
[5] f(a::Int64)
@ Main ./REPL[2]:1
=#
if startswith(sprint(show, last_frame), "#")
kept_frames[i-1] = false
end

#= Handles this case
g(x, y=1, z=2) = error();
g(1)

which otherwise ends up as:

[2] g(x::Int64, y::Int64, z::Int64)
@ Main ./REPL[1]:1
[3] g(x::Int64) <-- useless
@ Main ./REPL[1]:1
=#
if frame.linfo isa MethodInstance && last_frame.linfo isa MethodInstance &&
frame.linfo.def isa Method && last_frame.linfo.def isa Method
m, last_m = frame.linfo.def::Method, last_frame.linfo.def::Method
params, last_params = Base.unwrap_unionall(m.sig).parameters, Base.unwrap_unionall(last_m.sig).parameters

if last_m.nkw != 0
pos_sig_params = Base.rewrap_unionall(Tuple{last_params[(last_m.nkw+2):end]...}, last_m.sig).parameters
Copy link
Member

Choose a reason for hiding this comment

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

Seems like you meant just last_params[(last_m.nkw+2):end] here, since the rest doesn't generally make sense

Copy link
Member Author

Choose a reason for hiding this comment

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

I took it from here:

pos_sig = Base.rewrap_unionall(Tuple{uw.parameters[(def.nkw+2):end]...}, sig)

Copy link
Member

Choose a reason for hiding this comment

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

That is useful if you want to do subtyping queries, but it doesn't seem like you wanted to?

Copy link
Member

Choose a reason for hiding this comment

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

You potentially could phrase it as a subtyping query though, by adding Vararg to the shorter one

issame = true
if pos_sig_params == params
kept_frames[i] = false
end
end
if length(last_params) > length(params)
issame = true
for i = 1:length(params)
issame &= params[i] == last_params[i]
end
if issame
kept_frames[i] = false
end
end
end

# TODO: Detect more cases that can be collapsed
end
last_frame = frame
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 @@ -875,7 +942,9 @@ function process_backtrace(t::Vector, limit::Int=typemax(Int); skipC = true)
if n > 0
push!(ret, (last_frame, n))
end
return _simplify_include_frames(ret)
trace = _simplify_include_frames(ret)
trace = _collapse_repeated_frames(trace)
return trace
end

function show_exception_stack(io::IO, stack)
Expand Down
40 changes: 40 additions & 0 deletions test/errorshow.jl
Original file line number Diff line number Diff line change
Expand Up @@ -947,3 +947,43 @@ let buf = IOBuffer()
Base.show_method_candidates(buf, Base.MethodError(isa, ()), pairs((a = 5,)))
@test isempty(take!(buf))
end

f_internal_wrap(g, a; kw...) = error();
@inline f_internal_wrap(a; kw...) = f_internal_wrap(identity, a; kw...);
bt = try
f_internal_wrap(1)
catch
catch_backtrace()
end
@test !occursin("#f_internal_wrap#", sprint(Base.show_backtrace, bt))

g_collapse_pos(x, y=1.0, z=2.0) = error()
bt = try
g_collapse_pos(1.0)
catch
catch_backtrace()
end
bt_str = sprint(Base.show_backtrace, bt)
@test occursin("g_collapse_pos(x::Float64, y::Float64, z::Float64)", bt_str)
@test !occursin("g_collapse_pos(x::Float64)", bt_str)

g_collapse_kw(x; y=2.0) = error()
bt = try
g_collapse_kw(1.0)
catch
catch_backtrace()
end
bt_str = sprint(Base.show_backtrace, bt)
@test occursin("g_collapse_kw(x::Float64; y::Float64)", bt_str)
@test !occursin("g_collapse_kw(x::Float64)", bt_str)

g_collapse_pos_kw(x, y=1.0; z=2.0) = error()
bt = try
g_collapse_pos_kw(1.0)
catch
catch_backtrace()
end
bt_str = sprint(Base.show_backtrace, bt)
@test occursin("g_collapse_pos_kw(x::Float64, y::Float64; z::Float64)", bt_str)
@test !occursin("g_collapse_pos_kw(x::Float64, y::Float64)", bt_str)
@test !occursin("g_collapse_pos_kw(x::Float64)", bt_str)