Skip to content

Commit

Permalink
Bummer we do need to walk dynscopes to find method we are defined in …
Browse files Browse the repository at this point in the history
…so that

if it happens to be from define_method we can prevent LJE since otherwise it
would LJE by being marked as !definedWithinMethod (lexically it is within
another closure and not a method).

Ultimately, if/when cleanup happens here we should really entertain storing
the lexical method dynscope (either from IRMethod or define_method) so we are
not constantly doing this walking.  If we did that then we could eliminate
definedWithinMethod flag and also our search within dynscope for the method
dynscope.  At that point we would field access the method and make sure it
is somewhere up stack to detect if the proc is a migrated proc or not.  This
last comment might make you think we could set a flag on the proc if it
migrates but a proc can exist in n places at runtime.
enebo committed Mar 24, 2017
1 parent 30ca8c7 commit 2be06c6
Showing 1 changed file with 4 additions and 1 deletion.
Original file line number Diff line number Diff line change
@@ -84,8 +84,11 @@ public static boolean inProc(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.

dynScope = getContainingMethodsDynamicScope(dynScope);
boolean inDefineMethod = dynScope != null && dynScope.getStaticScope().isArgumentScope() && dynScope.getStaticScope().getScopeType().isBlock();

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

0 comments on commit 2be06c6

Please sign in to comment.