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

bug in dispatch #18985

Closed
CarloLucibello opened this issue Oct 17, 2016 · 8 comments
Closed

bug in dispatch #18985

CarloLucibello opened this issue Oct 17, 2016 · 8 comments
Labels
types and dispatch Types, subtyping and method dispatch

Comments

@CarloLucibello
Copy link
Contributor

CarloLucibello commented Oct 17, 2016

The following code works correctly

 julia> f{T<:Number}(x::T, y...) = (println(length(y)); f(y[1], y[2:end]...))
f (generic function with 1 method)

julia> f{T<:Number}(x::T) = "hi"
f (generic function with 2 methods)

julia> f(1,rand(10)...)
10
9
8
7
6
5
4
3
2
1
"hi"

But it fails when I change to <:Union the parametrization

julia> f{T<:Union{Float32,Float64}}(x::T, y...) = (println(length(y)); f(y[1], y[2:end]...))
f (generic function with 1 method)

julia> f{T<:Union{Float32,Float64}}(x::T) = "hi"
f (generic function with 2 methods)

julia> f(1.,rand(10)...)
10
9
8
7
6
5
4
3
2
1
0
ERROR: BoundsError: attempt to access ()
  at index [1]
 in f(::Float64) at ./REPL[1]:1 (repeats 11 times)
 in eval_user_input(::Any, ::Base.REPL.REPLBackend) at ./REPL.jl:64
 in macro expansion at ./REPL.jl:95 [inlined]
 in (::Base.REPL.##3#4{Base.REPL.REPLBackend})() at ./event.jl:68

At the last step we have a wrong dispatch to the first method instead of the second

Bye,
Carlo

@nalimilan
Copy link
Member

That's because y... can match zero arguments too. Change the first function to this (which is actually more natural here):

f{T<:Union{Float32,Float64}}(x::T, y, z...) = (println(length(z)+1); f(y, z...))

@nalimilan
Copy link
Member

That said, a redefinition warning should probably be printed in that case.

@nalimilan nalimilan reopened this Oct 17, 2016
@nalimilan nalimilan changed the title bug in dispatch Missing redefinition warning when method with varargs shadows another one Oct 17, 2016
@CarloLucibello
Copy link
Contributor Author

isn't the vararg signature less specialized than the other one, so that f(x) should always be preferred to f(x, y...)?

Also, the fact that T<:Number works and T<:Union is clearly an incoherent behaviour (bug?)

@Gnimuc
Copy link
Contributor

Gnimuc commented Oct 17, 2016

I'm curious: what's the subtype of Union{Float32,Float64}? is it necessary to use parametric method here? Why not just write f(x::Union{Float32,Float64}, y...) = (println(length(y)); f(y[1], y[2:end]...))?

FYI, using typealias will suppress the error:

julia> typealias F32F64 Union{Float32, Float64}
Union{Float32,Float64}

julia> f{T<:F32F64}(x::T) = "hi"
f (generic function with 1 method)

julia> f{T<:F32F64}(x::T, y...) = (println(length(y)); f(y[1], y[2:end]...))
f (generic function with 2 methods)

julia> f(1.,rand(10)...)
10
9
8
7
6
5
4
3
2
1
"hi"

@yuyichao yuyichao changed the title Missing redefinition warning when method with varargs shadows another one bug in dispatch Oct 17, 2016
@yuyichao
Copy link
Contributor

I think this is a dispatch error.

This is different from the default argument case and there are only two methods and the second one should always be more specific than the first one. Of course, defining it the way @nalimilan mentioned above is still preferred since it's easier to handle for the compiler and is less likely to cause ambiguity when there's other methods.

@CarloLucibello
Copy link
Contributor Author

just to point to a real case scenario, I encountered this issue here JuliaCollections/Iterators.jl#85 while trying to define iteratorsize for chained iterators

@JeffBezanson JeffBezanson added the types and dispatch Types, subtyping and method dispatch label Oct 17, 2016
@JeffBezanson
Copy link
Member

I believe this is fixed by #18457 - @CarloLucibello can you confirm?

@CarloLucibello
Copy link
Contributor Author

CarloLucibello commented Jan 16, 2017

It's working now, nice! @JeffBezanson

julia> f{T<:Union{Float32,Float64}}(x::T, y...) = (println(length(y)); f(y[1], y[2:end]...))
f (generic function with 1 method)

julia> f{T<:Union{Float32,Float64}}(x::T) = "hi"
f (generic function with 2 methods)

julia> f(1.,rand(10)...)
10
9
8
7
6
5
4
3
2
1
"hi"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
types and dispatch Types, subtyping and method dispatch
Projects
None yet
Development

No branches or pull requests

5 participants