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

add hasmethod_n intrinsic to check for the existance of n argument methods for a function. #56279

Open
oscardssmith opened this issue Oct 21, 2024 · 12 comments
Labels
regression 1.10 Regression in the 1.10 release

Comments

@oscardssmith
Copy link
Member

oscardssmith commented Oct 21, 2024

In 1.9: it was possible to qurery the method table to see whether a given function has a 2 argument method: e.g.

julia> g(u::Float64,p) = u

julia> hasmethod(g, Tuple{Int, Int})
false

julia> hasmethod(g, Tuple{Any, Any})
false

julia> hasmethod(g, Tuple{Union{}, Union{}})
true

However on 1.10, we can't express Tuple{Union{}, Union{}} so we can't express it like this (instead you have to do something like searching through all methods which can't be done at compile-time.

This is closely related to #52385, but IMO is sufficiently different to be worth separating into a separate issue.

@oscardssmith oscardssmith added the regression 1.10 Regression in the 1.10 release label Oct 21, 2024
@vtjnash
Copy link
Member

vtjnash commented Oct 21, 2024

julia> hasmethod(g, Tuple{Union{}, Union{}})
true

This result seems like bug: it probably should have returned false, since Julia may assume that the method list queries obeys subtyping transitivity, while that method cannot be called / does not exist for all supertypes of the input query. That is a bug in v1.9 though, and gets fixed (not a regression) in v1.10.

@topolarity
Copy link
Member

that method cannot be called / does not exist for all supertypes of the input query

What supertype can it not be called for?

@nsajko
Copy link
Contributor

nsajko commented Oct 21, 2024

What supertype can it not be called for?

Tuple{Int, Int}:

  • is a supertype of Tuple{Union{}, Union{}}
  • isn't associated with a method of g

Not that I understand what's going on here, though.

@topolarity
Copy link
Member

But that's also true for:

julia> g(::Union{Int,Float64}) = 13
julia> hasmethod(g, Tuple{Int})
true

Tuple{Int,Float32} is a supertype of Tuple{Int} and isn't associated with any method of g

I thought that hasmethod is supposed to return true iff the argtype provided is completely covered by any method definition.
(IIUC, that's what @oscardssmith is expecting here too.)

@topolarity
Copy link
Member

Is the disagreement about whether Tuple{Float64, Any} is a supertype of Tuple{Union{},Union{}}?

@vtjnash
Copy link
Member

vtjnash commented Oct 22, 2024

Ah right. Maybe the issue is that it may return false because it determines is ambiguous which method to (not) call, so it would be wrong to ever use as an approximation of whether a method exists with that number of arguments?

@vtjnash
Copy link
Member

vtjnash commented Oct 22, 2024

true iff the argtype provided is completely covered by any method definition

Covered by exactly one method is the actual definition of this query, since that is the definition used by invoke, and this does the same query as invoke during runtime (during inference, it does the same query also as invoke, including first discarding the possibility of any parameter being Union{})

@oscardssmith
Copy link
Member Author

One resolution for this would be a new hasmethod_n_args(n) intrinsic. For anyone interested, the reason I want this capability is to be able to write https://github.com/SciML/SciMLBase.jl/blob/3c1211ae5cc030dbd0d991f0ce26fe7521b904d1/src/utils.jl#L246 in a better (preferably compile time-executable) way.

@topolarity
Copy link
Member

Covered by exactly one method is the actual definition of this query, since that is the definition used by invoke, and this does the same query as invoke during runtime

Right, I see - this is emulating invoke so for inhabited types hasmethod clearly must be true if there is a most-specific, covering method

I don't think inhabited-ness is doing any work in that definition though TBH. The query over uninhabited types still seems reasonable to me (is there a most-specific, covering method) - I don't see where we discard any uninhabited parameter types?

Of course, that's assuming that #54792 lands one day and not all uninhabited types are type-equal to Union{} because otherwise this query is especially useless (it just checks whether the function has 1 method)

Ah right. Maybe the issue is that it may return false because it determines is ambiguous which method to (not) call, so it would be wrong to ever use as an approximation of whether a method exists with that number of arguments?

That's true - it seems like you'd want to throw out specificity and just check for covering methods for the query @oscardssmith wants

@oscardssmith
Copy link
Member Author

oscardssmith commented Oct 22, 2024

With the following example, I do think a hasmethod type version of this won't work well.

julia> g(u::Float64,p) = u
g (generic function with 1 method)

julia> g(u::Int,p) = u
g (generic function with 2 methods)

julia> hasmethod(g, Tuple{Union{}, Union{}}) #false because ambiguous
false

@bvdmitri
Copy link
Contributor

One resolution for this would be a new hasmethod_n_args(n) intrinsic.

This approach would be much clearer, as hasmethod(g, Tuple{Union{}, Union{}}) is quite obscure for someone unfamiliar with the underlying mechanics (like me). Just by looking at it, it's not intuitive that this checks for a method with two arguments and code just smells. It feels "hacky" at best. IMO most users would find this confusing (could be wrong though), since you need to understand the semantics around Union{}, its relationship to other types, and its interaction with Tuple{}. A cleaner alternative could be indeed a separate function or even maybe smth like hasmethod_n_args(g, Val(2)) if you need this check at compile time.

@JeffBezanson
Copy link
Member

Should this issue be renamed to a request for hasmethod_n?

@oscardssmith oscardssmith changed the title disallowing Tuple{Union{}} makes hasmethod less powerful. add hasmethod_n intrinsic to check for the existance of n argument methods for a function. Nov 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
regression 1.10 Regression in the 1.10 release
Projects
None yet
Development

No branches or pull requests

6 participants