Fixes #7218. Empty rescue block returns exception object instead of nil.#7282
Merged
enebo merged 1 commit intojruby:jruby-9.3from Aug 1, 2022
Merged
Fixes #7218. Empty rescue block returns exception object instead of nil.#7282enebo merged 1 commit intojruby:jruby-9.3from
enebo merged 1 commit intojruby:jruby-9.3from
Conversation
… of nil. The fix was an error in our parser. When we get exception variables in a rescue statement we will make effectively '_e = $!' and then append whatever is actually in the body of the exception body. In MRI's case it ends up being 'nil'. In our case it was 'null'. Our impl thinks it is smart and realizes nothing is happening and what should you append if it is null anyways and just left the body as only '_e = $!'. We needed '_e = $!; nil'. Trivial fix is to make sure any null return from compstmt in this grammar production will always just be nil. Impact from this change should only effect the case when a rescue body is empty and in the precence of an exception variable. What we did was wrong. I see no risk.
Member
Author
|
HEH. I always do this #7218 is fixed by this (subject never links the two???). |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The fix was an error in our parser. When we get exception variables in
a rescue statement we will make effectively '_e = $!' and then append
whatever is actually in the body of the exception body. In MRI's case
it ends up being 'nil'. In our case it was 'null'. Our impl thinks it
is smart and realizes nothing is happening and what should you append
if it is null anyways and just left the body as only '_e = $!'. We
needed '_e = $!; nil'.
Trivial fix is to make sure any null return from compstmt in this grammar
production will always just be nil. Impact from this change should only
effect the case when a rescue body is empty and in the precence of an
exception variable. What we did was wrong. I see no risk.