JRuby

ArgumentError output

Details

  • Type: Bug Bug
  • Status: Resolved Resolved
  • Priority: Major Major
  • Resolution: Fixed
  • Affects Version/s: JRuby 1.7.0.pre1
  • Fix Version/s: JRuby 1.7.0.pre2
  • Component/s: None
  • Labels:
    None
  • Number of attachments :
    0

Description

Given a file `args_error.rb` with

def my_meth(a)
end

my_meth 1, 2

In JRuby, this produces:

ArgumentError: wrong number of arguments (2 for 1)
  (root) at args_error.rb:4

In MRI 1.9:

args_error.rb:1:in `my_meth': wrong number of arguments (2 for 1) (ArgumentError)
	from args_error.rb:4:in `<main>'

In MRI 1.8.7:

args_error.rb:4:in `my_meth': wrong number of arguments (2 for 1) (ArgumentError)
	from args_error.rb:4

(Only the line number change)

The MRI version (and Rbx too by the way) mentions the method name.
I think this can be helpful, to easily understand what might be the problem without needing to go to the caller place, and when the call is dynamic (#send).

For the location, 1.8 seems to mention the caller location, while 1.9 shows the callee location (and likely showing the line of the method definition in its file, which can be very helpful).

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Charles Oliver Nutter added a comment -

A certain portion of this is due to our adding "Rubinius-style" backtraces in 1.6, which structure things a bit differently. In MRI backtrace mode, we're a bit closer:

system ~/projects/jruby $ jruby -X-C blah.rb
ArgumentError: wrong number of arguments (2 for 1)
  (root) at blah.rb:4

system ~/projects/jruby $ jruby -Xbacktrace.style=mri -X-C blah.rb
blah.rb:8:in `(root)': wrong number of arguments (2 for 1) (ArgumentError)

But we show the call site rather than the called method partially because we argument check a lot earlier than MRI or Rubinius do (i.e. before we even enter the method). I'm playing with at least getting the actual called name into the error, but getting the called method's line number might be harder.

Show
Charles Oliver Nutter added a comment - A certain portion of this is due to our adding "Rubinius-style" backtraces in 1.6, which structure things a bit differently. In MRI backtrace mode, we're a bit closer: system ~/projects/jruby $ jruby -X-C blah.rb ArgumentError: wrong number of arguments (2 for 1) (root) at blah.rb:4 system ~/projects/jruby $ jruby -Xbacktrace.style=mri -X-C blah.rb blah.rb:8:in `(root)': wrong number of arguments (2 for 1) (ArgumentError) But we show the call site rather than the called method partially because we argument check a lot earlier than MRI or Rubinius do (i.e. before we even enter the method). I'm playing with at least getting the actual called name into the error, but getting the called method's line number might be harder.
Hide
Permalink
Charles Oliver Nutter added a comment -

Ok, so I can get the called method name in the message easy enough, but getting the actual called method's file and line number won't be possible in the current compiler (though possible in the interpreter). This is because we do arity-checking outside the body of compiled code, but we generate backtraces off the Java stack trace; because the error occurs before we call the target method we have no stack trace element to yank out for the Ruby error.

This will probably be easier to do with the IR and IR2JVM execution mechanisms, since there arity-checking is just another instruction.

I think I will proceed with the error message change for now, since we at least would like to guarantee we have the target method's name in there somewhere.

Show
Charles Oliver Nutter added a comment - Ok, so I can get the called method name in the message easy enough, but getting the actual called method's file and line number won't be possible in the current compiler (though possible in the interpreter). This is because we do arity-checking outside the body of compiled code, but we generate backtraces off the Java stack trace; because the error occurs before we call the target method we have no stack trace element to yank out for the Ruby error. This will probably be easier to do with the IR and IR2JVM execution mechanisms, since there arity-checking is just another instruction. I think I will proceed with the error message change for now, since we at least would like to guarantee we have the target method's name in there somewhere.
Hide
Permalink
Benoit Daloze added a comment -

Sounds good, thanks a lot for the quick and accurate response.
Having the method name will definitely help for debugging this kind of error.

Show
Benoit Daloze added a comment - Sounds good, thanks a lot for the quick and accurate response. Having the method name will definitely help for debugging this kind of error.
Hide
Permalink
Charles Oliver Nutter added a comment -

I'm on the fence about whether to resolve this, but I have at least added the method name to the error message where I can.

commit 6ecfcee3afb2afffc8c0b7058fdc2c2ffc49da47
Author: Charles Oliver Nutter <headius@headius.com>
Date:   Thu Jul 5 15:05:59 2012 -0500

    Mostly fix JRUBY-6760: ArgumentError output
    
    It's not possible right now for us to have ArgumentError report
    the file:line of the called method, since arity checking occurs
    outside the method body. However, we usually do have the called
    name available at that point, so we can include it in the error
    message.

:100644 100644 bcd8ce1... 9ad7c1c... M	src/org/jruby/Ruby.java
:100644 100644 8ac7710... 572c8d2... M	src/org/jruby/RubyStruct.java
:100644 100644 feed634... 47d9d40... M	src/org/jruby/ast/ArgsNode.java
:100644 100644 5fa11c6... 3e13e16... M	src/org/jruby/ext/ffi/jffi/NativeInvoker.java
:100644 100644 b8dca78... 969b863... M	src/org/jruby/ext/socket/RubyUNIXSocket.java
:100644 100644 2c39223... 9446960... M	src/org/jruby/internal/runtime/methods/InterpretedMethod.java
:100644 100644 fec2384... 897f73f... M	src/org/jruby/internal/runtime/methods/JavaMethod.java
:100644 100644 15487b9... 56401e9... M	src/org/jruby/internal/runtime/methods/ReflectedJavaMethod.java
:100644 100644 9e36221... 06e91a8... M	src/org/jruby/java/proxies/JavaInterfaceTemplate.java
:100644 100644 860d7b7... efabc1c... M	src/org/jruby/runtime/Arity.java
Show
Charles Oliver Nutter added a comment - I'm on the fence about whether to resolve this, but I have at least added the method name to the error message where I can. commit 6ecfcee3afb2afffc8c0b7058fdc2c2ffc49da47 Author: Charles Oliver Nutter <headius@headius.com> Date: Thu Jul 5 15:05:59 2012 -0500 Mostly fix JRUBY-6760: ArgumentError output It's not possible right now for us to have ArgumentError report the file:line of the called method, since arity checking occurs outside the method body. However, we usually do have the called name available at that point, so we can include it in the error message. :100644 100644 bcd8ce1... 9ad7c1c... M src/org/jruby/Ruby.java :100644 100644 8ac7710... 572c8d2... M src/org/jruby/RubyStruct.java :100644 100644 feed634... 47d9d40... M src/org/jruby/ast/ArgsNode.java :100644 100644 5fa11c6... 3e13e16... M src/org/jruby/ext/ffi/jffi/NativeInvoker.java :100644 100644 b8dca78... 969b863... M src/org/jruby/ext/socket/RubyUNIXSocket.java :100644 100644 2c39223... 9446960... M src/org/jruby/internal/runtime/methods/InterpretedMethod.java :100644 100644 fec2384... 897f73f... M src/org/jruby/internal/runtime/methods/JavaMethod.java :100644 100644 15487b9... 56401e9... M src/org/jruby/internal/runtime/methods/ReflectedJavaMethod.java :100644 100644 9e36221... 06e91a8... M src/org/jruby/java/proxies/JavaInterfaceTemplate.java :100644 100644 860d7b7... efabc1c... M src/org/jruby/runtime/Arity.java
Hide
Permalink
Charles Oliver Nutter added a comment -

I'm going to call this resolved, because I don't think it's feasible to go any further with it.

In most Ruby implementations, arity checking happens once you're already inside the target method. This makes it possible to report the current method's file:line, but it also means that arity checking always happens regardless of whether it is needed.

In JRuby, mismatched arity generally doesn't even make it into the target method, because there's no path with that arity to even make the call. The error is issued before the method body starts executing, so the file:line context is not yet available. This means we report less information in backtraces, but it also means that matching-arity calls do no checking at all, which is part of the reason JRuby performs so well for those calls.

Making the file:line available to for the backtrace would mean we'd have to move the arity-checking down into the target method and perform the checks much more frequently. In some cases, it might simply be impossible...since as I stated above that mismatched arity calls will fail to even reach the target method body.

Show
Charles Oliver Nutter added a comment - I'm going to call this resolved, because I don't think it's feasible to go any further with it. In most Ruby implementations, arity checking happens once you're already inside the target method. This makes it possible to report the current method's file:line , but it also means that arity checking always happens regardless of whether it is needed. In JRuby, mismatched arity generally doesn't even make it into the target method, because there's no path with that arity to even make the call. The error is issued before the method body starts executing, so the file:line context is not yet available. This means we report less information in backtraces, but it also means that matching-arity calls do no checking at all, which is part of the reason JRuby performs so well for those calls. Making the file:line available to for the backtrace would mean we'd have to move the arity-checking down into the target method and perform the checks much more frequently. In some cases, it might simply be impossible...since as I stated above that mismatched arity calls will fail to even reach the target method body.
Hide
Permalink
Benoit Daloze added a comment -

I see, thanks for all the details!
It's indeed a nice optimization, and likely worth it, even if that means less callee info on error.

Show
Benoit Daloze added a comment - I see, thanks for all the details! It's indeed a nice optimization, and likely worth it, even if that means less callee info on error.

People

  • Assignee:
    Charles Oliver Nutter
    Reporter:
    Benoit Daloze
Vote (0)
Watch (3)

Dates

  • Created:
    Updated:
    Resolved: