Bug #13844: Toplevel returns should fire ensures

• Status: Closed
• Assignee: -
• Target version: -
• ruby -v: 2.4.1
• Backport: 2.2: DONTNEED, 2.3: DONTNEED, 2.4: DONE

[headius (Charles Nutter)]

In the following contexts, a return always fires the ensure that wraps it:

[] ~/projects/ruby $ ruby -e 'def foo; return; ensure; p :x; end; foo'
:x

[] ~/projects/ruby $ ruby -e 'def foo; 1.times { begin; return; ensure; p :x; end }; end; foo'
:x

However the new 2.4 support for toplevel returns does not fire ensures:

$ ruby -e 'begin; return; ensure; p :x; end'
<no output>

I believe this is inconsistent with how returns work everywhere else (both valid and invalid returns always fire ensure) and it should be changed to match.

[headius (Charles Nutter)]

See https://github.com/jruby/jruby/issues/4761 for our placeholder bug for this missing feature.

[shevegen (Robert A. Heiler)]

This is indeed weird that the blocks are treated differently depending on whether they
are defined in a method or outside of it. I do not assume that this was intentional.

On a side note, interestingly, the last variant aka:

begin
  return
ensure
  p :x
end

Yields no output when put into a .rb file and run via "ruby", but in IRB I get "LocalJumpError: unexpected return".

[headius (Charles Nutter)]

I do not assume it was intentional, but it was codified by tests added along with the toplevel return change. See test_syntax.rb in test_return_toplevel, where it tests that each of the following lines should exit silently:

return; raise
begin return; rescue SystemExit; exit false; end
begin return; ensure exit false; end
begin ensure return; end
begin raise; ensure; return; end
begin raise; rescue; return; end
return false; raise
return 1; raise

The first ensure line there is in my opinion incorrect; it should exit with failure.

The funny thing about this case is that the first ensure is not expected to fire, and the other two are expected to fire. I don't think the intent of toplevel return was to immediately exit the current script. I think the intent was to unroll the stack back to above the require or load that loaded this script, exactly like unrolling the stack to the method that called this one in a normal method ensure.

[headius (Charles Nutter)]

Oh also...IRB is a very weird beast. The code you enter isn't really running at toplevel; it's running in an eval that's likely within a block or method scope (I haven't looked recently). As a result, things you'd expect to work at toplevel (like returns, now) don't behave the same way. This bug is only about returns at the true top level scope in a Ruby file or -e script.

[nobu (Nobuyoshi Nakada)]

• Tracker changed from Bug to Feature
• ruby -v deleted (ruby 2.4.1p111 (2017-03-22 revision 58053) [x86_64-darwin16])
• Backport deleted (2.2: UNKNOWN, 2.3: UNKNOWN, 2.4: UNKNOWN)

The behavior was intentional.

[nobu (Nobuyoshi Nakada)]

• Tracker changed from Feature to Bug
• ruby -v set to 2.4.1
• Backport set to 2.2: DONTNEED, 2.3: DONTNEED, 2.4: REQUIRED

[nobu (Nobuyoshi Nakada)]

• Status changed from Open to Closed

Applied in changeset trunk|r59708.

---

compile.c: ensure after toplevel return

• compile.c (iseq_compile_each0): toplevel returns should fire
  ensures. [ruby-core:82492] [Bug #13844]

[Eregon (Benoit Daloze)]

Agreed, ensure should run in all cases escaping the scope, even more so when it lexically encloses the top-level return.

[nagachika (Tomoyuki Chikanaga)]

• Backport changed from 2.2: DONTNEED, 2.3: DONTNEED, 2.4: REQUIRED to 2.2: DONTNEED, 2.3: DONTNEED, 2.4: DONE

ruby_2_4 r59812 merged revision(s) 59708.

[nobu (Nobuyoshi Nakada)]

• Related to Bug #14061: Top-level return does not execute ensure if the file is loaded pr required added