-
-
Notifications
You must be signed in to change notification settings - Fork 923
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
Remove JumpException, Next, Redo, etc. exceptions from old runtime #2416
Comments
We still have SPECIAL_JUMP which is used for all sorts of enumerable messages and so SpecialJump for a simple marker flow control exception instance is overkill so we should consider killing SpecialJumpException altogether for: Exception SPECIAL_JUMP = new Exception(); There is a tiny risk some native exceptions use Return/Next/Redo but since they are never thrown in 9 I think it is correct for the extensions to error out and know about it sooner than later... |
How does IR represent control flow now, if not by exceptions? Especially NextException with blocks - that is handled by the yield-site, not by the block itself, is it? So it could be compiled separately from the yield-site and the control flow is non-local. |
IR uses exceptions as well. These older AST-runtime exceptions weren't being used. As for next, a next in a closure becomes a non-local return (which is handled using non-local exceptions anyway). https://github.com/jruby/jruby/blob/master/core/src/main/java/org/jruby/ir/IRBuilder.java#L2781-L2786 |
Uh .. I am clearly rusty at this point, but, no, that is not a non-local return, just a simple return which transfers control back to the yielder, and so no exceptions involved in next handling. |
These exceptions should no longer be thrown in the IR-based runtime. Consequently, all the try-catch handleres for these exceptions can be stripped out.
Audit these uses, verify that no runtime code throws these anymore, and see if any of these sites require equivalent IRReturnJump / IRBreakJump protection (unlikely, but doesn't hurt to verify while stripping out the old runtime ones).
The text was updated successfully, but these errors were encountered: