Project

General

Profile

Actions

Bug #13844

closed

Toplevel returns should fire ensures

Added by headius (Charles Nutter) over 6 years ago. Updated over 6 years ago.

Status:
Closed
Assignee:
-
Target version:
-
[ruby-core:82492]

Description

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.


Related issues 1 (0 open1 closed)

Related to Ruby master - Bug #14061: Top-level return does not execute ensure if the file is loaded pr requiredClosedActions

Updated by headius (Charles Nutter) over 6 years ago

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

Actions #2

Updated by shevegen (Robert A. Heiler) over 6 years ago

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".

Updated by headius (Charles Nutter) over 6 years ago

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.

Updated by headius (Charles Nutter) over 6 years ago

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.

Updated by nobu (Nobuyoshi Nakada) over 6 years ago

  • 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.

Actions #6

Updated by nobu (Nobuyoshi Nakada) over 6 years ago

  • 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
Actions #7

Updated by nobu (Nobuyoshi Nakada) over 6 years ago

  • Status changed from Open to Closed

Applied in changeset trunk|r59708.


compile.c: ensure after toplevel return

Updated by Eregon (Benoit Daloze) over 6 years ago

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

Updated by nagachika (Tomoyuki Chikanaga) over 6 years ago

  • 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.

Actions #10

Updated by nobu (Nobuyoshi Nakada) over 6 years ago

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

Also available in: Atom PDF

Like0
Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0