Skip to content

Commit cb5e806

Browse files
committed
yay. prism rescue is fixed for cases
1 parent 936594f commit cb5e806

1 file changed

Lines changed: 17 additions & 14 deletions

File tree

core/src/main/java/org/jruby/ir/builder/IRBuilder.java

Lines changed: 17 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,8 @@ protected int currentLine() {
170170
return lastProcessedLineNum;
171171
}
172172

173+
// FIXME: When legacy parser goes away this should be totally rewritten as prism represents the whole structure as one node vs
174+
// legacy which has ensure as the parent to any rescue.
173175
// FIXME: consider mod_rescue, rescue, and pure ensure as separate entries
174176
/**
175177
* @param body the main statements to be executed
@@ -184,18 +186,17 @@ protected int currentLine() {
184186
*
185187
* Note: reference is only passed in via Prism on legacy this is desugared into AST.
186188
*/
187-
protected Operand buildEnsureInternal(U body, U elseNode, U[] exceptions, U rescueBody, X optRescue, boolean isModifier,
188-
U ensureNode, boolean isRescue, U reference) {
189+
protected Operand buildEnsureInternal(U body, U elseNode, U[] exceptions, U rescueBody, X optRescue,
190+
boolean isModifier, U ensureNode, boolean isRescue, U reference) {
189191
var savedGlobalException = addResultInstr(new GetGlobalVariableInstr(temp(), symbol("$!")));
190192

191193
// The ensure code is built first so that when the protected body is being built,
192194
// the ensure code can be cloned at break/next/return sites in the protected body.
193195
EnsureBlockInfo ebi = new EnsureBlockInfo(scope, getCurrentLoop(), activeRescuers.peek(), currentLine() + 1);
194196

195-
/// // ENEBO: If this is ensure it won't save $! if it is empty ensure what does it need to do? It needs a
196-
// path to catch and rethrow
197-
// Rescue will change $! but we need to restore $! later.
198-
if (isRescue) ebi.savedGlobalException = savedGlobalException;
197+
// Rescue will change $! but we need to restore $! later. prism: ensure and isRescue means it is both (we
198+
// call this method again below to rip those two things apart.
199+
if (isRescue && ensureNode == null) ebi.savedGlobalException = savedGlobalException;
199200

200201
// Record body of ensure and push to ensure body stack if there is an actual ensure body.
201202
Operand ensureRetVal = processEnsureBody(ensureNode, ebi);
@@ -211,7 +212,12 @@ protected Operand buildEnsureInternal(U body, U elseNode, U[] exceptions, U resc
211212
// Generate IR for code being protected
212213
Operand rv;
213214
if (isRescue) {
214-
rv = buildRescueInternal(body, elseNode, exceptions, rescueBody, optRescue, isModifier, ebi, reference);
215+
if (ensureNode != null) {
216+
// both were passed in via Prism. Let's pretend they are nested like legacy where ensures enclose the rescue.
217+
rv = buildEnsureInternal(body, elseNode, exceptions, rescueBody, optRescue, isModifier, null, isRescue, reference);
218+
} else {
219+
rv = buildRescueInternal(body, elseNode, exceptions, rescueBody, optRescue, isModifier, ebi, reference);
220+
}
215221
} else {
216222
rv = build(body);
217223
}
@@ -222,7 +228,7 @@ protected Operand buildEnsureInternal(U body, U elseNode, U[] exceptions, U resc
222228

223229
// Is this a begin..(rescue..)?ensure..end node that actually computes a value?
224230
// (vs. returning from protected body)
225-
boolean isEnsureExpr = ensureNode != null && rv != U_NIL && !isRescue;
231+
boolean isEnsureExpr = ensureNode != null && rv != U_NIL;
226232

227233
Variable ensureExprValue = temp();
228234
// Clone the ensure body and jump to the end
@@ -247,14 +253,11 @@ protected Operand buildEnsureInternal(U body, U elseNode, U[] exceptions, U resc
247253
// 1. Ensure block has no explicit return => the result of the entire ensure expression is the result of the protected body.
248254
// 2. Ensure block has an explicit return => the result of the protected body is ignored.
249255
// U_NIL => there was a return from within the ensure block!
256+
// FIXME: This U_NIL case is wrong in a few cases: 'ensure; return 1 if true; end' or weirder 'ensure; return 1; true; end'
250257
if (ensureRetVal == U_NIL) rv = U_NIL;
251258

252-
// Return (rethrow exception/end)
253-
// rethrows the caught exception from the dummy ensure block
254-
addInstr(new ThrowExceptionInstr(exc));
255-
256-
// End label for the exception region
257-
addInstr(new LabelInstr(ebi.end));
259+
addInstr(new ThrowExceptionInstr(exc)); // rethrows the caught exception from the dummy ensure block
260+
addInstr(new LabelInstr(ebi.end)); // End label for the exception region
258261

259262
return isEnsureExpr ? ensureExprValue : rv;
260263
}

0 commit comments

Comments
 (0)