-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
drop dispatch rule that all method type parameters must be captured #21026
Comments
Hmm... actually, I can make the latter case work like I originally envisaged it working by introducing a dummy type which doesn't match, e.g. abstract type dummy end
g(::Union{Symbol,T,dummy}) where T = T
g(:x) # gives Symbol Anyone understand what's going on? |
Are you sure you didn't have another method defining already? |
Ah sorry you're right, disregard that, I had some other definition floating around that caused that. |
My and @vtjnash 's current thinking is that it would be simpler to drop the method-type-parameters-must-match rule, and make dispatch-equality the same as type-equality. The only reason for that extra rule was that you can access the values of parameters inside the method body, and if values for them can't be determined it's not so clear what to do. However, that should only affect uses of those variables and not dispatch itself. Best would probably be to give a specific kind of error when such a parameter is accessed. If you don't use the parameter names inside the method, everything is perfect. Or, you can write code like this:
and that will also work. |
I've tried to do some analysis of this. Modulo missing coverage of cases in my code, I think the following 10 methods are the full extent to which Base dispatch will get affected by this change: _absspvec_hcat(X::AbstractSparseArray{Tv, Ti, 1}...) where {Tv, Ti} in Base.SparseArrays at sparse/sparsevector.jl:874
_absspvec_vcat(X::AbstractSparseArray{Tv, Ti, 1}...) where {Tv, Ti} in Base.SparseArrays at sparse/sparsevector.jl:914
_totuple(::Type{Tuple{Vararg{E, N} where N}}, itr, s) where E in Base at tuple.jl:235
byteenv(env::Union{AbstractArray{Pair{T, B} where B, 1}, Tuple{Vararg{Pair{T, B} where B, N} where N}}) where T<:AbstractString in Base at process.jl:211
cat(catdims, A::AbstractArray{T, N} where N...) where T in Base at abstractarray.jl:1362
cat(catdims, xs::Union{Array{T, 1}, Array{T, 2}, Base.LinAlg.AbstractTriangular{T, A} where A<:(Array{T, 2} where T), Hermitian{T, A} where A<:(Array{T, 2} where T), Symmetric{T, A} where A<:(Array{T, 2} where T)}...) where T in Base.SparseArrays at sparse/sparsevector.jl:1016
convert(::Type{Tuple{Vararg{T, N} where N}}, x::Tuple) where T in Base at essentials.jl:69
eltype(::Type{#s48} where #s48<:Tuple{Vararg{E, N} where N}) where E in Base at tuple.jl:67
promote_leaf_eltypes(x::Union{AbstractArray{T, N} where N, Tuple{Vararg{T, N} where N}}) where T<:(AbstractArray{T, N} where N where T<:Number) in Base.LinAlg at linalg/generic.jl:1290
promote_leaf_eltypes(x::Union{AbstractArray{T, N} where N, Tuple{Vararg{T, N} where N}}) where T<:Number in Base.LinAlg at linalg/generic.jl:1289 Although there's some type system bugs which may cause other errors:
But that bug exists independent of the change proposed here however, so it's not in scope to detect these cases. |
Fixed by #23117 |
I think this is far easier for people to understand: # JULIA 0.7-DEV
julia> f(::Number...) = "heterogenous"
f (generic function with 2 methods)
julia> f(::T...) where T<:Number = "homogenous"
f (generic function with 1 method)
julia> f()
"homogenous" Than the old behavior: # JULIA 0.6
julia> f(::Number...) = "heterogenous"
f (generic function with 2 methods)
julia> f(::T...) where T<:Number = "homogenous"
f (generic function with 1 method)
julia> f()
"heterogenous" |
See the linked StaticArrays issue above, referring to After this change, what is the right way to define a vararg method for which at least one of the arguments must be of a given type? |
I'm curious about this too. I had been using the fact that all arguments must match as a "feature" and will need to do some rethinking in 0.7. In fact, for this reason I would suggest the "news item" in NEWS.md for this change be moved to "breaking changes." Perhaps if there's a simple succinct response to the above question, it even makes sense to include it in the new item as a suggested workaround. |
Hmm, since my last comment I've learned that the method used by StaticArrays is more fragile than I originally thought, already in 0.6: f(x::Number...) = "base"
f(x::Union{T, Number}...) where {T<:Int} = "specialization" causes the first The reason this works at all in StaticArrays in 0.6 is that the equivalent of the first f(x...) = "base"
f(x::Union{T, Number}...) where {T<:Int} = "specialization" Still, in 0.6, |
We can only express "at least one argument is a StaticArray" if the other argument types aren't supertypes of StaticArray. We would need to add an "existential Vararg" type for this. That could be a neat feature; |
That could be pretty neat indeed. Not required for this particular case, and perhaps going off topic, but I do wonder if at some point the full generality of something like C++'s |
IIUC C++'s metaprogramming ( |
This is interesting. In 0.6 we used the "feature" which was removed here to support dispatching to the map(+, [1,2,3], SVector(1,2,3), [1,3,4], [1,2,3], [1,2,3]) I'm not sure whether this was actually a good idea, but it seemed to work! If there's no other way to do this now in 0.7, we'll have to fall back to writing out several of the possible combinations for low numbers of arguments, and falling back to generic |
Though I guess another option for the particular case of |
It may be obvious that methods will only dispatch if all type parameters are captured, but based on my readings through the docs this was not something that had stuck in my mind (in fact, searching back I can't really find any mention of it, I'm only deducing this from trying various test cases). Anyway, it led to a bit of a hang up which I think could be avoided for others by documenting this more explicitly. In case it's not clear what I mean,
My real issue that led me here was similar to first example, where I wasn't able to figure out why a call wasn't dispatching until I eventually realized there was some lone parameter that wasn't matched for my particular arguments (and it was less obvious that this simplified example).
The text was updated successfully, but these errors were encountered: