Let IC determine new dynamic scope (now with closure fail fast code)

jruby · Oct 20, 2014 · 39f4e92 · subbuss · Oct 20, 2014 · enebo
1 parent b11ff3f
commit 39f4e92
Show file tree

Hide file tree

Showing 4 changed files with 26 additions and 20 deletions.
diff --git a/core/src/main/java/org/jruby/ir/IRScope.java b/core/src/main/java/org/jruby/ir/IRScope.java
@@ -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;

diff --git a/core/src/main/java/org/jruby/ir/interpreter/Interpreter.java b/core/src/main/java/org/jruby/ir/interpreter/Interpreter.java
@@ -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();
@@ -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));
                     } else {
                         processBookKeepingOp(context, instr, operation, name, args, self, block, implClass, visibility);
                     }

diff --git a/core/src/main/java/org/jruby/ir/operands/ClosureInterpreterContext.java b/core/src/main/java/org/jruby/ir/operands/ClosureInterpreterContext.java
@@ -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;
@@ -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();

diff --git a/core/src/main/java/org/jruby/ir/operands/InterpreterContext.java b/core/src/main/java/org/jruby/ir/operands/InterpreterContext.java
@@ -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;
@@ -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;