Skip to content

Splatted arguments sometimes aren't passed to methods properly. #2926

Closed
chanks opened this Issue Feb 7, 2014 · 14 comments

5 participants

@chanks
chanks commented Feb 7, 2014

This is a weird one. The JIT, maybe?

# test.rb
$index = 0

def m(input = 5)
  if input == 5
    puts $index += 1
  else
    raise "Input: #{input.inspect}"
  end
end

value = nil

loop do
  m(*value)
end

Then:

ruby test.rb
...
43304
43305
43306
43307
43308
43309
An exception occurred running test.rb:

    Input: nil (RuntimeError)

Backtrace:

                          Object#m at test.rb:7
          { } in Object#__script__ at test.rb:14
               Kernel(Object)#loop at kernel/common/kernel.rb:460
                 Object#__script__ at test.rb:13
  Rubinius::CodeLoader#load_script at kernel/delta/code_loader.rb:66
  Rubinius::CodeLoader.load_script at kernel/delta/code_loader.rb:201
           Rubinius::Loader#script at kernel/loader.rb:649
             Rubinius::Loader#main at kernel/loader.rb:831

ruby -v
rubinius 2.2.3 (2.1.0 4792e746 2013-12-29 JI) [x86_64-linux-gnu]
@chanks
chanks commented Feb 7, 2014

Just upgraded to 2.2.4, same thing happens.

@YorickPeterse
Rubinius member

This indeed seems to be the JIT. Running the script with the JIT disabled (rbx -Xint script.rb) does not seem to produce the same error.

@YorickPeterse YorickPeterse added the JIT label Feb 8, 2014
@dbussink
Rubinius member
dbussink commented Feb 8, 2014

Further simplification:

loop do
  value = nil
  ary = *value
  if ary == [nil]
    raise "error"
  end
end
@dbussink
Rubinius member
dbussink commented Feb 8, 2014

So I've found the issue and it's fairly straightforward. I'd like to offer up pairing on this with someone if they're interested in finding out more about how some of the bits in the JIT work and how this case can be fixed.

@YorickPeterse
Rubinius member

I'd be up for that, and I'm pretty sure others might be too (e.g. @razielgn might be interested as well).

@ileitch
Rubinius member
ileitch commented Feb 8, 2014

Maybe record a short video like you did once before? I enjoyed watching.

@dbussink
Rubinius member
dbussink commented Feb 9, 2014

@ileitch One thing I thought about is doing a hangout and recording that for posterity so people can view that.

@razielgn
razielgn commented Feb 9, 2014

An hangout sounds very good to me. We could join live to ask questions if we manage to find a common time/date. :)

@dbussink
Rubinius member

I can do a hangout Tuesday evening CET timezone, how does that sound?

@YorickPeterse
Rubinius member

Fine by me!

@razielgn

I can make it from 6pm to 8pm.

@dbussink
Rubinius member

How about Tuesday 19:00 CET then? We should get through it pretty quick I think.

@razielgn

Yeah, sounds good. Thank you. :)

@YorickPeterse
Rubinius member

19:00 is a bit too early for me as that's around the time I hop in a train. I'm perfectly fine with watching the recording though.

@razielgn razielgn added a commit that referenced this issue Feb 11, 2014
@razielgn razielgn Fixed a problem where splatted nil would get converted to [nil].
Fixes #2926.

Previously, the JIT code was not checking for nil, when trying to cast
the stack value to an array, thus creating an array with just nil in it.
This fix ensures an empty array gets created instead.

With help from @dbussink, and recorded it live on an hangout!
e1b064c
@dbussink dbussink closed this in #2938 Feb 12, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.