IRReturnJump when returning from a proc returned by a lambda #6350
Comments
eregon
changed the title
|
Also happens for: def m
b = -> { Proc.new { return 42 } }.call
b.call # returns from the method
p :after # never reached!
end
p mFYI The example is from oracle/truffleruby#1488 where we try to figure out what are the semantics of |
headius
changed the title
|
Backtrace for flow-control jumps is available with I believe both of these cases should be LocalJumpError. The bug is that we are not detecting the lambda scope when determining where to return to. The logic to generate the return jump exception detects the intervening lambda frame and sets that as the target for the jump. This is correct behavior; because the proc was created within a lambda, its return should be from the lambda level. The subsequent logic that unrolls the stack should raise either an IRReturnJump (if the target frame is active) or a Ruby LocalJumpError (if the target frame is no longer active). Unfortunately this logic skips over the lambda and instead finds the top-level (in the top-level example) or the method (in the method example). In both cases they are still on active. As a result, we raise an IRReturnJump, expecting it to be handled as a normal non-local return, when we should be raising a LocalJumpError. I see from oracle/truffleruby#1488 there is some abiguity in the CRuby behavior. I would consider that behavior to be incorrect; once the proc has escaped from the lambda, its return target is no longer valid. It should not return to a different place. This is likely only effective in CRuby because of their messy handling of these jumps... there happens to be someone ready to handle any old return jump, so it just happens to work. With a simple patch in JRuby, we get the behavior I think is correct for both cases: I will push a PR to see how it lands. |
In the logic to look for a value return target, we find a lambda, via IRRuntimeHelpers.initiateNonLocalReturn calling getContainingLambda. However when we later try to detect if the return jump should propagate as a LocalJumpError (because its target scope is no longer active) we do not do this same search for a lambda (see IRRuntimeHelpers.checkForLJE). This inconsistency causes the errors from jruby#6350, because we incorrectly detect the target as the lambda, but do the detection against its parent scope and raise an IRReturnJump with an improper target scope. This jump then bubbles all the way off the stack and never becomes a LocalJumpError. In CRuby, when the lambda becomes inactive, the returnn target moves to the containing return target scope. This means a given return can potentially have two different targets, depending on whether its containing lambda is still on the stack or not. I believe the appropriate behavior in both cases from jruby#6350 is to raise LocalJumpError immediately. In each case, the return target for the proc should be the lambda, but the lambda is no longer active. The return target should not change to the containing scope, because that's not the proper return target. The change here adds the lambda logic to the LJE check, so that it sees the lambda return target is no longer active and eagerly raises the LJE. We will need to reconcile this difference with CRuby, but since this is such an obscure case I doubt it will affect real-world code.
In the logic to look for a valid return target, we find a lambda, via `IRRuntimeHelpers.initiateNonLocalReturn` calling `getContainingLambda`. However when we later try to detect if the return jump should propagate as a LocalJumpError (because its target scope is no longer active) we do not do this same search for a lambda (see `IRRuntimeHelpers.checkForLJE`). This inconsistency causes the errors from jruby#6350, because we incorrectly detect the target as the lambda, but do the detection against its parent scope and raise an IRReturnJump with an improper target scope. This jump then bubbles all the way off the stack and never becomes a LocalJumpError. In CRuby, when the lambda becomes inactive, the returnn target moves to the containing return target scope. This means a given return can potentially have two different targets, depending on whether its containing lambda is still on the stack or not. I believe the appropriate behavior in both cases from jruby#6350 is to raise LocalJumpError immediately. In each case, the return target for the proc should be the lambda, but the lambda is no longer active. The return target should not change to the containing scope, because that's not the proper return target. The change here adds the lambda logic to the LJE check, so that it sees the lambda return target is no longer active and eagerly raises the LJE. We will need to reconcile this difference with CRuby, but since this is such an obscure case I doubt it will affect real-world code.
I agree the semantics seem confusing and potentially unintentional here. |
|
Given that we have Shyouhei on our side I think it's likely that the behavior we want will be considered the right behavior. I've merged the JRuby fix that makes both cases LJE. |
|
Matz has approved the error condition described above, which @ko1 has a patch for in ruby/ruby#4347. Nothing for us to do since we have already made this change for 9.3. |
Environment Information
Provide at least:
jruby 9.2.13.0 (2.5.7)Expected Behavior
Actual Behavior
Might relate to #1134.
The text was updated successfully, but these errors were encountered: