Misleading warning on ambiguous method definition

[burrowsa]

If I do this:

immutable Foo{T} end
=={T}(lhs::Foo{T},rhs::Foo{T}) = lhs===rhs
=={T}(lhs::Foo{T},rhs::Any) = false
=={T}(lhs::Any,rhs::Foo{T}) = false

I get this warning

Warning: New definition
==(Any,Foo{T}) at none:1
is ambiguous with:
==(Foo{T},Any) at none:1.
To fix, define
==(Foo{T},Foo{T})
before the new definition.

even though I have already defined ==(Foo{T},Foo{T}).

After a trying lots of alternatives it would seem that the solution is to do:

immutable Foo{T} end
=={T1,T2}(lhs::Foo{T1},rhs::Foo{T2}) = lhs===rhs
=={T}(lhs::Foo{T},rhs::Any) = false
=={T}(lhs::Any,rhs::Foo{T}) = false

So I think that the warning is misleading and should be changed to:

Warning: New definition
==(Any,Foo{T}) at none:1
is ambiguous with:
==(Foo{T},Any) at none:1.
To fix, define
==(Foo{T1},Foo{T2})
before the new definition.

[StefanKarpinski]

Perhaps we should print type parameters as _ in type vars with the understanding that multiple occurrences of _ do not mean the same thing. Naturally, when someone sees two occurrences of T they think they're the same T even though here they're different existential T variables. In cases where there's nothing "diagonal" about the type and the type vars are unconstrained it might be clearer to leave them off entirely.

[toivoh]

Couldn't they be renamed to be unique? Of course, it's a question of unique among what set of identifiers, with bad luck they can start to collide with argument and type names, and maybe more...

[StefanKarpinski]

They could, but that's such a pain.

[nalimilan]

Aren't there cases where two types are the same, and shouldn't this be visible somehow? Calling them T1, T2... sounds the most natural solution (don't worry about possible conflicts with variables, it's too unlikely).

[burrowsa]

From a user's perspective I think that if the warning is going to suggest a solution it should suggest one that works i.e. one I can just copy and paste.

I found it a bit confusing that the suggested code didn't have the template parameters defined. I think the perfect warning message would be:

Warning: New definition
==(Any,Foo{T}) at none:1
is ambiguous with:
==(Foo{T},Any) at none:1.
To fix, define
=={T1, T2}(Foo{T1},Foo{T2})
before the new definition.

but I the one I listed in my original post would be good enough.

[StefanKarpinski]

All of those type parameters are gratuitous. You could just do this:

Warning: New definition 
    ==(Any,Foo) at none:1
is ambiguous with: 
    ==(Foo,Any) at none:1.
To fix, define 
    ==(Foo,Foo)
before the new definition.

