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

proc with extra args incorrectly binds wrong post args #2485

Closed
enebo opened this Issue Jan 19, 2015 · 3 comments

Comments

Projects
None yet
3 participants
@enebo
Member

enebo commented Jan 19, 2015

This test in mri test suite shows it (and quite a bit more):

pr = proc {|a=:a,b=:b,c=:c,d,e|
  [a,b,c,d,e]
}

p pr.call([1,2,3,4,5,6])

recv_post_arg incorrectly binds d and e to 5,6 instead of 4,5.

@enebo enebo added this to the JRuby 9.0.0.0 milestone Jan 19, 2015

@enebo enebo modified the milestone: JRuby 9.0.0.0 Jul 14, 2015

@phluid61

This comment has been minimized.

Show comment
Hide comment
@phluid61

phluid61 Aug 24, 2016

Contributor

For the record, it's not related to the array:

pr = proc {|a=:a,b=:b,c=:c,d,e|
  [a,b,c,d,e]
}

p pr.call(1,2,3,4,5,6)
#=> [1, 2, 3, 5, 6]
Contributor

phluid61 commented Aug 24, 2016

For the record, it's not related to the array:

pr = proc {|a=:a,b=:b,c=:c,d,e|
  [a,b,c,d,e]
}

p pr.call(1,2,3,4,5,6)
#=> [1, 2, 3, 5, 6]
@headius

This comment has been minimized.

Show comment
Hide comment
@headius

headius Aug 24, 2016

Member

Still failing as of 9.1.3.0.

Member

headius commented Aug 24, 2016

Still failing as of 9.1.3.0.

@enebo enebo added this to the JRuby 9.1.4.0 milestone Aug 24, 2016

@enebo

This comment has been minimized.

Show comment
Hide comment
@enebo

enebo Aug 24, 2016

Member

I have this fixed locally but I am terribly uncomfortable with my fixes and will not check them in for 9.1.3.0. The actual issue (@phlui61 is correct Array is not part of this although the test case I got from MRI happens to wrap in an array) is that our recv_post_args instr expects to walk from end of incoming arg list. In this case, we pass in more args than can be filled so we walk from the extra arg back.

My fix corrects procs to cull their arg list to match the blocks expected arity. I am not in love with that solution partially because we have to construct a second args list. MRI as I discovered counts forward and calculates where post args begin. That is a better solution and one we should do.

Member

enebo commented Aug 24, 2016

I have this fixed locally but I am terribly uncomfortable with my fixes and will not check them in for 9.1.3.0. The actual issue (@phlui61 is correct Array is not part of this although the test case I got from MRI happens to wrap in an array) is that our recv_post_args instr expects to walk from end of incoming arg list. In this case, we pass in more args than can be filled so we walk from the extra arg back.

My fix corrects procs to cull their arg list to match the blocks expected arity. I am not in love with that solution partially because we have to construct a second args list. MRI as I discovered counts forward and calculates where post args begin. That is a better solution and one we should do.

@enebo enebo modified the milestones: JRuby 9.1.7.0, JRuby 9.1.6.0 Nov 7, 2016

@enebo enebo closed this in 4e4935e Dec 2, 2016

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