fix(tvix/eval): fix branching on catchable defaults (b/343) r/7178

This commit adds Opcode::OpJumpIfCatchable, which can be inserted
ahead of most VM operations which expect a boolean on the stack, in
order to handle catchables in branching position properly.

Other than remembering to patch the jump, no other changes should be
required.

This commit also fixes b/343 by emitting this new opcode when
compiling if-then-else.  There are probably other places where we
need to do the same thing.

Change-Id: I48de3010014c0bbeba15d34fc0d4800e0bb5a1ef
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10288
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
Autosubmit: Adam Joseph <adam@westernsemico.com>

6 files changed, 20 insertions, 0 deletions

diff --git a/tvix/eval/src/compiler/mod.rs b/tvix/eval/src/compiler/mod.rs
index 01e07304bc..4edb5f2045 100644
--- a/tvix/eval/src/compiler/mod.rs
+++ b/tvix/eval/src/compiler/mod.rs
@@ -864,6 +864,10 @@ impl Compiler<'_> {
         self.compile(slot, node.condition().unwrap());
         self.emit_force(&node.condition().unwrap());
 
+        let throw_idx = self.push_op(
+            OpCode::OpJumpIfCatchable(JumpOffset(0)),
+            &node.condition().unwrap(),
+        );
         let then_idx = self.push_op(
             OpCode::OpJumpIfFalse(JumpOffset(0)),
             &node.condition().unwrap(),
@@ -879,6 +883,7 @@ impl Compiler<'_> {
         self.compile(slot, node.else_body().unwrap());
 
         self.patch_jump(else_idx); // patch jump *over* else body
+        self.patch_jump(throw_idx); // patch jump *over* else body
     }
 
     /// Compile `with` expressions by emitting instructions that
@@ -1328,6 +1333,7 @@ impl Compiler<'_> {
             OpCode::OpJump(n)
             | OpCode::OpJumpIfFalse(n)
             | OpCode::OpJumpIfTrue(n)
+            | OpCode::OpJumpIfCatchable(n)
             | OpCode::OpJumpIfNotFound(n)
             | OpCode::OpJumpIfNoFinaliseRequest(n) => {
                 *n = offset;
diff --git a/tvix/eval/src/opcode.rs b/tvix/eval/src/opcode.rs
index 762cff7b52..4677981775 100644
--- a/tvix/eval/src/opcode.rs
+++ b/tvix/eval/src/opcode.rs
@@ -130,6 +130,12 @@ pub enum OpCode {
     /// of the stack is `false`.
     OpJumpIfFalse(JumpOffset),
 
+    /// Pop one stack item and jump forward in the bytecode
+    /// specified by the number of instructions in its usize
+    /// operand, *if* the value at the top of the stack is a
+    /// Value::Catchable.
+    OpJumpIfCatchable(JumpOffset),
+
     /// Jump forward in the bytecode specified by the number of
     /// instructions in its usize operand, *if* the value at the top
     /// of the stack is the internal value representing a missing
diff --git a/tvix/eval/src/tests/tvix_tests/notyetpassing/eval-okay-test-catchables-in-default-args.exp b/tvix/eval/src/tests/tvix_tests/eval-okay-test-catchables-in-default-args.exp
index c508d5366f..c508d5366f 100644
--- a/tvix/eval/src/tests/tvix_tests/notyetpassing/eval-okay-test-catchables-in-default-args.exp
+++ b/tvix/eval/src/tests/tvix_tests/eval-okay-test-catchables-in-default-args.exp
diff --git a/tvix/eval/src/tests/tvix_tests/notyetpassing/eval-okay-test-catchables-in-default-args.nix b/tvix/eval/src/tests/tvix_tests/eval-okay-test-catchables-in-default-args.nix
index 0523cf864c..0523cf864c 100644
--- a/tvix/eval/src/tests/tvix_tests/notyetpassing/eval-okay-test-catchables-in-default-args.nix
+++ b/tvix/eval/src/tests/tvix_tests/eval-okay-test-catchables-in-default-args.nix
diff --git a/tvix/eval/src/value/mod.rs b/tvix/eval/src/value/mod.rs
index 003567bfd4..300824b88f 100644
--- a/tvix/eval/src/value/mod.rs
+++ b/tvix/eval/src/value/mod.rs
@@ -639,6 +639,7 @@ impl Value {
     gen_is!(is_number, Value::Integer(_) | Value::Float(_));
     gen_is!(is_bool, Value::Bool(_));
     gen_is!(is_attrs, Value::Attrs(_));
+    gen_is!(is_catchable, Value::Catchable(_));
 
     /// Compare `self` against other using (fallible) Nix ordering semantics.
     ///
diff --git a/tvix/eval/src/vm/mod.rs b/tvix/eval/src/vm/mod.rs
index 6128e3a601..642b6317ca 100644
--- a/tvix/eval/src/vm/mod.rs
+++ b/tvix/eval/src/vm/mod.rs
@@ -562,6 +562,13 @@ impl<'o> VM<'o> {
                     }
                 }
 
+                OpCode::OpJumpIfCatchable(JumpOffset(offset)) => {
+                    debug_assert!(offset != 0);
+                    if self.stack_peek(0).is_catchable() {
+                        frame.ip += offset;
+                    }
+                }
+
                 OpCode::OpJumpIfNoFinaliseRequest(JumpOffset(offset)) => {
                     debug_assert!(offset != 0);
                     match self.stack_peek(0) {