Skip to content

Commit

Permalink
Fix #3356: Copy value of ensure-protected body in a tmp and return it
Browse files Browse the repository at this point in the history
* My memory of IR details are a bit fuzzy now since I have been
  away for so long, but this should work.
* Updated internal copy of JRuby rubyspec with new expectations.
subbuss committed Oct 9, 2015
1 parent e24b6e2 commit 89efda6
Showing 2 changed files with 31 additions and 4 deletions.
13 changes: 9 additions & 4 deletions core/src/main/java/org/jruby/ir/IRBuilder.java
Original file line number Diff line number Diff line change
@@ -2246,15 +2246,20 @@ public Operand buildEnsureInternal(Node ensureBodyNode, Node ensurerNode) {
activeRescuers.push(ebi.dummyRescueBlockLabel);

// Generate IR for code being protected
Variable ensureExprValue = createTemporaryVariable();
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
// OR if we are really processing a rescue node
if (ensurerNode != null && rv != U_NIL && !(ensureBodyNode instanceof RescueNode)) {
// Is this a begin..(rescue..)?ensure..end node that actually computes a value?
// (vs. returning from protected body)
boolean isEnsureExpr = ensurerNode != null && rv != U_NIL && !(ensureBodyNode instanceof RescueNode);

// Clone the ensure body and jump to the end
if (isEnsureExpr) {
addInstr(new CopyInstr(ensureExprValue, rv));
ebi.cloneIntoHostScope(this);
addInstr(new JumpInstr(ebi.end));
}
@@ -2290,7 +2295,7 @@ public Operand buildEnsureInternal(Node ensureBodyNode, Node ensurerNode) {
// End label for the exception region
addInstr(new LabelInstr(ebi.end));

return rv;
return isEnsureExpr ? ensureExprValue : rv;
}

public Operand buildEvStr(EvStrNode node) {
22 changes: 22 additions & 0 deletions spec/ruby/language/ensure_spec.rb
Original file line number Diff line number Diff line change
@@ -74,6 +74,28 @@
end
end

describe "The value of an ensure expression," do
it "in no-exception scenarios, is the value of the last stmt of the protected body" do
begin
v = 1
eval('x=1') # to prevent opts from triggering
v
ensure
v = 2
end.should == 1
end

it "when an exception is rescued, is the value of the rescuing block" do
begin
raise 'foo'
rescue
v = 3
ensure
v = 2
end.should == 3
end
end

describe "An ensure block inside a method" do
before :each do
@obj = EnsureSpec::Container.new

0 comments on commit 89efda6

Please sign in to comment.