[ruby-core:120034] [Ruby master Bug#20917] redo/next in nested begin block causes wrong order of execution
Issue #20917 has been reported by hoshiumiarata (Arata Hoshiumi). ---------------------------------------- Bug #20917: redo/next in nested begin block causes wrong order of execution https://bugs.ruby-lang.org/issues/20917 * Author: hoshiumiarata (Arata Hoshiumi) * Status: Open * ruby -v: ruby 3.3.5 (2024-09-03 revision ef084cc8f4) [x86_64-darwin24] * Backport: 3.1: UNKNOWN, 3.2: UNKNOWN, 3.3: UNKNOWN ---------------------------------------- It seems that `redo`/`next` in a nested `begin` block can cause the wrong order of execution. For example: ```ruby for _ in [0] puts 0 begin puts 1 begin puts 2 redo ensure puts 3 end ensure puts 4 break end end ``` It prints: ``` 0 1 2 3 4 3 4 => nil ``` But I think it should print: ``` 0 1 2 3 4 => nil ``` Because execution order should be: 1. `puts 0` 2. `puts 1` 3. `puts 2` 4. `redo` 5. unwind to nested `ensure` block 6. `puts 3` 7. unwind to outer `ensure` block 8. `puts 4` 9. `break` 10. end of loop Interestingly enough, if we add an empty `rescue` block before any of the `ensure` blocks, then the execution order is correct. -- https://bugs.ruby-lang.org/
Issue #20917 has been updated by Eregon (Benoit Daloze). Indeed, it seems like a bug, I think as well the "redo jump/unwind/exception" should run ensure's and get to the "break jump/unwind/exception" which should override the `redo` like when a Ruby exception overrides another. FWIW, this is `0 1 2 3 4` on both TruffleRuby and JRuby. ---------------------------------------- Bug #20917: redo/next in nested begin block causes wrong order of execution https://bugs.ruby-lang.org/issues/20917#change-110779 * Author: hoshiumiarata (Arata Hoshiumi) * Status: Open * ruby -v: ruby 3.3.5 (2024-09-03 revision ef084cc8f4) [x86_64-darwin24] * Backport: 3.1: UNKNOWN, 3.2: UNKNOWN, 3.3: UNKNOWN ---------------------------------------- It seems that `redo`/`next` in a nested `begin` block can cause the wrong order of execution. For example: ```ruby for _ in [0] puts 0 begin puts 1 begin puts 2 redo ensure puts 3 end ensure puts 4 break end end ``` It prints: ``` 0 1 2 3 4 3 4 => nil ``` But I think it should print: ``` 0 1 2 3 4 => nil ``` Because execution order should be: 1. `puts 0` 2. `puts 1` 3. `puts 2` 4. `redo` 5. unwind to nested `ensure` block 6. `puts 3` 7. unwind to outer `ensure` block 8. `puts 4` 9. `break` 10. end of loop Interestingly enough, if we add an empty `rescue` block before any of the `ensure` blocks, then the execution order is correct. -- https://bugs.ruby-lang.org/
Issue #20917 has been updated by kddnewton (Kevin Newton). Agreed this is a bug. For additional context, this is the same on parse.y and prism. ---------------------------------------- Bug #20917: redo/next in nested begin block causes wrong order of execution https://bugs.ruby-lang.org/issues/20917#change-110821 * Author: hoshiumiarata (Arata Hoshiumi) * Status: Open * ruby -v: ruby 3.3.5 (2024-09-03 revision ef084cc8f4) [x86_64-darwin24] * Backport: 3.1: UNKNOWN, 3.2: UNKNOWN, 3.3: UNKNOWN ---------------------------------------- It seems that `redo`/`next` in a nested `begin` block can cause the wrong order of execution. For example: ```ruby for _ in [0] puts 0 begin puts 1 begin puts 2 redo ensure puts 3 end ensure puts 4 break end end ``` It prints: ``` 0 1 2 3 4 3 4 => nil ``` But I think it should print: ``` 0 1 2 3 4 => nil ``` Because execution order should be: 1. `puts 0` 2. `puts 1` 3. `puts 2` 4. `redo` 5. unwind to nested `ensure` block 6. `puts 3` 7. unwind to outer `ensure` block 8. `puts 4` 9. `break` 10. end of loop Interestingly enough, if we add an empty `rescue` block before any of the `ensure` blocks, then the execution order is correct. -- https://bugs.ruby-lang.org/
Issue #20917 has been updated by wanabe (_ wanabe). I simplified the script to make it easier to read in ISEQ. ``` 1.times do begin redo ensure end ensure puts 1 break end ``` ``` $ ./miniruby -v --dump=insn a.rb ruby 4.1.0dev (2026-08-30T06:52:45Z :detached: a889da909c) +PRISM [x86_64-linux] == disasm: #<ISeq:<main>@a.rb:1 (1,0)-(9,3)> == catch table | catch type: break st: 0000 ed: 0004 sp: 0000 cont: 0004 | == disasm: #<ISeq:block in <main>@a.rb:1 (1,8)-(9,3)> | == catch table | | catch type: ensure st: 0001 ed: 0001 sp: 0001 cont: 0014 | | == disasm: #<ISeq:ensure in block in <main>@a.rb:4 (4,2)-(5,5)> | | local table (size: 1, argc: 0 [opts: 0, rest: -1, post: 0, block: -1, kw: -1@-1, kwrest: -1]) | | [ 1] "$!"@0 | | 0000 getlocal_WC_0 "$!"@0 ( 4) | | 0002 throw 0 | | catch type: ensure st: 0001 ed: 0013 sp: 0001 cont: 0014 | | catch type: ensure st: 0001 ed: 0001 sp: 0001 cont: 0023 | | == disasm: #<ISeq:ensure in block in <main>@a.rb:7 (7,2)-(8,7)> | | local table (size: 1, argc: 0 [opts: 0, rest: -1, post: 0, block: -1, kw: -1@-1, kwrest: -1]) | | [ 1] "$!"@0 | | 0000 putself ( 7)[Li] | | 0001 putobject_INT2FIX_1_ | | 0002 opt_send_without_block <calldata!mid:puts, argc:1, FCALL|ARGS_SIMPLE> | | 0004 pop | | 0005 putnil ( 8)[Li] | | 0006 throw 2 | | 0008 pop | | 0009 getlocal_WC_0 "$!"@0 ( 6) | | 0011 throw 0 | | catch type: ensure st: 0010 ed: 0014 sp: 0001 cont: 0023 | | catch type: redo st: 0001 ed: 0023 sp: 0000 cont: 0001 | | catch type: next st: 0001 ed: 0023 sp: 0000 cont: 0023 | |------------------------------------------------------------------------ | 0000 nop ( 1)[Bc] | 0001 putself ( 7)[Li] | 0002 putobject_INT2FIX_1_ | 0003 opt_send_without_block <calldata!mid:puts, argc:1, FCALL|ARGS_SIMPLE> | 0005 pop | 0006 putnil ( 8)[Li] | 0007 throw 2 | 0009 pop | 0010 jump 1 ( 3) | 0012 putnil | 0013 nop | 0014 putself ( 7)[Li] | 0015 putobject_INT2FIX_1_ | 0016 opt_send_without_block <calldata!mid:puts, argc:1, FCALL|ARGS_SIMPLE> | 0018 pop | 0019 putnil ( 8)[Li] | 0020 throw 2 | 0022 pop | 0023 leave ( 9)[Br] |------------------------------------------------------------------------ 0000 putobject_INT2FIX_1_ ( 1)[Li] 0001 send <calldata!mid:times, argc:0>, block in <main> 0004 leave ``` `catch type: ensure st: 0001 ed: 0013 sp: 0001 cont: 0014` catches `throw 2` but I guess it is unexpected. For example, if I make a change like this, it seems like it would work in this case, but I can't tell if there are any side effects. (To be honest, Gemini AI wrote this code for me, so I don't really understand what it means.) ```diff diff --git a/compile.c b/compile.c index 2ff207928a5..d92770e9b63 100644 --- a/compile.c +++ b/compile.c @@ -6675,7 +6675,13 @@ add_ensure_iseq(LINK_ANCHOR *const ret, rb_iseq_t *iseq, int is_return) LABEL *lend = NEW_LABEL(0); INIT_ANCHOR(ensure_part); - add_ensure_range(iseq, enlp->erange, lstart, lend); + struct iseq_compile_data_ensure_node_stack *e = prev_enlp; + while (e) { + if (e->erange != NULL) { + add_ensure_range(iseq, e->erange, lstart, lend); + } + e = e->prev; + } ISEQ_COMPILE_DATA(iseq)->ensure_node_stack = enlp->prev; ADD_LABEL(ensure_part, lstart); diff --git a/prism_compile.c b/prism_compile.c index 69e7d535491..9402395ace5 100644 --- a/prism_compile.c +++ b/prism_compile.c @@ -4751,7 +4751,13 @@ pm_add_ensure_iseq(LINK_ANCHOR *const ret, rb_iseq_t *iseq, int is_return, pm_sc LABEL *lstart = NEW_LABEL(0); LABEL *lend = NEW_LABEL(0); - add_ensure_range(iseq, enlp->erange, lstart, lend); + struct iseq_compile_data_ensure_node_stack *e = prev_enlp; + while (e) { + if (e->erange != NULL) { + add_ensure_range(iseq, e->erange, lstart, lend); + } + e = e->prev; + } ISEQ_COMPILE_DATA(iseq)->ensure_node_stack = enlp->prev; PUSH_LABEL(ensure_part, lstart); ``` ``` $ make -j miniruby 2>&1 >/dev/null && ./miniruby a.rb && ./miniruby -v --dump=insn a.rb 1 ruby 4.1.0dev (2026-08-30T06:52:45Z :detached: a889da909c) +PRISM [x86_64-linux] == disasm: #<ISeq:<main>@a.rb:1 (1,0)-(9,3)> == catch table | catch type: break st: 0000 ed: 0004 sp: 0000 cont: 0004 | == disasm: #<ISeq:block in <main>@a.rb:1 (1,8)-(9,3)> | == catch table | | catch type: ensure st: 0001 ed: 0001 sp: 0001 cont: 0014 | | == disasm: #<ISeq:ensure in block in <main>@a.rb:4 (4,2)-(5,5)> | | local table (size: 1, argc: 0 [opts: 0, rest: -1, post: 0, block: -1, kw: -1@-1, kwrest: -1]) | | [ 1] "$!"@0 | | 0000 getlocal_WC_0 "$!"@0 ( 4) | | 0002 throw 0 | | catch type: ensure st: 0001 ed: 0001 sp: 0001 cont: 0014 | | catch type: ensure st: 0010 ed: 0013 sp: 0001 cont: 0014 | | catch type: ensure st: 0001 ed: 0001 sp: 0001 cont: 0023 | | == disasm: #<ISeq:ensure in block in <main>@a.rb:7 (7,2)-(8,7)> | | local table (size: 1, argc: 0 [opts: 0, rest: -1, post: 0, block: -1, kw: -1@-1, kwrest: -1]) | | [ 1] "$!"@0 | | 0000 putself ( 7)[Li] | | 0001 putobject_INT2FIX_1_ | | 0002 opt_send_without_block <calldata!mid:puts, argc:1, FCALL|ARGS_SIMPLE> | | 0004 pop | | 0005 putnil ( 8)[Li] | | 0006 throw 2 | | 0008 pop | | 0009 getlocal_WC_0 "$!"@0 ( 6) | | 0011 throw 0 | | catch type: ensure st: 0001 ed: 0001 sp: 0001 cont: 0023 | | catch type: ensure st: 0010 ed: 0014 sp: 0001 cont: 0023 | | catch type: redo st: 0001 ed: 0023 sp: 0000 cont: 0001 | | catch type: next st: 0001 ed: 0023 sp: 0000 cont: 0023 | |------------------------------------------------------------------------ | 0000 nop ( 1)[Bc] | 0001 putself ( 7)[Li] | 0002 putobject_INT2FIX_1_ | 0003 opt_send_without_block <calldata!mid:puts, argc:1, FCALL|ARGS_SIMPLE> | 0005 pop | 0006 putnil ( 8)[Li] | 0007 throw 2 | 0009 pop | 0010 jump 1 ( 3) | 0012 putnil | 0013 nop | 0014 putself ( 7)[Li] | 0015 putobject_INT2FIX_1_ | 0016 opt_send_without_block <calldata!mid:puts, argc:1, FCALL|ARGS_SIMPLE> | 0018 pop | 0019 putnil ( 8)[Li] | 0020 throw 2 | 0022 pop | 0023 leave ( 9)[Br] |------------------------------------------------------------------------ 0000 putobject_INT2FIX_1_ ( 1)[Li] 0001 send <calldata!mid:times, argc:0>, block in <main> 0004 leave ``` ---------------------------------------- Bug #20917: redo/next in nested begin block causes wrong order of execution https://bugs.ruby-lang.org/issues/20917#change-118732 * Author: hoshiumiarata (Arata Hoshiumi) * Status: Open * ruby -v: ruby 3.3.5 (2024-09-03 revision ef084cc8f4) [x86_64-darwin24] * Backport: 3.1: UNKNOWN, 3.2: UNKNOWN, 3.3: UNKNOWN ---------------------------------------- It seems that `redo`/`next` in a nested `begin` block can cause the wrong order of execution. For example: ```ruby for _ in [0] puts 0 begin puts 1 begin puts 2 redo ensure puts 3 end ensure puts 4 break end end ``` It prints: ``` 0 1 2 3 4 3 4 => nil ``` But I think it should print: ``` 0 1 2 3 4 => nil ``` Because execution order should be: 1. `puts 0` 2. `puts 1` 3. `puts 2` 4. `redo` 5. unwind to nested `ensure` block 6. `puts 3` 7. unwind to outer `ensure` block 8. `puts 4` 9. `break` 10. end of loop Interestingly enough, if we add an empty `rescue` block before any of the `ensure` blocks, then the execution order is correct. -- https://bugs.ruby-lang.org/
participants (4)
-
Eregon (Benoit Daloze) -
hoshiumiarata (Arata Hoshiumi)
-
kddnewton (Kevin Newton)
-
wanabe (_ wanabe)