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.

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

Bad nonlocal return with re-wrapped define_method block #2833

Closed
headius opened this Issue Apr 14, 2015 · 3 comments

Comments

Projects
None yet
2 participants
@headius
Copy link
Member

headius commented Apr 14, 2015

This example should print out :here but it does not, because the non-local return in the define_method block causes the test method to return early.

class Foo
  def define(name, &block)
    class << self; self; end.send(:define_method, name, block)
  end

  def define2(name, &block)
    define(name, &block)
  end

  def test
    define(:foo) { return 1 }
    call_foo
    puts :here
  end

  def call_foo
    foo
  end
end

Foo.new.test

This is causing mocha's test suite to bail out prematurely.

@headius headius added this to the 9.0.0.0.pre2 milestone Apr 14, 2015

@enebo

This comment has been minimized.

Copy link
Member

enebo commented Apr 14, 2015

Changing this method from:

  def define(name, &block)
    class << self; self; end.send(:define_method, name, block)
  end

to

  def define(name, &block)
    class << self; self; end.send(:define_method, name, &block)
  end

Makes this code work. So procizilation is breaking this somehow.

@enebo

This comment has been minimized.

Copy link
Member

enebo commented Apr 14, 2015

Reduced case:

a = proc { return 1 }
define_method(:foo, a)
p foo

@enebo enebo closed this in 04ffe40 Apr 14, 2015

@enebo enebo added core and removed ir labels Apr 14, 2015

@enebo

This comment has been minimized.

Copy link
Member

enebo commented Apr 14, 2015

This seemingly is not an IR bug but one where we did not convert the incoming proc into a lambda. I am not 100% sure how this changes the flow control nature of the problem but I saw no reason why we should be obeying proc rules for a block which now the body of a method. Nothing failed with this change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.