Skip to content

Commit

Permalink
Replace maybeLambda in CheckForLJEInstr to be definedWithinMethod and…
Browse files Browse the repository at this point in the history
… this is

also an inversion of the logic since in check we are only concerned with
when it is not defined within a method but having not in the name just felt
wrong.

Made private helper getContainingMethodOrLambdasDynamicScope to simplify
iniateNonLocalReturn to just be the two throws along with comments to help
explain this to the uninitiated (or me in 4 months).
enebo committed Mar 23, 2017
1 parent 8253c47 commit 67da032
Showing 5 changed files with 39 additions and 28 deletions.
6 changes: 3 additions & 3 deletions core/src/main/java/org/jruby/ir/IRBuilder.java
Original file line number Diff line number Diff line change
@@ -3696,9 +3696,9 @@ public Operand buildReturn(ReturnNode returnNode) {
// 2. lambda (return) [dynamic] // FIXME: I believe ->() can be static and omit LJE check.
// 3. migrated closure (LJE) [dynamic]
// 4. eval/for (return) [static]
boolean notDefinedWithinMethod = scope.getNearestMethod() == null;
if (!(scope instanceof IREvalScript) && !(scope instanceof IRFor)) addInstr(new CheckForLJEInstr(notDefinedWithinMethod));
addInstr(new NonlocalReturnInstr(retVal, notDefinedWithinMethod ? "--none--" : scope.getNearestMethod().getName()));
boolean definedWithinMethod = scope.getNearestMethod() != null;
if (!(scope instanceof IREvalScript) && !(scope instanceof IRFor)) addInstr(new CheckForLJEInstr(definedWithinMethod));
addInstr(new NonlocalReturnInstr(retVal, definedWithinMethod ? scope.getNearestMethod().getName() : "--none--" ));
} else if (scope.isModuleBody()) {
IRMethod sm = scope.getNearestMethod();

24 changes: 16 additions & 8 deletions core/src/main/java/org/jruby/ir/instructions/CheckForLJEInstr.java
Original file line number Diff line number Diff line change
@@ -13,27 +13,35 @@
import org.jruby.runtime.ThreadContext;

public class CheckForLJEInstr extends NoOperandInstr {
private boolean maybeLambda;
// We know the proc/lambda was not lexically made within a method scope. If it is a lambda
// then it will not matter but if it is a proc and it is not found to be within a define_method
// closure then we will raise an LJE if this true.
private boolean definedWithinMethod;

public CheckForLJEInstr(boolean maybeLambda) {
public CheckForLJEInstr(boolean notDefinedWithinMethod) {
super(Operation.CHECK_FOR_LJE);

this.maybeLambda = maybeLambda;
this.definedWithinMethod = notDefinedWithinMethod;
}

@Deprecated
public boolean maybeLambda() {
return maybeLambda;
return !definedWithinMethod;
}

public boolean isDefinedWithinMethod() {
return definedWithinMethod;
}

@Override
public Instr clone(CloneInfo info) {
return new CheckForLJEInstr(maybeLambda);
return new CheckForLJEInstr(definedWithinMethod);
}

@Override
public void encode(IRWriterEncoder e) {
super.encode(e);
e.encode(maybeLambda());
e.encode(isDefinedWithinMethod());
}

public static CheckForLJEInstr decode(IRReaderDecoder d) {
@@ -46,11 +54,11 @@ public void visit(IRVisitor visitor) {

@Override
public String[] toStringNonOperandArgs() {
return new String[] { "maybe_lambda: " + maybeLambda};
return new String[] { "definedWithinMethod: " + definedWithinMethod};
}

public void check(ThreadContext context, DynamicScope dynamicScope, Block.Type blockType) {
IRRuntimeHelpers.checkForLJE(context, dynamicScope, maybeLambda, blockType);
IRRuntimeHelpers.checkForLJE(context, dynamicScope, definedWithinMethod, blockType);
}

@Override
9 changes: 4 additions & 5 deletions core/src/main/java/org/jruby/ir/runtime/IRReturnJump.java
Original file line number Diff line number Diff line change
@@ -2,19 +2,18 @@

import org.jruby.exceptions.Unrescuable;
import org.jruby.runtime.DynamicScope;
import org.jruby.util.cli.Options;

public class IRReturnJump extends IRJump implements Unrescuable {
public final DynamicScope methodToReturnFrom;
public final Object returnValue;

private IRReturnJump(DynamicScope scope, Object rv) {
this.methodToReturnFrom = scope;
private IRReturnJump(DynamicScope scopeToReturnFrom, Object rv) {
this.methodToReturnFrom = scopeToReturnFrom;
this.returnValue = rv;
}

public static IRReturnJump create(DynamicScope scope, Object rv) {
return new IRReturnJump(scope, rv);
public static IRReturnJump create(DynamicScope scopeToReturnFrom, Object rv) {
return new IRReturnJump(scopeToReturnFrom, rv);
}

@Override
26 changes: 15 additions & 11 deletions core/src/main/java/org/jruby/ir/runtime/IRRuntimeHelpers.java
Original file line number Diff line number Diff line change
@@ -81,16 +81,23 @@ public static boolean inProc(Block.Type blockType) {

// FIXME: ENEBO: If we inline this instr then dynScope will be for the inlined dynscope and that scope could be many things.
// CheckForLJEInstr.clone should convert this as appropriate based on what it is being inlined into.
public static void checkForLJE(ThreadContext context, DynamicScope dynScope, boolean notDefinedWithinMethod, Block.Type blockType) {
public static void checkForLJE(ThreadContext context, DynamicScope dynScope, boolean definedWithinMethod, Block.Type blockType) {
if (inLambda(blockType)) return; // break/return in lambda unconditionally a return.

// Is our proc in something unreturnable (e.g. module/class) or has it migrated (lexical parent method not in stack any more)?
if (notDefinedWithinMethod || !context.scopeExistsOnCallStack(getContainingMethodsDynamicScope(dynScope))) {
if (!definedWithinMethod || !context.scopeExistsOnCallStack(getContainingMethodsDynamicScope(dynScope))) {
throw IRException.RETURN_LocalJumpError.getException(context.runtime);
}
}

// Finds dynamic method this proc exists in or null if it is not within one.
// Create a jump for a non-local return which will return from nearest lambda (which may be itself) or method.
public static IRubyObject initiateNonLocalReturn(ThreadContext context, DynamicScope dynScope, Block.Type blockType, IRubyObject returnValue) {
if (IRRuntimeHelpers.inLambda(blockType)) throw new IRWrappedLambdaReturnValue(returnValue);

throw IRReturnJump.create(getContainingMethodOrLambdasDynamicScope(dynScope), returnValue);
}

// Finds dynamicscope method this proc exists in or null if it is not within one.
private static DynamicScope getContainingMethodsDynamicScope(DynamicScope dynScope) {
for (; dynScope != null; dynScope = dynScope.getParentScope()) {
StaticScope scope = dynScope.getStaticScope();
@@ -103,22 +110,19 @@ private static DynamicScope getContainingMethodsDynamicScope(DynamicScope dynSco
return null;
}

/*
* Handle non-local returns (ex: when nested in closures, root scopes of module/class/sclass bodies)
*/
public static IRubyObject initiateNonLocalReturn(ThreadContext context, DynamicScope dynScope, Block.Type blockType, IRubyObject returnValue) {
if (IRRuntimeHelpers.inLambda(blockType)) throw new IRWrappedLambdaReturnValue(returnValue);

// Finds dynamicscope method or lambda this proc exists in or null if it is not within one.
private static DynamicScope getContainingMethodOrLambdasDynamicScope(DynamicScope dynScope) {
// If not in a lambda, check if this was a non-local return
for (; dynScope != null; dynScope = dynScope.getParentScope()) {
StaticScope staticScope = dynScope.getStaticScope();
IRScope scope = staticScope.getIRScope();

// 1) method 2) lambda 3) closure (define_method) for zsuper
if (scope instanceof IRMethod || (scope instanceof IRClosure && (dynScope.isLambda() || staticScope.isArgumentScope()))) break;
if (scope instanceof IRMethod ||
(scope instanceof IRClosure && (dynScope.isLambda() || staticScope.isArgumentScope()))) return dynScope;
}

throw IRReturnJump.create(dynScope, returnValue);
return null;
}

@JIT
2 changes: 1 addition & 1 deletion core/src/main/java/org/jruby/ir/targets/JVMVisitor.java
Original file line number Diff line number Diff line change
@@ -1134,7 +1134,7 @@ private void checkArity(int required, int opt, boolean rest, boolean receivesKey
public void CheckForLJEInstr(CheckForLJEInstr checkForljeinstr) {
jvmMethod().loadContext();
jvmLoadLocal(DYNAMIC_SCOPE);
jvmAdapter().ldc(checkForljeinstr.maybeLambda());
jvmAdapter().ldc(checkForljeinstr.isDefinedWithinMethod());
jvmMethod().loadBlockType();
jvmAdapter().invokestatic(p(IRRuntimeHelpers.class), "checkForLJE", sig(void.class, ThreadContext.class, DynamicScope.class, boolean.class, Block.Type.class));
}

0 comments on commit 67da032

Please sign in to comment.