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

Ugly method ambiguity errors #17007

Closed
nalimilan opened this issue Jun 18, 2016 · 5 comments
Closed

Ugly method ambiguity errors #17007

nalimilan opened this issue Jun 18, 2016 · 5 comments
Labels
regression Regression in behavior compared to a previous version

Comments

@nalimilan
Copy link
Member

Ambiguity errors are really hard to read and leak svec in 0.5. I consider this as a regression since ambiguity warnings were much cleaner on 0.4.

julia> f(x::Integer, y::Integer) = true
f (generic function with 1 method)

julia> f(x::Int, y::Any) = false
f (generic function with 2 methods)

julia> f(1, 2)
ERROR: MethodError: f(::Int64, ::Int64) is ambiguous. Candidates:
  svec(Tuple{#f,Int64,Int64},svec(),f(x::Int64, y) at REPL[2]:1)
  svec(Tuple{#f,Int64,Int64},svec(),f(x::Integer, y::Integer) at REPL[1]:1)
 in eval(::Module, ::Any) at ./boot.jl:231
 in macro expansion at ./REPL.jl:92 [inlined]
 in (::Base.REPL.##1#2{Base.REPL.REPLBackend})() at ./event.jl:46
@nalimilan nalimilan added the regression Regression in behavior compared to a previous version label Jun 18, 2016
@timholy
Copy link
Sponsor Member

timholy commented Jun 18, 2016

Just looks like the printing is broken: it should show just the 3rd item in the svec. Prob. broken in 0fa8bb5.

@nalimilan
Copy link
Member Author

Thanks, Tim!

Though I just realized something else is missing: in 0.4, a suggestion was also printed, giving the signature of the method one needs to define to fix the ambiguity. I think this was really useful, as ambiguities are quite hard to reason about, especially for newcomers. Any idea when/why it was removed?

@timholy
Copy link
Sponsor Member

timholy commented Jun 20, 2016

As for "why," compare using Images, DataArrays on both julia-0.4 and 0.5, and you'll appreciate the motivation 😄. More detail in #6190 and #16125.

I suppose we could add the printing of a signature of the method to resolve the ambiguity. In principle it's not hard: it's just typeintersect(args1, args2) on the argument tuple-types. However, I'm not certain it's all that useful except in cases where there are only two methods that are ambiguous: if there are 5 candidates, there are 10 pairs of methods to perform type-intersection on. I would also say it's hit-and-miss whether the advice is really worth following, vs. thinking it through carefully and redesigning the API. What are your thoughts?

@ufechner7
Copy link

ufechner7 commented Jun 20, 2016

Interestingly, in Julia 0.4.5 the warning is already printed, when the second function is defined. It is not necessary to call it:
`julia> f(x::Integer, y::Integer) = true
f (generic function with 1 method)

julia> f(x::Int, y::Any) = false
WARNING: New definition
f(Int64, Any) at none:1
is ambiguous with:
f(Integer, Integer) at none:1.
To fix, define
f(Int64, Integer)
before the new definition.
f (generic function with 2 methods)`

@JeffBezanson JeffBezanson self-assigned this Jun 20, 2016
@JeffBezanson JeffBezanson removed their assignment Jun 20, 2016
@mauro3
Copy link
Contributor

mauro3 commented Jun 21, 2016

@ufechner7: with #16125 the warning on definition was changed to an error on usage.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
regression Regression in behavior compared to a previous version
Projects
None yet
Development

No branches or pull requests

5 participants