Fix show for MethodList when methods are from another module

[ararslan]

When a type is defined in one module but its constructor's 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 sense of isdefined) in the module in which its constructor's first method (in the sense of first(methods())) is defined, which isn't always true.

To fix this, we can look through the available methods and choose the first in which the type is defined in the method's module, falling back to the method table's module otherwise. I'm not sure that this is necessarily the best way to fix the issue but it does fix it.

Fixes #49382
Fixes #49403
Fixes #52043

[aviatesk]

Suggested change

	i = findfirst(m -> isdefined(m.module, m.name), ms.ms)
	i === nothing ? mt.module : which(ms.ms[i].module, ms.ms[i].name)
	sig = Base.unwrap_unionall(ms[1].sig)::DataType # Tuple{Type{...}}
	((sig.parameters[1]::DataType).parameters[1]::DataType).name.module

This seems very hacky, but would be more correct?

[vtjnash]

I would think most of those type asserts are invalid, because the general assumption here (that method tables are owned by modules) is invalid itself

[vtjnash]

Looking over the code a bit closer, it looks like it wants to check it the method itself is being defined on a function / type in the same module or in a dependency. But that needs to be a per-method query, and is not valid to derive from the method table

[ararslan]

I guess the difficulty is defining what "same module" means here since you need to select a starting point for comparison in order to cycle the colors. @aviatesk's suggestion does seem better than what I've implemented, except it uses ms[1] as on current master, the defining module of which isn't necessarily meaningful. But it at least shouldn't error since it isn't using which as on master.

[ararslan]

Combining part of @aviatesk's suggestion with part of what's currently implemented in this PR:

Suggested change

	i = findfirst(m -> isdefined(m.module, m.name), ms.ms)
	i === nothing ? mt.module : which(ms.ms[i].module, ms.ms[i].name)
	i = something(findfirst(m -> isdefined(parentmodule(m), m.name), ms), 1)
	parentmodule(fieldtype(unwrap_unionall(ms[i].sig), 1).parameters[1])

Does this seem reasonable, @vtjnash?

[vtjnash]

It is a bit better. The more generally correct version of this needs to be done for each method however, first attempting to derive the method table it is being added to, then deriving if it is the same as the method itself. Something like this:

mmt = get_methodtable(meth)
modulecolor = if mmt.module === parentmodule(meth)
        nothing
    else
        # 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(meth.sig)::DataType).parameters[1]) # try to get the type associated with the first argument
        isType(ft) && (ft = argument_datatype(ft.parameters[1])) # if the ft is `Type`, try to unwrap it again
        if ft === nothing || parentmodule(meth) === parentmodule(ft) !== Core
            nothing
        else
            # get color for meth
        end
end
...

n.b. In C, the first ft computation is called jl_nth_argument_datatype, but it seems we don't export that to Julia

[aviatesk]

LGTM. I've looked into the discussion and it seems reasonable to me.

[vtjnash]

Why would the method table lookup ever return nothing?

[ararslan]

Why would the method table lookup ever return nothing?

I wondered the same, turns out it does so for builtins. This is actually handled explicitly in a couple of places, e.g.:

julia/base/compiler/utilities.jl

Lines 157 to 158 in 9443c76

	mt = ccall(:jl_method_get_table, Any, (Any,), method)
	mt === nothing && return nothing

Method table lookup uses the first item of the method's signature, but the method for a builtin has signature Tuple, which causes nth_methodtable to return nothing.