-
-
Notifications
You must be signed in to change notification settings - Fork 925
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
LocalJumpError: unexpected break in spec/ruby/language/break_spec.rb #4577
Comments
Would be nice to get this fixed and the spec running. |
So this is another case where we are propagating the break out like we should, but we don't produce a rescuable LJE until it's too late. So there's some missing logic at the closure boundary (a proc in this case) to know that the break has no target and the LJE needs to start propagating earlier. |
The full backtrace is
So it seems the stack is unwound until the load call, which is way too far.
|
Yes, it doesn't stop at the rescue for the reason I mentioned above. We need to raise the LJE immediately when we can determine the break is not valid, so it can be rescued. A hacky alternative I've considered is to make LJE and superclasses match our non-local flow exceptions, but it would be better to just raise the real one. |
Fixes #4686. Fixes #4577. The logic here adds finally wrappers around all call paths that receive a block. When the call exits, the block is marked "escaped" since it no longer has a method activation to go with it. This indicates that non-local flow, like break, should immediately trigger a LocalJumpError. This passes specs but has not been tested extensively with other types of call forms that receive blocks.
Reopening to backport to 9.1.14. |
jruby 9.1.9.0-SNAPSHOT (2.3.3) 2017-04-28 038f790 OpenJDK 64-Bit Server VM 25.121-b14 on 1.8.0_121-b14 +jit [linux-x86_64]
Which can be reproduced with:
This specific spec is currently excluded in jruby.2.3.mspec so that's why it got unnoticed.
There are only 3
ruby_exe
calls in that spec so once this is fixed I would advise to include it on CI.The text was updated successfully, but these errors were encountered: