Skip to content

Commit

Permalink
Fix show for MethodList when methods are from another module
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]>
  • Loading branch information
ararslan and vtjnash committed Dec 21, 2023
1 parent c1e1d5c commit ba08ba6
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 13 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.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
7 changes: 7 additions & 0 deletions test/show.jl
Original file line number Diff line number Diff line change
Expand Up @@ -2652,3 +2652,10 @@ let buf = IOBuffer()
Base.show_tuple_as_call(buf, Symbol(""), Tuple{Function,Any})
@test String(take!(buf)) == "(::Function)(::Any)"
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 ba08ba6

Please sign in to comment.