Restructure and simplify ensure a bit en route to fix.

jruby · May 9, 2015 · 5415730 · 5415730
1 parent 8a9d5be
commit 5415730
Show file tree

Hide file tree

Showing 3 changed files with 17 additions and 24 deletions.
diff --git a/core/src/main/java/org/jruby/ir/IRBuilder.java b/core/src/main/java/org/jruby/ir/IRBuilder.java
@@ -354,11 +354,6 @@ private void emitEnsureBlocks(IRLoop loop) {
             // ensure blocks from the loops they are present in.
             if (loop != null && ebi.innermostLoop != loop) break;
 
-            // SSS FIXME: Should $! be restored before or after the ensure block is run?
-            if (ebi.savedGlobalException != null) {
-                addInstr(new PutGlobalVarInstr("$!", ebi.savedGlobalException));
-            }
-
             // Clone into host scope
             ebi.cloneIntoHostScope(this);
         }
@@ -2215,12 +2210,8 @@ Exception region start marker_2 (protected by whichever block handles exceptions
 
      * ****************************************************************/
     public Operand buildEnsureNode(EnsureNode ensureNode) {
-        // Save $! in a temp var so it can be restored when the exception gets handled.
-        Variable savedGlobalException = createTemporaryVariable();
-        addInstr(new GetGlobalVariableInstr(savedGlobalException, "$!"));
-
         // prepare $!-clearing ensure block
-        EnsureBlockInfo lastErrorReset = resetLastErrorPreamble(savedGlobalException);
+        EnsureBlockInfo lastErrorReset = resetLastErrorPreamble();
 
         Node bodyNode = ensureNode.getBodyNode();
 
@@ -2249,7 +2240,7 @@ public Operand buildEnsureNode(EnsureNode ensureNode) {
         activeRescuers.push(ebi.dummyRescueBlockLabel);
 
         // Generate IR for code being protected
-        Operand rv = bodyNode instanceof RescueNode ? buildRescueInternal((RescueNode) bodyNode, ebi, savedGlobalException) : build(bodyNode);
+        Operand rv = bodyNode instanceof RescueNode ? buildRescueInternal((RescueNode) bodyNode, ebi, lastErrorReset.savedGlobalException) : build(bodyNode);
 
         // End of protected region
         addInstr(new ExceptionRegionEndMarkerInstr());
@@ -3033,15 +3024,11 @@ public Operand buildRegexp(RegexpNode reNode) {
     }
 
     public Operand buildRescue(RescueNode node) {
-        // Save $! in a temp var so it can be restored when the exception gets handled.
-        Variable savedGlobalException = createTemporaryVariable();
-        addInstr(new GetGlobalVariableInstr(savedGlobalException, "$!"));
-
         // prepare $!-clearing ensure block
-        EnsureBlockInfo lastErrorReset = resetLastErrorPreamble(savedGlobalException);
+        EnsureBlockInfo lastErrorReset = resetLastErrorPreamble();
 
         // build the rescue itself
-        Operand rv = buildRescueInternal(node, null, savedGlobalException);
+        Operand rv = buildRescueInternal(node, null, lastErrorReset.savedGlobalException);
 
         // close out the $!-clearing region
         resetLastErrorPostamble(lastErrorReset);
@@ -3077,12 +3064,18 @@ private void resetLastErrorPostamble(EnsureBlockInfo lastErrorReset) {
         addInstr(new LabelInstr(lastErrorReset.end));
     }
 
-    private EnsureBlockInfo resetLastErrorPreamble(Variable savedGlobalException) {
+    private EnsureBlockInfo resetLastErrorPreamble() {
+        // Save $! in a temp var so it can be restored when the exception gets handled.
+        Variable savedGlobalException = createTemporaryVariable();
+        addInstr(new GetGlobalVariableInstr(savedGlobalException, "$!"));
+
         EnsureBlockInfo lastErrorReset = new EnsureBlockInfo(scope,
                 null,
                 getCurrentLoop(),
                 activeRescuers.peek());
 
+        lastErrorReset.savedGlobalException = savedGlobalException;
+
         lastErrorReset.addInstr(new PutGlobalVarInstr("$!", savedGlobalException));
 
         // ------------ Build the protected region ------------
@@ -3096,12 +3089,6 @@ private EnsureBlockInfo resetLastErrorPreamble(Variable savedGlobalException) {
     }
 
     private Operand buildRescueInternal(RescueNode rescueNode, EnsureBlockInfo ensure, Variable savedGlobalException) {
-        // Wrap the entire begin+rescue+ensure+else with $!-resetting "finally" logic
-
-        // Save $! in a temp var so it can be restored when the exception gets handled.
-        addInstr(new GetGlobalVariableInstr(savedGlobalException, "$!"));
-        if (ensure != null) ensure.savedGlobalException = savedGlobalException;
-
         // Labels marking start, else, end of the begin-rescue(-ensure)-end block
         Label rBeginLabel = ensure == null ? getNewLabel() : ensure.regionStart;
         Label rEndLabel   = ensure == null ? getNewLabel() : ensure.end;

diff --git a/core/src/main/java/org/jruby/runtime/ContextAwareBlockBody.java b/core/src/main/java/org/jruby/runtime/ContextAwareBlockBody.java
@@ -1,5 +1,6 @@
 package org.jruby.runtime;
 
+import org.jruby.EvalType;
 import org.jruby.parser.StaticScope;
 
 /**

diff --git a/core/src/main/java/org/jruby/runtime/MethodBlockBody.java b/core/src/main/java/org/jruby/runtime/MethodBlockBody.java
@@ -1,5 +1,6 @@
 package org.jruby.runtime;
 
+import org.jruby.EvalType;
 import org.jruby.RubyModule;
 import org.jruby.internal.runtime.methods.DynamicMethod;
 import org.jruby.parser.StaticScope;
@@ -85,4 +86,8 @@ public int getLine() {
     public ArgumentDescriptor[] getArgumentDescriptors() {
         return argsDesc;
     }
+
+    @Override
+    public void setEvalType(EvalType evalType) {
+    }
 }