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

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>
This commit is contained in:
Adam Joseph 2023-12-11 21:17:22 -08:00 committed by clbot
parent e54eeda0ff
commit 9792920f8c
6 changed files with 20 additions and 0 deletions

View file

@ -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;

View file

@ -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

View file

@ -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.
///

View file

@ -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) {