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

compiled .rb does not handle rest kwargs right #3734

Closed
kares opened this Issue Mar 14, 2016 · 5 comments

Comments

Projects
None yet
2 participants
@kares
Member

kares commented Mar 14, 2016

Have been adding some "edge" case specs for jrubyc as there's been a lot of regression around loading compiled .class files.

Issue

Stumbled upon:

  def generic(*args, **kwargs, &block)
    [ args, kwargs, block ]
  end

# when compiled .class files is loaded with such method it will : 

klass.new.generic('0', 111, arg: 1) { 'block' } # ["0", 111, {:arg=>1}]

this is part of the spec:jrubyc suite

Expected Behavior

bin/jruby -S rake spec:jrubyc currently pending due :

  1) JRuby::Compiler.compile_argv compiles hashy_kwargs.class correctly
     Failure/Error: expect( res[0] ).to eql [ '0', 111 ]

       expected: ["0", 111]
            got: ["0", 111, {:arg=>1}]

       (compared using eql?)
     # ./spec/jrubyc/java/loading_spec.rb:55:in `block in (root)'

@kares kares added this to the JRuby 9.1.0.0 milestone Mar 14, 2016

@headius

This comment has been minimized.

Show comment
Hide comment
@headius

headius Apr 19, 2016

Member

I can still fix this for 9.1. Must be using incorrect logic for kwargs in the compiler.

Member

headius commented Apr 19, 2016

I can still fix this for 9.1. Must be using incorrect logic for kwargs in the compiler.

@headius

This comment has been minimized.

Show comment
Hide comment
@headius

headius Apr 19, 2016

Member

Very strange. I did not realize this was a jrubyc test at first, and tested normal JIT compilation...which works fine. Current jrubyc in 9k does not actually compile code all the way to JVM bytecode, so this is likely a problem with how we're encoding IR into the binary string we store in jrubyc's .class output.

@enebo I'll be poking at IR serialization, but maybe you have some ideas here.

Member

headius commented Apr 19, 2016

Very strange. I did not realize this was a jrubyc test at first, and tested normal JIT compilation...which works fine. Current jrubyc in 9k does not actually compile code all the way to JVM bytecode, so this is likely a problem with how we're encoding IR into the binary string we store in jrubyc's .class output.

@enebo I'll be poking at IR serialization, but maybe you have some ideas here.

@headius

This comment has been minimized.

Show comment
Hide comment
@headius

headius Apr 19, 2016

Member

IR for running normally at command line (works properly):

begin INSTANCE_METHOD<generic>
signature(pre=0,opt=0,post=0,rest=NORM,kwargs=0,kwreq=0,kwrest=true)
declared variables
  args(0:0)
  kwargs(0:2)
  block(0:1)
  00:           %self := recv_self
  01:            %v_0 := load_implicit_closure
  02:  %current_scope := copy(scope<0>)
  03: %current_module := copy(mod<0>)
  04:                    check_arity(required: 0, opt: 0, rest: true, receivesKeywords: true, restKey: 2)
  05:           *args := recv_rest_arg(required: 0, argIndex: 0)
  06:         *kwargs := recv_kw_rest_arg(required: 0, argIndex: -1)
  07:            %v_3 := load_implicit_closure
  08:          *block := reify_closure(%v_3)
  09:                    line_num(lineNumber: 1)
  10:            %v_4 := copy(ary<*args, *kwargs, *block>)
  11:                    return(%v_4)

And running from jrubyc-compiled output (which does not work properly):

begin INSTANCE_METHOD<generic>
signature(pre=0,opt=0,post=0,rest=NORM,kwargs=0,kwreq=0,kwrest=true)
declared variables
  args(0:0)
  kwargs(0:2)
  block(0:1)
  00:           %self := recv_self
  01:            %v_0 := load_implicit_closure
  02:  %current_scope := copy(scope<0>)
  03: %current_module := copy(mod<0>)
  04:                    check_arity(required: 0, opt: 0, rest: true, receivesKeywords: true, restKey: 2)
  05:           *args := recv_rest_arg(required: 0, argIndex: 0)
  06:         *kwargs := recv_kw_rest_arg(required: 0, argIndex: -1)
  07:            %v_3 := load_implicit_closure
  08:          *block := reify_closure(%v_3)
  09:                    line_num(lineNumber: 1)
  10:            %v_4 := copy(ary<*args, *kwargs, *block>)
  11:                    return(%v_4)

