diff options
author | Adam Joseph <adam@westernsemico.com> | 2023-12-12T05·17-0800 |
---|---|---|
committer | clbot <clbot@tvl.fyi> | 2023-12-12T14·55+0000 |
commit | 9792920f8cdec92aa2c650de8cfd0a85fa7dce52 (patch) | |
tree | baa5eb473824dc67af8965ef7b7cd9d69f6231a3 | |
parent | e54eeda0ff4b777899491e2a753481e9c73af7df (diff) |
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>
-rw-r--r-- | tvix/eval/src/compiler/mod.rs | 6 | ||||
-rw-r--r-- | tvix/eval/src/opcode.rs | 6 | ||||
-rw-r--r-- | tvix/eval/src/tests/tvix_tests/eval-okay-test-catchables-in-default-args.exp (renamed from tvix/eval/src/tests/tvix_tests/notyetpassing/eval-okay-test-catchables-in-default-args.exp) | 0 | ||||
-rw-r--r-- | tvix/eval/src/tests/tvix_tests/eval-okay-test-catchables-in-default-args.nix (renamed from tvix/eval/src/tests/tvix_tests/notyetpassing/eval-okay-test-catchables-in-default-args.nix) | 0 | ||||
-rw-r--r-- | tvix/eval/src/value/mod.rs | 1 | ||||
-rw-r--r-- | tvix/eval/src/vm/mod.rs | 7 |
6 files changed, 20 insertions, 0 deletions
diff --git a/tvix/eval/src/compiler/mod.rs b/tvix/eval/src/compiler/mod.rs index 01e07304bc3f..4edb5f204533 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 762cff7b5282..467798177550 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 c508d5366f70..c508d5366f70 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 0523cf864c3b..0523cf864c3b 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 003567bfd440..300824b88ff7 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 6128e3a601a0..642b6317caee 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) { |