refactor(tvix/eval): Compile OpAssert using conditional jumps
In order to behave nicely with tryEval, asserts need to leave the instruction pointer in a reasonable place even if they fail - whereas with the previous implementation catching a failed assert would still end up running the op for the *body* of the assert. With this change, we compile asserts much more like an `if` expression with conditional jumps rather than having an OpAssert op. Change-Id: I1b266c3be90185c84000da6b1995ac3e6fd5471b Reviewed-on: https://cl.tvl.fyi/c/depot/+/6925 Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
This commit is contained in:
parent
1ab252470a
commit
899fbdbddb
3 changed files with 26 additions and 10 deletions
|
@ -630,16 +630,34 @@ impl Compiler<'_> {
|
||||||
self.patch_jump(final_jump);
|
self.patch_jump(final_jump);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Compile `assert` expressions using jumping instructions in the VM.
|
||||||
|
///
|
||||||
|
/// ```notrust
|
||||||
|
/// ┌─────────────────────┐
|
||||||
|
/// │ 0 [ conditional ] │
|
||||||
|
/// │ 1 JUMP_IF_FALSE →┼─┐
|
||||||
|
/// │ 2 [ main body ] │ │ Jump to else body if
|
||||||
|
/// ┌┼─3─← JUMP │ │ condition is false.
|
||||||
|
/// Jump over else body ││ 4 OP_ASSERT_FAIL ←┼─┘
|
||||||
|
/// if condition is true.└┼─5─→ ... │
|
||||||
|
/// └─────────────────────┘
|
||||||
|
/// ```
|
||||||
fn compile_assert(&mut self, slot: LocalIdx, node: &ast::Assert) {
|
fn compile_assert(&mut self, slot: LocalIdx, node: &ast::Assert) {
|
||||||
// Compile the assertion condition to leave its value on the stack.
|
// Compile the assertion condition to leave its value on the stack.
|
||||||
self.compile(slot, &node.condition().unwrap());
|
self.compile(slot, &node.condition().unwrap());
|
||||||
self.emit_force(&node.condition().unwrap());
|
self.emit_force(&node.condition().unwrap());
|
||||||
self.push_op(OpCode::OpAssert, &node.condition().unwrap());
|
let then_idx = self.push_op(OpCode::OpJumpIfFalse(JumpOffset(0)), node);
|
||||||
|
|
||||||
// The runtime will abort evaluation at this point if the
|
self.push_op(OpCode::OpPop, node);
|
||||||
// assertion failed, if not the body simply continues on like
|
|
||||||
// normal.
|
|
||||||
self.compile(slot, &node.body().unwrap());
|
self.compile(slot, &node.body().unwrap());
|
||||||
|
|
||||||
|
let else_idx = self.push_op(OpCode::OpJump(JumpOffset(0)), node);
|
||||||
|
|
||||||
|
self.patch_jump(then_idx);
|
||||||
|
self.push_op(OpCode::OpPop, node);
|
||||||
|
self.push_op(OpCode::OpAssertFail, &node.condition().unwrap());
|
||||||
|
|
||||||
|
self.patch_jump(else_idx);
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Compile conditional expressions using jumping instructions in the VM.
|
/// Compile conditional expressions using jumping instructions in the VM.
|
||||||
|
|
|
@ -121,8 +121,8 @@ pub enum OpCode {
|
||||||
/// Close scopes while leaving their expression value around.
|
/// Close scopes while leaving their expression value around.
|
||||||
OpCloseScope(Count), // number of locals to pop
|
OpCloseScope(Count), // number of locals to pop
|
||||||
|
|
||||||
/// Asserts stack top is a boolean, and true.
|
/// Return an error indicating that an `assert` failed
|
||||||
OpAssert,
|
OpAssertFail,
|
||||||
|
|
||||||
// Lambdas & closures
|
// Lambdas & closures
|
||||||
OpCall,
|
OpCall,
|
||||||
|
|
|
@ -561,11 +561,9 @@ impl<'o> VM<'o> {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
OpCode::OpAssert => {
|
OpCode::OpAssertFail => {
|
||||||
if !fallible!(self, self.pop().as_bool()) {
|
|
||||||
return Err(self.error(ErrorKind::AssertionFailed));
|
return Err(self.error(ErrorKind::AssertionFailed));
|
||||||
}
|
}
|
||||||
}
|
|
||||||
|
|
||||||
OpCode::OpCall => {
|
OpCode::OpCall => {
|
||||||
let callable = self.pop();
|
let callable = self.pop();
|
||||||
|
|
Loading…
Reference in a new issue