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

[reflection] methods(show, ...) error ? #38249

Closed
o314 opened this issue Oct 31, 2020 · 7 comments
Closed

[reflection] methods(show, ...) error ? #38249

o314 opened this issue Oct 31, 2020 · 7 comments
Labels
docs This change adds or pertains to documentation

Comments

@o314
Copy link
Contributor

o314 commented Oct 31, 2020

f,t = show, Tuple{IO,Vararg{Any}} # find all show with IO as first arg, whatever after
m = methods(f, t) |> collect |> last # take last one

@show m # will display "m = show(x) in Base at show.jl:325" @ julia v1.3.1

@test_broken Base.tuple_type_tail(m.sig).parameters[1] <: IO # IO is not the first arg
@test_broken Base.tuple_type_tail(m.sig) <: t

Doc is very sparse. Code too.
It seems to me it's a bug

@timholy
Copy link
Sponsor Member

timholy commented Oct 31, 2020

I don't think this is a bug. The method show(::Any) is applicable for show(::IO):

julia> io = IOBuffer();

julia> m = @which show(io)                           # Julia 1.6
show(x) in Base at show.jl:392

julia> m.sig
Tuple{typeof(show), Any}

To me the docs seem clear, but that's not a fair test since I've spent time working on this code. It would be awesome if you were to submit a PR that explained this in a way that would be clear to you; we can then edit any mistakes and make it less confusing for everyone.

@timholy timholy added the docs This change adds or pertains to documentation label Oct 31, 2020
@o314
Copy link
Contributor Author

o314 commented Oct 31, 2020

To me it's still surprising, due to the fact that

using Test
type_morespecific(a, b) = ccall(:jl_type_morespecific, Cint, (Any,Any), a, b)!=0

f,t = show, Tuple{IO,Vararg{Any}} # find all show with IO as first arg, whatever after
ms = methods(f, t) |> collect;
@test length(bms) > 200

bms = [type_morespecific(Base.tuple_type_tail(m.sig), Tuple{IO,Vararg{Any}}) for m in ms];
@test length(filter(==(false),bms)) == 1

It's the only one that breaks the type_morespecific order, and goes against my expectation.
I may be not the best person to propose a better explanation in doc for now :(

@vtjnash
Copy link
Sponsor Member

vtjnash commented Oct 31, 2020

It's the only one that breaks the type_morespecific order

Indeed, this fact (that up to one method will be a supertype) is guaranteed by construction, so your observation is fairly astute.

@timholy
Copy link
Sponsor Member

timholy commented Nov 1, 2020

I think I'm beginning to guess that the problematic part is the definition of "match". Does this help?

julia> abstract type Outer end

julia> abstract type Middle<:Outer end

julia> struct Inner<:Middle end

julia> f(::Middle) = 1
f (generic function with 1 method)

julia> type_morespecific(a, b) = ccall(:jl_type_morespecific, Cint, (Any,Any), a, b)!=0
type_morespecific (generic function with 1 method)

julia> ms = methods(f, Tuple{Middle})
# 1 method for generic function "f":
[1] f(::Middle) in Main at REPL[4]:1

julia> [type_morespecific(Base.tuple_type_tail(m.sig), Tuple{Middle}) for m in ms]
1-element Vector{Bool}:
 0

julia> ms = methods(f, Tuple{Outer})
# 1 method for generic function "f":
[1] f(::Middle) in Main at REPL[4]:1

julia> [type_morespecific(Base.tuple_type_tail(m.sig), Tuple{Outer}) for m in ms]
1-element Vector{Bool}:
 1

julia> ms = methods(f, Tuple{Inner})
# 1 method for generic function "f":
[1] f(::Middle) in Main at REPL[4]:1

julia> [type_morespecific(Base.tuple_type_tail(m.sig), Tuple{Inner}) for m in ms]
1-element Vector{Bool}:
 0

@o314
Copy link
Contributor Author

o314 commented Nov 2, 2020

Effectively, it's a subtype query, not an assertional def. There could be a super method.

My point was may be more, effectively, that they must be only a few - Jameson say even unique if any.
I was doing some repackaging work, and i guess those methods play an import role there, when one has to build interface between different large codebases.

It would be interesting to do a population study, above for example show of the ones that do break this order.
That may be a good source of test for "Interfaces for Abstract Types" #6975

@o314
Copy link
Contributor Author

o314 commented Nov 3, 2020

Another point that may seem strange at edge case :

using Test

lt_type(a, b) = ccall(:jl_type_morespecific, Cint, (Any,Any), a, b)!=0
lt_methods(f, sig) =
    [m for m in methods(f, sig) if lt_type(Base.tuple_type_tail(m.sig), sig)]
not_lt_methods(f, sig) =
    [m for m in methods(f, sig) if !lt_type(Base.tuple_type_tail(m.sig), sig)]


f(::Number, ::Number) = 0
f(::Number, ::Integer) = 1
f(::Integer, ::Number) = 2
f(::Int, ::Number) = 10
f(::Number, ::Int) = 20
f(::Integer, ::Integer) = 100

@test_throws MethodError f(1::Int,1::Int) # ambiguous, 4 candidates match
@test methods(f, Tuple{Int,Int}) |> isempty # no result
@test lt_methods(f, Tuple{Int,Int}) |> isempty
# not_lt_methods(f, Tuple{Int,Int})

f called on ::Int, ::Int will lead to 4 matches, and then an ambiguity error,
whereas if one queries those methods via methods an empty set is returned. hasmethod behaves identically.

May be it should be better explained in docs, or does this need a change.

@o314
Copy link
Contributor Author

o314 commented Dec 22, 2020

Hello Mr.s @timholy , @vtjnash maybe too
Feel free to close this issue if needed .
Keeping the issues count low is a good thing :)

@vtjnash vtjnash closed this as completed Dec 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs This change adds or pertains to documentation
Projects
None yet
Development

No branches or pull requests

3 participants