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

Confusing MethodError for missing parameter #17319

Closed
timholy opened this issue Jul 7, 2016 · 5 comments
Closed

Confusing MethodError for missing parameter #17319

timholy opened this issue Jul 7, 2016 · 5 comments

Comments

@timholy
Copy link
Member

timholy commented Jul 7, 2016

Deliberately create a method that's missing a static parameter:

julia> foo{T}(x::AbstractVector) = 1
WARNING: static parameter T does not occur in signature for foo at REPL[1]:1.
The method will not be callable.
foo (generic function with 1 method)

Then:

julia> foo(1:5)
ERROR: MethodError: no method matching foo(::UnitRange{Int64})
Closest candidates are:
  foo{T}(::AbstractArray{T,1})
 in eval(::Module, ::Any) at ./boot.jl:234
 in macro expansion at ./REPL.jl:92 [inlined]
 in (::Base.REPL.##1#2{Base.REPL.REPLBackend})() at ./event.jl:46

Definitely confusing: that foo candidate sure looks like it matches, and the absence of any red text in the arguments, to indicate failure to match, seems to confirm that.

For comparison, let's add another method:

julia> foo(x::Number) = 2
foo (generic function with 2 methods)

julia> foo(1:5)
ERROR: MethodError: no method matching foo(::UnitRange{Int64})
Closest candidates are:
  foo{T}(::AbstractArray{T,1})
  foo(!Matched::Number)
 in eval(::Module, ::Any) at ./boot.jl:234
 in macro expansion at ./REPL.jl:92 [inlined]
 in (::Base.REPL.##1#2{Base.REPL.REPLBackend})() at ./event.jl:46

For reproduction here, I've annotated any red text in the "closest candidates" with !Matched. (Note the method we want does not have !Matched for any of its arguments.)

@yuyichao
Copy link
Contributor

yuyichao commented Jul 7, 2016

Should the missing type parameter just be an error? I cant think of why it is useful

@timholy
Copy link
Member Author

timholy commented Jul 7, 2016

That's not a bad solution.

It may help to explain that the context this came up in was make testall; I went away and did something else for a few minutes, then came back and saw the error waiting for me at the console. I spent several minutes in puzzlement before scrolling up several pages to the build log and see the warning. If we changed it to an error, this would be caught more easily, because the test suite wouldn't run. But if you're copy/pasting several pages of code from an editor window to the REPL, you may not notice the error and then might be puzzled about why the method isn't listed as a candidate in the method table. ("Dammit, I can see that method right there in the code, why isn't it showing up?") It seems possible that an even better solution would be to insert it into the method table, but have some dedicated way of revealing the problem to the user.

That said, I agree that an error would likely be an improvement over where we are now.

@nalimilan
Copy link
Member

@timholy I'd say raising an error should be the preferred way to report problems. If we start thinking that the user might miss some errors, then should we also accept invalid syntax and try to do something with it in the hope of printing a more helpful message later? :-)

More general fix: make paste at the REPL stop at the first error.

@timholy
Copy link
Member Author

timholy commented Jul 7, 2016

Good point, @nalimilan. I was just trying to think of a scenario in which the current behavior would be useful.

@yuyichao
Copy link
Contributor

Fixed by #23117

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants