Skip to content

Commit

Permalink
Let IC determine new dynamic scope (now with closure fail fast code)
Browse files Browse the repository at this point in the history
  • Loading branch information
enebo committed Oct 20, 2014
1 parent b11ff3f commit 39f4e92
Show file tree
Hide file tree
Showing 4 changed files with 26 additions and 20 deletions.
3 changes: 2 additions & 1 deletion core/src/main/java/org/jruby/ir/IRScope.java
Original file line number Diff line number Diff line change
Expand Up @@ -622,7 +622,8 @@ public synchronized InterpreterContext prepareForInterpretation() {
// Linearize CFG, etc.
Instr[] linearizedInstrArray = prepareInstructions();

interpreterContext = new InterpreterContext(staticScope, getTemporaryVariablesCount(), getBooleanVariablesCount(),
interpreterContext = new InterpreterContext(staticScope, this instanceof IRMetaClassBody,
getTemporaryVariablesCount(), getBooleanVariablesCount(),
getFixnumVariablesCount(), getFloatVariablesCount(), getFlags(), linearizedInstrArray);

return interpreterContext;
Expand Down
18 changes: 1 addition & 17 deletions core/src/main/java/org/jruby/ir/interpreter/Interpreter.java
Original file line number Diff line number Diff line change
Expand Up @@ -508,21 +508,6 @@ private static void processOtherOp(ThreadContext context, Instr instr, Operation
}
}

private static DynamicScope getNewDynScope(ThreadContext context, IRScope scope, StaticScope currScope) {
// SSS NOTE: Method/module scopes only!
//
// Blocks are a headache -- so, these instrs. are only added to IRMethods.
// Blocks have more complicated logic for pushing a dynamic scope (see InterpretedIRBlockBody)
if (scope instanceof IRMetaClassBody) {
// Add a parent-link to current dynscope to support non-local returns cheaply
// This doesn't affect variable scoping since local variables will all have
// the right scope depth.
return DynamicScope.newDynamicScope(currScope, context.getCurrentScope());
} else {
return DynamicScope.newDynamicScope(currScope);
}
}

private static IRubyObject interpret(ThreadContext context, IRubyObject self,
IRScope scope, Visibility visibility, RubyModule implClass, String name, IRubyObject[] args, Block block, Block.Type blockType) {
InterpreterContext interpreterContext = scope.getInterpreterContext();
Expand Down Expand Up @@ -585,8 +570,7 @@ private static IRubyObject interpret(ThreadContext context, IRubyObject self,
break;
case BOOK_KEEPING_OP:
if (operation == Operation.PUSH_BINDING) {
currDynScope = getNewDynScope(context, scope, currScope);
context.pushScope(currDynScope);
context.pushScope(interpreterContext.newDynamicScope(context));

This comment has been minimized.

Copy link
@subbuss

subbuss Oct 20, 2014

Contributor

This is incorrect. You still need to update currDynScope -- it is live and used within the interp loop.

This comment has been minimized.

Copy link
@enebo

enebo Oct 21, 2014

Author Member

For posterity @subbuss corrected this in a later patch.

} else {
processBookKeepingOp(context, instr, operation, name, args, self, block, implClass, visibility);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ public ClosureInterpreterContext(StaticScope scope, int temporaryVariablecount,
int temporaryFixnumVariablecount, int temporaryFloatVariablecount,
EnumSet<IRFlags> flags, Instr[] instructions,
Operand self, StaticScope staticScope, BlockBody body) {
super(scope, temporaryVariablecount, temporaryBooleanVariablecount, temporaryFixnumVariablecount,
super(scope, false, temporaryVariablecount, temporaryBooleanVariablecount, temporaryFixnumVariablecount,
temporaryFloatVariablecount, flags, instructions);

this.self = self;
Expand All @@ -34,6 +34,15 @@ public ClosureInterpreterContext(StaticScope scope, int temporaryVariablecount,

public StaticScope getStaticScope() { return staticScope; }

/**
* Blocks have more complicated logic for pushing a dynamic scope (see InterpretedIRBlockBody).
* We throw an error in case somehow we mistakenly try and push a binding.
*/
@Override
public DynamicScope newDynamicScope(ThreadContext context) {
throw new RuntimeException("We do not push bindings for closures");
}

@Override
public Object retrieve(ThreadContext context, IRubyObject self, StaticScope currScope, DynamicScope currDynScope, Object[] temp) {
staticScope.determineModule();
Expand Down
14 changes: 13 additions & 1 deletion core/src/main/java/org/jruby/ir/operands/InterpreterContext.java
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,16 @@ public class InterpreterContext extends Operand {
private final boolean reuseParentDynScope;
private final boolean popDynScope;
private final boolean receivesKeywordArguments;
private final boolean metaClassBodyScope;

public InterpreterContext(StaticScope staticScope,
public InterpreterContext(StaticScope staticScope, boolean metaClassBodyScope,
int temporaryVariablecount, int temporaryBooleanVariablecount,
int temporaryFixnumVariablecount, int temporaryFloatVariablecount,
EnumSet<IRFlags> flags, Instr[] instructions) {
super(null);

this.staticScope = staticScope;
this.metaClassBodyScope = metaClassBodyScope; // IRMetaClassBody
this.temporaryVariablecount = temporaryVariablecount;
this.temporaryBooleanVariablecount = temporaryBooleanVariablecount;
this.temporaryFixnumVariablecount = temporaryFixnumVariablecount;
Expand Down Expand Up @@ -77,6 +79,16 @@ public Instr[] getInstructions() {
return instructions;
}

/**
* Get a new dynamic scope. Note: This only works for method scopes (ClosureIC will throw).
*/
public DynamicScope newDynamicScope(ThreadContext context) {
// Add a parent-link to current dynscope to support non-local returns cheaply. This doesn't
// affect variable scoping since local variables will all have the right scope depth.
if (metaClassBodyScope) return DynamicScope.newDynamicScope(staticScope, context.getCurrentScope());

return DynamicScope.newDynamicScope(staticScope);
}

public boolean hasExplicitCallProtocol() {
return hasExplicitCallProtocol;
Expand Down

0 comments on commit 39f4e92

Please sign in to comment.