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.

src/compile.c

@@ -759,7 +759,7 @@ static void block_get_unbound_vars(block b, jv *vars) {
 
 /* 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 @@ static block bind_alternation_matchers(block matchers, block body) {
   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 @@ static block bind_alternation_matchers(block matchers, block body) {
   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 @@ block gen_destructure(block var, block matchers, block body) {
   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