Fix destructuring alternation
Attempting to use the existing FORK_OPT opcode resulted in difficulty
knowing when to pop an error message off the stack and when not to. This
commit makes DESTRUCTURE_ALT a real opcode that is identical to
FORK_OPT, except for never pushing the error message onto the stack when
continuing from an error backtrack.
Some small changes were necessary to the DUP/POP behavior surrounding
destructuring to accomodate this.
diff --git a/src/compile.c b/src/compile.c
index 33a8e72..bd0cb6d 100644
--- a/src/compile.c
+++ b/src/compile.c
@@ -759,7 +759,7 @@
/* Build wrappers around destructuring matchers so that we can chain them
* when we have errors. The approach is as follows:
- * FORK_OPT NEXT_MATCHER (unless last matcher)
+ * DESTRUCTURE_ALT NEXT_MATCHER (unless last matcher)
* existing_matcher_block
* JUMP BODY
*/
@@ -767,20 +767,18 @@
block preamble = {0};
block altmatchers = {0};
block mb = {0};
+ block final_matcher = matchers;
// Pass through the matchers to find all destructured names.
- while (matchers.first && matchers.first->op == DESTRUCTURE_ALT) {
- block_append(&altmatchers, inst_block(block_take(&matchers)));
+ while (final_matcher.first && final_matcher.first->op == DESTRUCTURE_ALT) {
+ block_append(&altmatchers, inst_block(block_take(&final_matcher)));
}
// We don't have any alternations here, so we can use the simplest case.
if (altmatchers.first == NULL) {
- return bind_matcher(matchers, body);
+ return bind_matcher(final_matcher, body);
}
- // The final matcher needs to strip the error from the previous FORK_OPT
- block final_matcher = BLOCK(gen_op_simple(POP), gen_op_simple(DUP), matchers);
-
// Collect var names
jv all_vars = jv_object();
block_get_unbound_vars(altmatchers, &all_vars);
@@ -800,16 +798,11 @@
for (inst *i = altmatchers.first; i; i = i->next) {
block submatcher = i->subfn;
- // Get rid of the error from the previous matcher
- if (mb.first != NULL) {
- submatcher = BLOCK(gen_op_simple(POP), gen_op_simple(DUP), submatcher);
- }
-
// If we're successful, jump to the end of the matchers
submatcher = BLOCK(submatcher, gen_op_target(JUMP, final_matcher));
- // FORK_OPT to the end of this submatcher so we can skip to the next one on error
- mb = BLOCK(mb, gen_op_target(FORK_OPT, submatcher), submatcher);
+ // DESTRUCTURE_ALT to the end of this submatcher so we can skip to the next one on error
+ mb = BLOCK(mb, gen_op_target(DESTRUCTURE_ALT, submatcher), submatcher);
// We're done with this inst and we don't want it anymore
// But we can't let it free the submatcher block.
@@ -1003,12 +996,13 @@
if (body.first && body.first->op == TOP)
top = inst_block(block_take(&body));
- if (matchers.first && matchers.first->op == DESTRUCTURE_ALT && !block_is_noop(var)) {
+ if (matchers.first && matchers.first->op == DESTRUCTURE_ALT) {
block_append(&var, gen_op_simple(DUP));
- block_append(&matchers, gen_op_simple(POP));
+ } else {
+ top = BLOCK(top, gen_op_simple(DUP));
}
- return BLOCK(top, gen_op_simple(DUP), gen_subexp(var), gen_op_simple(POP), bind_alternation_matchers(matchers, body));
+ return BLOCK(top, gen_subexp(var), gen_op_simple(POP), bind_alternation_matchers(matchers, body));
}
// Like gen_var_binding(), but bind `break`'s wildcard unbound variable
diff --git a/src/execute.c b/src/execute.c
index d567216..8eb41cc 100644
--- a/src/execute.c
+++ b/src/execute.c
@@ -799,21 +799,27 @@
}
case FORK_OPT:
+ case DESTRUCTURE_ALT:
case FORK: {
stack_save(jq, pc - 1, stack_get_pos(jq));
pc++; // skip offset this time
break;
}
- case ON_BACKTRACK(FORK_OPT): {
+ case ON_BACKTRACK(FORK_OPT):
+ case ON_BACKTRACK(DESTRUCTURE_ALT): {
if (jv_is_valid(jq->error)) {
// `try EXP ...` backtracked here (no value, `empty`), so we backtrack more
jv_free(stack_pop(jq));
goto do_backtrack;
}
// `try EXP ...` exception caught in EXP
- jv_free(stack_pop(jq)); // free the input
- stack_push(jq, jv_invalid_get_msg(jq->error)); // push the error's message
+ // DESTRUCTURE_ALT doesn't want the error message on the stack,
+ // as we would just want to throw it away anyway.
+ if (opcode != ON_BACKTRACK(DESTRUCTURE_ALT)) {
+ jv_free(stack_pop(jq)); // free the input
+ stack_push(jq, jv_invalid_get_msg(jq->error)); // push the error's message
+ }
jq->error = jv_null();
uint16_t offset = *pc++;
pc += offset;
diff --git a/src/opcode_list.h b/src/opcode_list.h
index ffb27d2..886131d 100644
--- a/src/opcode_list.h
+++ b/src/opcode_list.h
@@ -44,5 +44,5 @@
OP(MODULEMETA, CONSTANT, 0, 0)
OP(GENLABEL, NONE, 0, 1)
-OP(DESTRUCTURE_ALT, NONE, 0, 0)
+OP(DESTRUCTURE_ALT, BRANCH, 0, 0)
OP(STOREVN, VARIABLE, 1, 0)
diff --git a/tests/jq.test b/tests/jq.test
index 8771ba6..7e2dd43 100644
--- a/tests/jq.test
+++ b/tests/jq.test
@@ -726,6 +726,107 @@
[4, 5, null, null, 7, null]
[null, null, null, null, null, "foo"]
+# Destructuring DUP/POP issues
+.[] | . as {a:$a} ?// {a:$a} ?// {a:$a} | $a
+[[3],[4],[5],6]
+# Runtime error: "jq: Cannot index array with string \"c\""
+
+.[] as {a:$a} ?// {a:$a} ?// {a:$a} | $a
+[[3],[4],[5],6]
+# Runtime error: "jq: Cannot index array with string \"c\""
+
+[[3],[4],[5],6][] | . as {a:$a} ?// {a:$a} ?// {a:$a} | $a
+null
+# Runtime error: "jq: Cannot index array with string \"c\""
+
+[[3],[4],[5],6] | .[] as {a:$a} ?// {a:$a} ?// {a:$a} | $a
+null
+# Runtime error: "jq: Cannot index array with string \"c\""
+
+.[] | . as {a:$a} ?// {a:$a} ?// $a | $a
+[[3],[4],[5],6]
+[3]
+[4]
+[5]
+6
+
+.[] as {a:$a} ?// {a:$a} ?// $a | $a
+[[3],[4],[5],6]
+[3]
+[4]
+[5]
+6
+
+[[3],[4],[5],6][] | . as {a:$a} ?// {a:$a} ?// $a | $a
+null
+[3]
+[4]
+[5]
+6
+
+[[3],[4],[5],6] | .[] as {a:$a} ?// {a:$a} ?// $a | $a
+null
+[3]
+[4]
+[5]
+6
+
+.[] | . as {a:$a} ?// $a ?// {a:$a} | $a
+[[3],[4],[5],6]
+[3]
+[4]
+[5]
+6
+
+.[] as {a:$a} ?// $a ?// {a:$a} | $a
+[[3],[4],[5],6]
+[3]
+[4]
+[5]
+6
+
+[[3],[4],[5],6][] | . as {a:$a} ?// $a ?// {a:$a} | $a
+null
+[3]
+[4]
+[5]
+6
+
+[[3],[4],[5],6] | .[] as {a:$a} ?// $a ?// {a:$a} | $a
+null
+[3]
+[4]
+[5]
+6
+
+.[] | . as $a ?// {a:$a} ?// {a:$a} | $a
+[[3],[4],[5],6]
+[3]
+[4]
+[5]
+6
+
+.[] as $a ?// {a:$a} ?// {a:$a} | $a
+[[3],[4],[5],6]
+[3]
+[4]
+[5]
+6
+
+[[3],[4],[5],6][] | . as $a ?// {a:$a} ?// {a:$a} | $a
+null
+[3]
+[4]
+[5]
+6
+
+[[3],[4],[5],6] | .[] as $a ?// {a:$a} ?// {a:$a} | $a
+null
+[3]
+[4]
+[5]
+6
+
. as $dot|any($dot[];not)
[1,2,3,4,true,false,1,2,3,4,5]
true
