Skip to content

Commit

Permalink
[IR] Get rid of static scope id from non-local return handling.
Browse files Browse the repository at this point in the history
* Use "statically determined" but a dynamic scope marker (dynamic scope
  reference) to figure out what scopes to return from.

* Meta class bodies need special handling since they behave like
  method bodies in terms of variable sharing, but returns from them
  behave like non-local returns.

* All previously passing tests now pass. The test case from github
  issue 1644 also passes now.

  jruby -X-CIR -e "def foo; p :in; if block_given?; yield; else; foo { return }; end; p :out; end; foo"

  TODO: Add MRI/rubyspec test case.
  • Loading branch information
subbuss committed Apr 23, 2014
1 parent 06179e2 commit 5f5538f
Show file tree
Hide file tree
Showing 11 changed files with 59 additions and 31 deletions.
Expand Up @@ -21,6 +21,7 @@
import org.jruby.parser.StaticScope;
import org.jruby.runtime.Arity;
import org.jruby.runtime.Block;
import org.jruby.runtime.DynamicScope;
import org.jruby.runtime.Helpers;
import org.jruby.runtime.PositionAware;
import org.jruby.runtime.ThreadContext;
Expand Down Expand Up @@ -125,7 +126,14 @@ private void post(ThreadContext context) {

private void pre(ThreadContext context, IRubyObject self, String name, Block block) {
// update call stacks (push: frame, class, scope, etc.)
context.preMethodFrameAndScope(getImplementationClass(), name, self, block, method.getStaticScope());
// SSS FIXME: If this is going to slow down the common case, we could
// create a specialized version of InterpretedIRMethod for meta class bodies
if (method instanceof IRMetaClassBody) {
context.preMethodFrameAndClass(getImplementationClass(), name, self, block, method.getStaticScope());
context.pushScope(DynamicScope.newDynamicScope(method.getStaticScope(), context.getCurrentScope()));
} else {
context.preMethodFrameAndScope(getImplementationClass(), name, self, block, method.getStaticScope());
}
context.setCurrentVisibility(getVisibility());
}

Expand Down
9 changes: 4 additions & 5 deletions core/src/main/java/org/jruby/ir/IRBuilder.java
Expand Up @@ -3227,17 +3227,16 @@ public Operand buildReturn(ReturnNode returnNode, IRScope s) {

if (s instanceof IRClosure) {
// If 'm' is a block scope, a return returns from the closest enclosing method.
// If this happens to be a module body, the runtime throws a local jump error if
// the closure is a proc. If the closure is a lambda, then this is just a normal
// return and the static methodIdToReturnFrom value is ignored
// If this happens to be a module body, the runtime throws a local jump error if the
// closure is a proc. If the closure is a lambda, then this becomes a normal return.
IRMethod m = s.getNearestMethod();
addInstr(s, new NonlocalReturnInstr(retVal, m == null ? "--none--" : m.getName(), m == null ? -1 : m.getScopeId()));
addInstr(s, new NonlocalReturnInstr(retVal, m == null ? "--none--" : m.getName(), m == null));
} else if (s.isModuleBody()) {
IRMethod sm = s.getNearestMethod();

// Cannot return from top-level module bodies!
if (sm == null) addInstr(s, new ThrowExceptionInstr(IRException.RETURN_LocalJumpError));
else addInstr(s, new NonlocalReturnInstr(retVal, sm.getName(), sm.getScopeId()));
else addInstr(s, new NonlocalReturnInstr(retVal, sm.getName(), false));
} else {
addInstr(s, new ReturnInstr(retVal));
}
Expand Down
4 changes: 4 additions & 0 deletions core/src/main/java/org/jruby/ir/IRScopeType.java
Expand Up @@ -7,6 +7,10 @@ public static IRScopeType fromOrdinal(int value) {
return value < 0 || value >= values().length ? null : values()[value];
}

public boolean isMethodType() {
return this == INSTANCE_METHOD || this == CLASS_METHOD;
}

public boolean isClosureType() {
return this == CLOSURE || this == FOR || this == EVAL_SCRIPT;
}
Expand Down
@@ -1,33 +1,34 @@
package org.jruby.ir.instructions;

import org.jruby.ir.IRFlags;
import org.jruby.ir.IRMethod;
import org.jruby.ir.IRScope;
import org.jruby.ir.IRVisitor;
import org.jruby.ir.Operation;
import org.jruby.ir.operands.Boolean;
import org.jruby.ir.operands.Operand;
import org.jruby.ir.operands.Fixnum;
import org.jruby.ir.operands.StringLiteral;
import org.jruby.ir.operands.Variable;
import org.jruby.ir.transformations.inlining.InlinerInfo;

public class NonlocalReturnInstr extends ReturnBase implements FixedArityInstr {
public final String methodName;
public final int methodIdToReturnFrom;
public final boolean maybeLambda;

public NonlocalReturnInstr(Operand returnValue, String methodName, int methodIdToReturnFrom) {
public NonlocalReturnInstr(Operand returnValue, String methodName, boolean maybeLambda) {
super(Operation.NONLOCAL_RETURN, returnValue);
this.methodName = methodName;
this.methodIdToReturnFrom = methodIdToReturnFrom;
this.maybeLambda = maybeLambda;
}

@Override
public Operand[] getOperands() {
return new Operand[] { returnValue, new StringLiteral(methodName), new Fixnum(methodIdToReturnFrom) };
return new Operand[] { returnValue, new StringLiteral(methodName), new Boolean(maybeLambda) };
}

@Override
public String toString() {
return getOperation() + "(" + returnValue + ", <" + methodName + ":" + methodIdToReturnFrom + ">" + ")";
return getOperation() + "(" + returnValue + ", <" + methodName + ":" + maybeLambda + ">" + ")";
}

public boolean computeScopeFlags(IRScope scope) {
Expand All @@ -39,15 +40,15 @@ public boolean computeScopeFlags(IRScope scope) {
public Instr cloneForInlining(InlinerInfo ii) {
switch (ii.getCloneMode()) {
case CLOSURE_INLINE:
if (ii.getInlineHostScope().getScopeId() == methodIdToReturnFrom) {
if (ii.getInlineHostScope() instanceof IRMethod) {
// Treat like inlining of a regular method-return
Variable v = ii.getCallResultVariable();
return v == null ? null : new CopyInstr(v, returnValue.cloneForInlining(ii));
}
// fall through
case ENSURE_BLOCK_CLONE:
case NORMAL_CLONE:
return new NonlocalReturnInstr(returnValue.cloneForInlining(ii), methodName, methodIdToReturnFrom);
return new NonlocalReturnInstr(returnValue.cloneForInlining(ii), methodName, maybeLambda);
default:
return super.cloneForInlining(ii);
}
Expand Down
Expand Up @@ -91,10 +91,10 @@ public IRubyObject callHelper(ThreadContext context, DynamicScope currDynScope,
return IRRuntimeHelpers.handlePropagatedBreak(context, scope,
args[0].retrieve(context, self, currDynScope, temp), blockType);
case HANDLE_NONLOCAL_RETURN:
return IRRuntimeHelpers.handleNonlocalReturn(scope,
return IRRuntimeHelpers.handleNonlocalReturn(scope, currDynScope,
args[0].retrieve(context, self, currDynScope, temp), blockType);
case HANDLE_BREAK_AND_RETURNS_IN_LAMBDA:
return IRRuntimeHelpers.handleBreakAndReturnsInLambdas(context, scope,
return IRRuntimeHelpers.handleBreakAndReturnsInLambdas(context, scope, currDynScope,
args[0].retrieve(context, self, currDynScope, temp), blockType);
case IS_DEFINED_BACKREF:
return IRRuntimeHelpers.isDefinedBackref(context);
Expand Down
Expand Up @@ -427,7 +427,7 @@ private static IRubyObject processReturnOp(ThreadContext context, Instr instr, O
IRubyObject rv = (IRubyObject)retrieveOp(ri.getReturnValue(), context, self, currDynScope, temp);
// If not in a lambda, check if this was a non-local return
if (!IRRuntimeHelpers.inLambda(blockType)) {
IRRuntimeHelpers.initiateNonLocalReturn(context, (IRStaticScope)currDynScope.getStaticScope(), ri.methodIdToReturnFrom, rv);
IRRuntimeHelpers.initiateNonLocalReturn(context, currDynScope, ri.maybeLambda, rv);
}
return rv;
}
Expand Down
Expand Up @@ -179,7 +179,7 @@ public Instr decodeInner(Operation operation) {
case MATCH3: return new Match3Instr(d.decodeVariable(), d.decodeOperand(), d.decodeOperand());
case METHOD_DEFINED: return new MethodDefinedInstr(d.decodeVariable(), d.decodeOperand(), (StringLiteral) d.decodeOperand());
case METHOD_LOOKUP: return new MethodLookupInstr(d.decodeVariable(), d.decodeOperand(), d.decodeOperand());
case NONLOCAL_RETURN: return new NonlocalReturnInstr(d.decodeOperand(), d.decodeString(), d.decodeInt());
case NONLOCAL_RETURN: return new NonlocalReturnInstr(d.decodeOperand(), d.decodeString(), d.decodeBoolean());
case NOP: return NopInstr.NOP;
case NORESULT_CALL: return decodeNoResultCall();
case POP_BINDING: return new PopBindingInstr();
Expand Down
Expand Up @@ -431,7 +431,7 @@ private void encodeMethodLookupInstr(MethodLookupInstr instr) {
private void encodeNonlocalReturnInstr(NonlocalReturnInstr instr) {
e.encode(instr.getReturnValue());
e.encode(instr.methodName);
e.encode(instr.methodIdToReturnFrom);
e.encode(instr.maybeLambda);
}

private void encodeCallBaseInstr(CallBase instr) {
Expand Down
7 changes: 4 additions & 3 deletions core/src/main/java/org/jruby/ir/runtime/IRReturnJump.java
Expand Up @@ -4,9 +4,10 @@
import java.lang.ref.SoftReference;

import org.jruby.exceptions.Unrescuable;
import org.jruby.runtime.DynamicScope;

public class IRReturnJump extends RuntimeException implements Unrescuable {
public int methodToReturnFrom;
public DynamicScope methodToReturnFrom;
public Object returnValue;

private IRReturnJump() {}
Expand All @@ -17,7 +18,7 @@ private IRReturnJump() {}

private static ThreadLocal<Reference<IRReturnJump>> threadLocalRJ = new ThreadLocal<Reference<IRReturnJump>>();

public static IRReturnJump create(int scopeId, Object rv) {
public static IRReturnJump create(DynamicScope scope, Object rv) {
IRReturnJump rj;
Reference<IRReturnJump> rjRef = threadLocalRJ.get();
if (rjRef != null) {
Expand All @@ -26,7 +27,7 @@ public static IRReturnJump create(int scopeId, Object rv) {
rj = new IRReturnJump();
threadLocalRJ.set(new SoftReference<IRReturnJump>(rj));
}
rj.methodToReturnFrom = scopeId;
rj.methodToReturnFrom = scope;
rj.returnValue = rv;
return rj;
}
Expand Down
29 changes: 22 additions & 7 deletions core/src/main/java/org/jruby/ir/runtime/IRRuntimeHelpers.java
Expand Up @@ -76,8 +76,23 @@ public static boolean inProc(Block.Type blockType) {
/*
* Handle non-local returns (ex: when nested in closures, root scopes of module/class/sclass bodies)
*/
public static void initiateNonLocalReturn(ThreadContext context, IRStaticScope scope, int methodToReturnFrom, IRubyObject returnValue) {
public static void initiateNonLocalReturn(ThreadContext context, DynamicScope dynScope, boolean maybeLambda, IRubyObject returnValue) {
IRStaticScope scope = (IRStaticScope)dynScope.getStaticScope();
IRScopeType scopeType = scope.getScopeType();
while (dynScope != null) {
IRStaticScope ss = (IRStaticScope)dynScope.getStaticScope();
// SSS FIXME: Why is scopeType empty? Looks like this static-scope
// was not associated with the AST scope that got converted to IR.
//
// Ruby code: lambda { Thread.new { return }.join }.call
//
// To be investigated.
IRScopeType ssType = ss.getScopeType();
if (ssType != null && ssType.isMethodType()) {
break;
}
dynScope = dynScope.getNextCapturedScope();
}

// SSS FIXME: Why is scopeType empty? Looks like this static-scope
// was not associated with the AST scope that got converted to IR.
Expand All @@ -86,25 +101,25 @@ public static void initiateNonLocalReturn(ThreadContext context, IRStaticScope s
//
// To be investigated.
if ( (scopeType == null || (scopeType.isClosureType() && scopeType != IRScopeType.EVAL_SCRIPT))
&& (methodToReturnFrom == -1 || !context.scopeExistsOnCallStack(methodToReturnFrom)))
&& (maybeLambda || !context.scopeExistsOnCallStack(dynScope)))
{
// Cannot return from the call that we have long since exited.
throw IRException.RETURN_LocalJumpError.getException(context.runtime);
}

// methodtoReturnFrom will not be -1 for explicit returns from class/module/sclass bodies
throw IRReturnJump.create(methodToReturnFrom, returnValue);
throw IRReturnJump.create(dynScope, returnValue);
}

public static IRubyObject handleNonlocalReturn(IRStaticScope scope, Object rjExc, Block.Type blockType) throws RuntimeException {
public static IRubyObject handleNonlocalReturn(IRStaticScope scope, DynamicScope dynScope, Object rjExc, Block.Type blockType) throws RuntimeException {
if (!(rjExc instanceof IRReturnJump)) {
Helpers.throwException((Throwable)rjExc);
return null;
} else {
IRReturnJump rj = (IRReturnJump)rjExc;

// - If we are in a lambda or if we are in the method scope we are supposed to return from, stop propagating
if (inNonMethodBodyLambda(scope, blockType) || (rj.methodToReturnFrom == scope.getScopeId())) return (IRubyObject) rj.returnValue;
if (inNonMethodBodyLambda(scope, blockType) || (rj.methodToReturnFrom == dynScope)) return (IRubyObject) rj.returnValue;

// - If not, Just pass it along!
throw rj;
Expand Down Expand Up @@ -135,12 +150,12 @@ public static IRubyObject initiateBreak(ThreadContext context, IRStaticScope sco
}
}

public static IRubyObject handleBreakAndReturnsInLambdas(ThreadContext context, IRStaticScope scope, Object exc, Block.Type blockType) throws RuntimeException {
public static IRubyObject handleBreakAndReturnsInLambdas(ThreadContext context, IRStaticScope scope, DynamicScope dynScope, Object exc, Block.Type blockType) throws RuntimeException {
if ((exc instanceof IRBreakJump) && inNonMethodBodyLambda(scope, blockType)) {
// We just unwound all the way up because of a non-local break
throw IRException.BREAK_LocalJumpError.getException(context.getRuntime());
} else if (exc instanceof IRReturnJump) {
return handleNonlocalReturn(scope, exc, blockType);
return handleNonlocalReturn(scope, dynScope, exc, blockType);
} else {
// Propagate
Helpers.throwException((Throwable)exc);
Expand Down
4 changes: 2 additions & 2 deletions core/src/main/java/org/jruby/runtime/ThreadContext.java
Expand Up @@ -591,10 +591,10 @@ public boolean isJumpTargetAlive(int target, int skipFrames) {
* @return true if it exists
* false if not
**/
public boolean scopeExistsOnCallStack(int scopeId) {
public boolean scopeExistsOnCallStack(DynamicScope scope) {
DynamicScope[] stack = scopeStack;
for (int i = scopeIndex; i >= 0; i--) {
if (((IRStaticScope)stack[i].getStaticScope()).getScopeId() == scopeId) return true;
if (stack[i] == scope) return true;
}
return false;
}
Expand Down

0 comments on commit 5f5538f

Please sign in to comment.