Treat RescueNodes like EnsureNodes to ensure $! is save-restored.

* This fixes the snippets in #2910. * More needs to be done to clean this up and get rid of other save-restore code for $! elsewhere. * Committing this to check if it successfully passes travis tests.
jruby · May 9, 2015 · bc1aec2 · bc1aec2
1 parent b765631
commit bc1aec2
Showing 1 changed file with 24 additions and 7 deletions.
diff --git a/core/src/main/java/org/jruby/ir/IRBuilder.java b/core/src/main/java/org/jruby/ir/IRBuilder.java
@@ -2214,9 +2214,16 @@ Exception region start marker_2 (protected by whichever block handles exceptions
           L3:
 
      * ****************************************************************/
-    public Operand buildEnsureNode(EnsureNode ensureNode) {
-        Node bodyNode = ensureNode.getBodyNode();
+    public Operand buildEnsureNode(final EnsureNode ensureNode) {
+        CodeBlock ensureCodeBuilder = new CodeBlock() {
+            public Operand run() {
+                return (ensureNode.getEnsureNode() == null) ? manager.getNil() : build(ensureNode.getEnsureNode());
+            };
+        };
+        return buildEnsureInternal(ensureNode.getBodyNode(), ensureCodeBuilder);
+    }
 
+    public Operand buildEnsureInternal(Node ensureBodyNode, CodeBlock ensureCodeBuilder) {
         // ------------ Build the body of the ensure block ------------
         //
         // The ensure code is built first so that when the protected body is being built,
@@ -2225,12 +2232,12 @@ public Operand buildEnsureNode(EnsureNode ensureNode) {
         // Push a new ensure block node onto the stack of ensure bodies being built
         // The body's instructions are stashed and emitted later.
         EnsureBlockInfo ebi = new EnsureBlockInfo(scope,
-            (bodyNode instanceof RescueNode) ? (RescueNode)bodyNode : null,
+            (ensureBodyNode instanceof RescueNode) ? (RescueNode)ensureBodyNode : null,
             getCurrentLoop(),
             activeRescuers.peek());
 
         ensureBodyBuildStack.push(ebi);
-        Operand ensureRetVal = (ensureNode.getEnsureNode() == null) ? manager.getNil() : build(ensureNode.getEnsureNode());
+        Operand ensureRetVal = ensureCodeBuilder.run();
         ensureBodyBuildStack.pop();
 
         // ------------ Build the protected region ------------
@@ -2242,15 +2249,15 @@ public Operand buildEnsureNode(EnsureNode ensureNode) {
         activeRescuers.push(ebi.dummyRescueBlockLabel);
 
         // Generate IR for code being protected
-        Operand rv = bodyNode instanceof RescueNode ? buildRescueInternal((RescueNode) bodyNode, ebi) : build(bodyNode);
+        Operand rv = ensureBodyNode instanceof RescueNode ? buildRescueInternal((RescueNode) ensureBodyNode, ebi) : build(ensureBodyNode);
 
         // End of protected region
         addInstr(new ExceptionRegionEndMarkerInstr());
         activeRescuers.pop();
 
         // Clone the ensure body and jump to the end.
         // Don't bother if the protected body ended in a return.
-        if (rv != U_NIL && !(bodyNode instanceof RescueNode)) {
+        if (rv != U_NIL && !(ensureBodyNode instanceof RescueNode)) {
             ebi.cloneIntoHostScope(this);
             addInstr(new JumpInstr(ebi.end));
         }
@@ -3023,7 +3030,17 @@ public Operand buildRegexp(RegexpNode reNode) {
     }
 
     public Operand buildRescue(RescueNode node) {
-        return buildRescueInternal(node, null);
+        final Variable savedGlobalException = createTemporaryVariable();
+        addInstr(new GetGlobalVariableInstr(savedGlobalException, "$!"));
+
+        CodeBlock ensureBodyBuilder = new CodeBlock() {
+            public Operand run() {
+                // Restore "$!"
+                addInstr(new PutGlobalVarInstr("$!", savedGlobalException));
+                return manager.getNil();
+            };
+        };
+        return buildEnsureInternal(node, ensureBodyBuilder);
     }
 
     private Operand buildRescueInternal(RescueNode rescueNode, EnsureBlockInfo ensure) {