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.

Sign up for GitHub

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

Jump to bottom

Convert Splat operand into BuildSplatInstr #2409

Closed
subbuss opened this issue Jan 2, 2015 · 2 comments
Closed

Convert Splat operand into BuildSplatInstr #2409

subbuss opened this issue Jan 2, 2015 · 2 comments
Labels
ir
Milestone
JRuby 9.0.0.0-pre1

Comments

@subbuss
Copy link
Contributor

subbuss commented Jan 2, 2015

Splat operands are being generated in two contexts: call arguments and splats that show up in ruby code. For the latter, we need to use a BuildSplatInstr since these splats could throw exceptions if to_ary or to_a calls fails. Operands aren't supposed to be throwing exceptions.

So, we can use the current Splat operand for scenarios that won't throw exceptions (call args currently, but actual splats in future when we can determine they will succeed), and use BuildSplatInstr for the other splat use scenarios.
@subbuss subbuss added the ir label Jan 2, 2015
@subbuss subbuss added this to the JRuby 9.0.0.0 milestone Jan 2, 2015
subbuss added a commit that referenced this issue Jan 5, 2015
@subbuss
Fix rest arg recording in IRClosure (long commit msg for a 1 line-fix)
5ef2a90 
* Recording a rest arg for blocks involved creating a splat (just
  like in a method). However, the unsplat flag wasn't being set
  in IRClosure (unlike in a method).

* This bug only affected zsuper in a block that used by define_method.
  See output on example enclosed further below in master vs 1.7 ...
  zsuper from foo and bar emit different output which is broken.

  However, zsuper in define_method is no longer supported in
  Ruby 2.x, it appears. MRI throws the following RuntimeError:

"implicit argument passing of super from method defined by define_method() is not supported. Specify all arguments explicitly. (RuntimeError)"

  So, this bug (or bugfix) is not really relevant anymore. Hence,
  I am not adding a test/spec to document this behavior, but including
  a test snippet for it in the commit message.

  We should simply get rid of this support and simplify the zsuper
  logic in the IRBuilder.

* I am fixing this here so I can work on #2409 and implement a clean fix
  without worrying about different behavior for methods and closures.

--------------------------
class C
  def foo(a, *b)
    p "foo-a: #{a}; foo-b[0]: #{b[0]}"
  end

  def bar(a, *b)
    p "bar-a: #{a}; bar-b[0]: #{b[0]}"
  end
end

class D < C
  def self.doit(&blk)
    define_method :foo, blk
  end

  def bar(a, *b)
    super
  end
end

D.doit { |a, *b| super }

d = D.new
d.bar(1, 2, 3)
d.foo(1, 2, 3)
--------------------------
[subbu@earth ir] jruby /tmp/bug.rb
"bar-a: 1; bar-b[0]: 2"
"foo-a: 1; foo-b[0]: [2, 3]"
--------------------------
@headius
Copy link
Member

headius commented Jan 5, 2015

I'm fine doing JIT fixes necessary for this. Is that just a matter of handling BuildSplatInstr?

@subbuss
Copy link
Contributor Author

subbuss commented Jan 5, 2015

I thought I already handled BuildSplatInstr and the new Splat operand .. but I might have missed other uses of the old Splat or maybe there is some special-case handling that I overlooked.

@subbuss subbuss closed this as completed in 29371d9 Jan 5, 2015
subbuss added a commit that referenced this issue Jan 5, 2015
@subbuss
Fix #2409: Unbreak JIT
d211f19 
* It was just a stray context load that I forgot to remove.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
ir
Projects
None yet
Milestone
JRuby 9.0.0.0-pre1
Development

No branches or pull requests
2 participants
@headius @subbuss