I'm not sure I see a difference yet.

Member

headius commented Apr 19, 2016

IR for running normally at command line (works properly):

begin INSTANCE_METHOD<generic>
signature(pre=0,opt=0,post=0,rest=NORM,kwargs=0,kwreq=0,kwrest=true)
declared variables
  args(0:0)
  kwargs(0:2)
  block(0:1)
  00:           %self := recv_self
  01:            %v_0 := load_implicit_closure
  02:  %current_scope := copy(scope<0>)
  03: %current_module := copy(mod<0>)
  04:                    check_arity(required: 0, opt: 0, rest: true, receivesKeywords: true, restKey: 2)
  05:           *args := recv_rest_arg(required: 0, argIndex: 0)
  06:         *kwargs := recv_kw_rest_arg(required: 0, argIndex: -1)
  07:            %v_3 := load_implicit_closure
  08:          *block := reify_closure(%v_3)
  09:                    line_num(lineNumber: 1)
  10:            %v_4 := copy(ary<*args, *kwargs, *block>)
  11:                    return(%v_4)

And running from jrubyc-compiled output (which does not work properly):

begin INSTANCE_METHOD<generic>
signature(pre=0,opt=0,post=0,rest=NORM,kwargs=0,kwreq=0,kwrest=true)
declared variables
  args(0:0)
  kwargs(0:2)
  block(0:1)
  00:           %self := recv_self
  01:            %v_0 := load_implicit_closure
  02:  %current_scope := copy(scope<0>)
  03: %current_module := copy(mod<0>)
  04:                    check_arity(required: 0, opt: 0, rest: true, receivesKeywords: true, restKey: 2)
  05:           *args := recv_rest_arg(required: 0, argIndex: 0)
  06:         *kwargs := recv_kw_rest_arg(required: 0, argIndex: -1)
  07:            %v_3 := load_implicit_closure
  08:          *block := reify_closure(%v_3)
  09:                    line_num(lineNumber: 1)
  10:            %v_4 := copy(ary<*args, *kwargs, *block>)
  11:                    return(%v_4)

I'm not sure I see a difference yet.

@headius

This comment has been minimized.

Show comment
Hide comment
@headius

headius Apr 19, 2016

Member

More confirmation that something is getting lost in IR persistence:

[] ~/projects/jruby $ jruby -Xir.writing blah.rb
[["0", 111], {:arg=>1}, #<Proc:0x46dffdc3@blah.rb:7>]

[] ~/projects/jruby $ jruby -Xir.reading blah.rb
[["0", 111, {:arg=>1}], {}, #<Proc:0x5bc79255@blah.rb:7>]
Member

headius commented Apr 19, 2016

More confirmation that something is getting lost in IR persistence:

[] ~/projects/jruby $ jruby -Xir.writing blah.rb
[["0", 111], {:arg=>1}, #<Proc:0x46dffdc3@blah.rb:7>]

[] ~/projects/jruby $ jruby -Xir.reading blah.rb
[["0", 111, {:arg=>1}], {}, #<Proc:0x5bc79255@blah.rb:7>]
@headius

This comment has been minimized.

Show comment
Hide comment
@headius

headius Apr 19, 2016

Member

The blah.rb in question that I'm using to test:

def generic(*args, **kwargs, &block)
  [ args, kwargs, block ]
end

p generic('0', 111, arg: 1) { 'block' }
Member

headius commented Apr 19, 2016

The blah.rb in question that I'm using to test:

def generic(*args, **kwargs, &block)
  [ args, kwargs, block ]
end

p generic('0', 111, arg: 1) { 'block' }

@headius headius closed this in 5bfa81c Apr 19, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment