fix: foreach should not break init backtracking with DUPN

Simplifies the compilation of `foreach` to avoid the problem in jqlang#3227 by removing unnecessary `FORK ... BACKTRACK`. Does not address `reduce`, which is less trivial.
kanwren · Feb 17, 2025 · def2639 · def2639
1 parent 6d8e9f9
commit def2639
Show file tree

Hide file tree

Showing 2 changed files with 27 additions and 39 deletions.
diff --git a/src/compile.c b/src/compile.c
@@ -845,46 +845,26 @@ block gen_reduce(block source, block matcher, block init, block body) {
 }
 
 block gen_foreach(block source, block matcher, block init, block update, block extract) {
-  block output = gen_op_targetlater(JUMP);
   block state_var = gen_op_var_fresh(STOREV, "foreach");
-  block loop = BLOCK(gen_op_simple(DUPN),
-                     // get a value from the source expression:
-                     source,
-                     // destructure the value into variable(s) for all the code
-                     // in the body to see
-                     bind_alternation_matchers(matcher,
-                                  // load the loop state variable
-                                  BLOCK(gen_op_bound(LOADV, state_var),
-                                        // generate updated state
-                                        update,
-                                        // save the updated state for value extraction
-                                        gen_op_simple(DUP),
-                                        // save new state
-                                        gen_op_bound(STOREV, state_var),
-                                        // extract an output...
-                                        extract,
-                                        // ...and output it by jumping
-                                        // past the BACKTRACK that comes
-                                        // right after the loop body,
-                                        // which in turn is there
-                                        // because...
-                                        //
-                                        // (Incidentally, extract can also
-                                        // backtrack, e.g., if it calls
-                                        // empty, in which case we don't
-                                        // get here.)
-                                        output)));
-  block foreach = BLOCK(gen_op_simple(DUP),
-                        init,
-                        state_var,
-                        gen_op_target(FORK, loop),
-                        loop,
-                        // ...at this point `foreach`'s original input
-                        // will be on top of the stack, and we don't
-                        // want to output it, so we backtrack.
-                        gen_op_simple(BACKTRACK));
-  inst_set_target(output, foreach); // make that JUMP go bast the BACKTRACK at the end of the loop
-  return foreach;
+  return BLOCK(gen_op_simple(DUP),
+               init,
+               state_var,
+               gen_op_simple(DUP),
+               // get a value from the source expression:
+               source,
+               // destructure the value into variable(s) for all the code
+               // in the body to see
+               bind_alternation_matchers(matcher,
+                            // load the loop state variable
+                            BLOCK(gen_op_bound(LOADV, state_var),
+                                  // generate updated state
+                                  update,
+                                  // save the updated state for value extraction
+                                  gen_op_simple(DUP),
+                                  // save new state
+                                  gen_op_bound(STOREV, state_var),
+                                  // extract an output...
+                                  extract)));
 }
 
 block gen_definedor(block a, block b) {

diff --git a/tests/jq.test b/tests/jq.test
@@ -2289,3 +2289,11 @@ try ltrimstr("x") catch "x", try rtrimstr("x") catch "x" | "ok"
 try ["OK", setpath([[1]]; 1)] catch ["KO", .]
 []
 ["KO","Cannot update field at array index of array"]
+
+# regression test for #3227
+foreach .[] as $x (0, 1; . + $x)
+[1, 2]
+1
+3
+2
+4