Skip to content

Fix redo keyword - #6440

Merged
matz merged 3 commits into
mruby:masterfrom
dearblue:redo
Dec 6, 2024
Merged

Fix redo keyword#6440
matz merged 3 commits into
mruby:masterfrom
dearblue:redo

Conversation

@dearblue

@dearblue dearblue commented Dec 1, 2024

Copy link
Copy Markdown
Contributor

The main purpose is to fix #6439, but includes additional testing and a fix to allow redo from nested loopinfo.

The additional test code should be correct as it was also confirmed by “ruby 3.4.0preview2 (2024-10-07 master 32c733f57b) +PRISM [amd64-freebsd14]”.

Add `OP_NOP` to distinguish `retry` and jump targets while maintaining instruction compatibility.
Ideally, it might be preferable to separate them into `OP_REDO`.

fixed mruby#6439
@dearblue

dearblue commented Dec 1, 2024

Copy link
Copy Markdown
Contributor Author

I'll leave here a patch to include the OP_REDO instruction based on this PR.
This patch also tests successfully.

Details
diff --git a/include/mruby/ops.h b/include/mruby/ops.h
index cda796f08..56e430a33 100644
--- a/include/mruby/ops.h
+++ b/include/mruby/ops.h
@@ -54,6 +54,7 @@ OPCODE(JMPIF,      BS)       /* if R[a] pc+=b */
 OPCODE(JMPNOT,     BS)       /* if !R[a] pc+=b */
 OPCODE(JMPNIL,     BS)       /* if R[a]==nil pc+=b */
 OPCODE(JMPUW,      S)        /* unwind_and_jump_to(a) */
+OPCODE(REDO,       S)        /* pc+=b (with ensure handler) */
 OPCODE(EXCEPT,     B)        /* R[a] = exc */
 OPCODE(RESCUE,     BB)       /* R[b] = R[a].isa?(R[b]) */
 OPCODE(RAISEIF,    B)        /* raise(R[a]) if R[a] */
diff --git a/mrbgems/mruby-compiler/core/codegen.c b/mrbgems/mruby-compiler/core/codegen.c
index 364ce2048..05e1c9260 100644
--- a/mrbgems/mruby-compiler/core/codegen.c
+++ b/mrbgems/mruby-compiler/core/codegen.c
@@ -1323,7 +1323,6 @@ for_body(codegen_scope *s, node *tree)
   /* construct loop */
   lp = loop_push(s, LOOP_FOR);
   lp->pc1 = new_label(s);
-  genop_0(s, OP_NOP); /* for redo */
 
   /* loop body */
   codegen(s, tree->cdr->cdr->car, VAL);
@@ -2590,7 +2589,6 @@ codegen(codegen_scope *s, node *tree, int val)
         pos = genjmp2_0(s, OP_JMPIF, cursp(), NOVAL);
       }
       lp->pc1 = new_label(s);
-      genop_0(s, OP_NOP); /* for redo */
       codegen(s, tree->cdr, NOVAL);
       genjmp(s, OP_JMP, lp->pc0);
       dispatch(s, pos);
@@ -3190,7 +3188,7 @@ codegen(codegen_scope *s, node *tree, int val)
         raise_error(s, "unexpected redo");
       }
       if (lp->type != LOOP_BEGIN && lp->type != LOOP_RESCUE) {
-        genjmp(s, OP_JMPUW, lp->pc1);
+        genjmp(s, OP_REDO, lp->pc1);
         break;
       }
     }
diff --git a/src/codedump.c b/src/codedump.c
index 556e4f62d..f9e6b08c4 100644
--- a/src/codedump.c
+++ b/src/codedump.c
@@ -308,6 +308,10 @@ codedump(mrb_state *mrb, const mrb_irep *irep, FILE *out)
       i = pc - irep->iseq;
       fprintf(out, "JMPUW\t\t%03d\n", (int)i+(int16_t)a);
       break;
+    CASE(OP_REDO, S):
+      i = pc - irep->iseq;
+      fprintf(out, "REDO\t\t%03d\n", (int)i+(int16_t)a);
+      break;
     CASE(OP_JMPIF, BS):
       i = pc - irep->iseq;
       fprintf(out, "JMPIF\t\tR%d\t%03d\t", a, (int)i+(int16_t)b);
diff --git a/src/vm.c b/src/vm.c
index 4d78035ad..54db0c8e8 100644
--- a/src/vm.c
+++ b/src/vm.c
@@ -1124,7 +1124,8 @@ mrb_yield_cont(mrb_state *mrb, mrb_value b, mrb_value self, mrb_int argc, const
 #define RBREAK_TAG_FOREACH(f) \
   f(RBREAK_TAG_BREAK, 0) \
   f(RBREAK_TAG_JUMP, 1) \
-  f(RBREAK_TAG_STOP, 2)
+  f(RBREAK_TAG_REDO, 2) \
+  f(RBREAK_TAG_STOP, 3)
 
 #define RBREAK_TAG_DEFINE(tag, i) tag = i,
 enum {
@@ -1733,6 +1734,29 @@ RETRY_TRY_BLOCK:
       JUMP;
     }
 
+    // TODO: OP_JMPUW との共通処理が出来るかもしれない
+    CASE(OP_REDO, S) {
+      a = (uint32_t)((ci->pc - irep->iseq) + (int16_t)a);
+      CHECKPOINT_RESTORE(RBREAK_TAG_REDO) {
+        struct RBreak *brk = (struct RBreak*)mrb->exc;
+        mrb_value target = mrb_break_value_get(brk);
+        mrb_assert(mrb_integer_p(target));
+        a = (uint32_t)mrb_integer(target);
+        mrb_assert(a >= 0 && a < irep->ilen);
+      }
+      CHECKPOINT_MAIN(RBREAK_TAG_REDO) {
+        if (irep->clen > 0 &&
+            (ch = catch_handler_find(irep, ci->pc, MRB_CATCH_FILTER_ENSURE))) {
+          THROW_TAGGED_BREAK(mrb, RBREAK_TAG_REDO, mrb->c->ci, mrb_fixnum_value(a));
+        }
+      }
+      CHECKPOINT_END(RBREAK_TAG_REDO);
+
+      mrb->exc = NULL; /* clear break object */
+      ci->pc = irep->iseq + a;
+      JUMP;
+    }
+
     CASE(OP_EXCEPT, B) {
       mrb_value exc;
 

@matz
matz merged commit ac64365 into mruby:master Dec 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

OP_JMPUW does not call the ensure block when it jumps to the beginning of the begin block

2 participants