Repository navigation
Conversation
Binding#local_variable_get and #local_variable_set took a variable's depth from the static scopes and used it to index the dynamic ones. The two chains don't line up inside a for loop: the loop body gets its own dynamic scope in the interpreter, but a block inside it has the enclosing method's static scope as its parent. So from a block in a for loop, the depth landed one scope short, on the loop body's empty scope, and raised "DynamicScope0 only supports scopes with 0 variables". Walk the dynamic scopes instead and use the first one whose static scope has the variable, stopping at the method's scope as before. Fixes jruby#9849
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
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.
Fixes #9849.
Binding#local_variable_getand#local_variable_setlooked up a variable's depth in the static scopes and then used that depth to index the dynamic scopes. Inside aforloop those don't line up. The interpreter gives the loop body a dynamic scope of its own, but a block inside the loop has the enclosing method's static scope as its parent, skipping the loop. So from a block in aforloop the depth landed one scope short, on the loop body's emptyDynamicScope0, which raised "DynamicScope0 only supports scopes with 0 variables".evalwasn't affected because it gets its depths from the IR scopes, which already account forforloops.This walks the dynamic scopes instead and uses the first one whose own static scope has the variable, stopping at the method's scope as
isDefinedOrImplicitdoes. That's right whether or not the loop body has a scope of its own, which differs between the interpreter and the JIT. New variables still go into the binding's eval scope, and implicititis still rejected the same way.I added a spec for each method to ruby/spec's
local_variable_get_spec.rbandlocal_variable_set_spec.rb. Both fail on master and pass with this change. I also compared about a dozen binding cases against CRuby in the interpreter and both JIT modes, including nestedforloops, variables declared in the loop body,Proc#binding, and new variables set through the binding, and they all match. The Binding,eval,Proc#bindingandforspecs and the MRIeval,proc,iteratorandsyntaxtests pass in all three modes.