The real problem is how type vars are printed in this context (and in others, since there's no input syntax for them).

[simonster]

IMO the most confusing case is:

julia> f{T}(::Array{T}, ::Array{T}) = 1
f (generic function with 1 method)

julia> f{T}(::Array{T}, ::Array) = 1
f (generic function with 2 methods)

julia> methods(f)
# 2 methods for generic function "f":
f{T}(::Array{T,N},::Array{T,N}) at none:1
f{T}(::Array{T,N},::Array{T,N}) at none:1

[burrowsa]

I did try ==(Foo, Foo) but I still got the warning:

immutable Foo{T} end
==(lhs::Foo,rhs::Foo) = lhs===rhs
=={T}(lhs::Any,rhs::Foo{T}) = false
=={T}(lhs::Foo{T},rhs::Any) = false

Warning: New definition
==(Foo{T},Any) at none:1
is ambiguous with:
==(Any,Foo{T}) at none:1.
To fix, define
==(Foo{T},Foo{T})
before the new definition.

The only thing that worked was: =={T1,T2}(lhs::Foo{T1},rhs::Foo{T2})

[StefanKarpinski]

If so, it's a bug. ==(lhs::Foo, rhs::Foo) should mean the same thing as =={T1,T2}(lhs::Foo{T1},rhs::Foo{T2}).

[burrowsa]

OK, should I raise another issue for that too?

[StefanKarpinski]

No, we can leave it all here for now. Unless I'm being really dumb, this does appear to be a problem:

julia> immutable Foo{T} end

julia> =={S,T}(lhs::Foo{S},rhs::Foo{T}) = lhs===rhs
== (generic function with 47 methods)

julia> =={S}(lhs::Foo{S},rhs::Any) = false
== (generic function with 48 methods)

julia> =={T}(lhs::Any,rhs::Foo{T}) = false
== (generic function with 49 methods)

julia> immutable Foo{T} end

julia> ==(lhs::Foo,rhs::Foo) = lhs===rhs
== (generic function with 47 methods)

julia> =={S}(lhs::Foo{S},rhs::Any) = false
== (generic function with 48 methods)

julia> =={T}(lhs::Any,rhs::Foo{T}) = false
Warning: New definition
    ==(Any,Foo{T}) at none:1
is ambiguous with:
    ==(Foo{S},Any) at none:1.
To fix, define
    ==(Foo{S},Foo{T})
before the new definition.
== (generic function with 49 methods)

[toivoh]

Come to think about it, I if we don't want to give the typevars unique names, it would still be a huge improvement if the error message in the original post had type parameters on the function name as well,

Warning: New definition
=={T}(Any,Foo{T}) at none:1
is ambiguous with:
=={T}(Foo{T},Any) at none:1.
To fix, define
=={T,T}(Foo{T},Foo{T})
before the new definition.

then you would see that there is something fishy with the last one.
I would still be interested to hear from @JeffBezanson how much of a pain it would be to get unique names for the typevars. This kind of error does come up from time to time, and can be extremely confusing to people just when they start to experiment with parametric methods, at which point they might very well be confused enough already.

[JeffBezanson]

It wouldn't be too bad --- you can create an environment mapping the parameters of the second signature to new names (as necessary), and then call jl_instantiate_type_with on the intersected type and that environment.

[vtjnash]

No, we can leave it all here for now. Unless I'm being really dumb, this does appear to be a problem:

@StefanKarpinski I think this is because matching a type parameter is better than not matching a type parameter, so Foo{T} is considered stricter than just Foo

The real problem is how type vars are printed in this context (and in others, since there's no input syntax for them).

Is this frowned upon in some way?

julia> immutable Foo{T} end

julia> ==(lhs::Foo{TypeVar(:T)},rhs::Foo{TypeVar(:T)}) = lhs===rhs
== (generic function with 47 methods)

julia> =={T}(lhs::Foo{T},rhs::Any) = false
== (generic function with 48 methods)

julia> =={T}(lhs::Any,rhs::Foo{T}) = false
== (generic function with 49 methods)

I find you can do some fun things with it:

typealias Index Union(AbstractVector{TypeVar(:I,Integer)}, Integer)
isindex(i::Index) = true
isindex(otherwise) = false

julia> isindex(1)
true

julia> isindex([1,2])
true

julia> isindex([1,2.0])
false

(generalization of RangeIndex in subarray.jl)

[artkuo]

I'm not sure if it's related, but I get the following warning (0.5.0-dev, 0.4):

julia> foo(A::AbstractVecOrMat, B::Tridiagonal) = println("tridiagonal")
foo (generic function with 1 method)

julia> foo(A::AbstractVector, B::AbstractMatrix) = println("abstract matrix")
WARNING: New definition
    foo(AbstractArray{T<:Any, 1}, AbstractArray{T<:Any, 2}) at none:1
is ambiguous with:
    foo(Union{AbstractArray{T<:Any, 1}, AbstractArray{T<:Any, 2}}, Base.LinAlg.Tridiagonal) at none:1.
To fix, define
    foo(AbstractArray{T<:Any, 1}, Base.LinAlg.Tridiagonal)
before the new definition.
foo (generic function with 2 methods)

The warning can be avoided by inserting the following between the two lines:

foo(A::AbstractVector, B::Tridiagonal) = println("tridiagonal vector")

Or the first line can be split into separate definitions for AbstractVector and AbstractMatrix to eliminate the problem. There seems to be confusion related to the Union that is AbstractVecOrMat, which is so distracting that the other argument, AbstractMatrix is not recognized to be an unambiguous super of Tridiagonal?

[mauro3]

Close now that no more warnings are printed?