Skip to content

Commit

Permalink
Fix show for MethodList when methods are from another module (#52354
Browse files Browse the repository at this point in the history
)

When a type is defined in one module but its methods are defined
elsewhere, `show_method_table` errors due to an incorrect lookup of the
defining module used to determine colors for printing. In particular,
the code had been assuming that the type is defined in the module in
which its constructor's first method (in the sense of
`first(methods())`) is defined, which isn't always true.

The color used for printing the module name needs to be determined on a
per-method basis and can't be correctly done based on the method table's
module. For each method, we attempt to derive the module for the method
table to which the method was added, then determine whether it's the
same as the defining module for the method.

Fixes #49382
Fixes #49403
Fixes #52043

Co-Authored-By: Jameson Nash <[email protected]>
(cherry picked from commit 40bc64c)
  • Loading branch information
ararslan committed Dec 22, 2023
1 parent b6dd527 commit 2243b8c
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 14 deletions.
37 changes: 24 additions & 13 deletions base/methodshow.jl
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,29 @@ function show_method_list_header(io::IO, ms::MethodList, namefmt::Function)
!iszero(n) && print(io, ":")
end

# Determine the `modulecolor` value to pass to `show_method`
function _modulecolor(method::Method)
mmt = get_methodtable(method)
if mmt === nothing || mmt.module === parentmodule(method)
return nothing
end
# `mmt` is only particularly relevant for external method tables. Since the primary
# method table is shared, we now need to distinguish "primary" methods by trying to
# check if there is a primary `DataType` to identify it with. c.f. how `jl_method_def`
# would derive this same information (for the name).
ft = argument_datatype((unwrap_unionall(method.sig)::DataType).parameters[1])
# `ft` should be the type associated with the first argument in the method signature.
# If it's `Type`, try to unwrap it again.
if isType(ft)
ft = argument_datatype(ft.parameters[1])
end
if ft === nothing || parentmodule(method) === parentmodule(ft) !== Core
return nothing
end
m = parentmodule_before_main(method)
return get!(() -> popfirst!(STACKTRACE_MODULECOLORS), STACKTRACE_FIXEDCOLORS, m)
end

function show_method_table(io::IO, ms::MethodList, max::Int=-1, header::Bool=true)
mt = ms.mt
name = mt.name
Expand All @@ -300,12 +323,6 @@ function show_method_table(io::IO, ms::MethodList, max::Int=-1, header::Bool=tru
last_shown_line_infos = get(io, :last_shown_line_infos, nothing)
last_shown_line_infos === nothing || empty!(last_shown_line_infos)

modul = if mt === _TYPE_NAME.mt && length(ms) > 0 # type constructor
which(ms.ms[1].module, ms.ms[1].name)
else
mt.module
end

digit_align_width = length(string(max > 0 ? max : length(ms)))

for meth in ms
Expand All @@ -315,13 +332,7 @@ function show_method_table(io::IO, ms::MethodList, max::Int=-1, header::Bool=tru

print(io, " ", lpad("[$n]", digit_align_width + 2), " ")

modulecolor = if parentmodule(meth) == modul
nothing
else
m = parentmodule_before_main(meth)
get!(() -> popfirst!(STACKTRACE_MODULECOLORS), STACKTRACE_FIXEDCOLORS, m)
end
show_method(io, meth; modulecolor)
show_method(io, meth; modulecolor=_modulecolor(meth))

file, line = updated_methodloc(meth)
if last_shown_line_infos !== nothing
Expand Down
6 changes: 5 additions & 1 deletion base/reflection.jl
Original file line number Diff line number Diff line change
Expand Up @@ -2033,7 +2033,11 @@ function delete_method(m::Method)
end

function get_methodtable(m::Method)
return ccall(:jl_method_get_table, Any, (Any,), m)::Core.MethodTable
mt = ccall(:jl_method_get_table, Any, (Any,), m)
if mt === nothing
return nothing
end
return mt::Core.MethodTable
end

"""
Expand Down
7 changes: 7 additions & 0 deletions test/show.jl
Original file line number Diff line number Diff line change
Expand Up @@ -2630,3 +2630,10 @@ end
ir = Core.Compiler.complete(compact)
verify_display(ir)
end

module Issue49382
abstract type Type49382 end
end
using .Issue49382
(::Type{Issue49382.Type49382})() = 1
@test sprint(show, methods(Issue49382.Type49382)) isa String

0 comments on commit 2243b8c

Please sign in to comment.