From b9566da5c94c974eb3a25f66c6c86b90ccd02ca1 Mon Sep 17 00:00:00 2001 From: Vincent Ambo Date: Thu, 1 Sep 2022 16:23:48 +0300 Subject: [PATCH] refactor(tvix/eval): add and use Compiler::push_op method This is currently just a wrapper around Chunk::push_op, but will gain the span resolution logic in a moment. Change-Id: I862bf9ecff0932f8da6708401ea044b9442c5d5b Reviewed-on: https://cl.tvl.fyi/c/depot/+/6377 Tested-by: BuildkiteCI Reviewed-by: sterni --- tvix/eval/src/compiler/mod.rs | 125 ++++++++++++++++++---------------- 1 file changed, 65 insertions(+), 60 deletions(-) diff --git a/tvix/eval/src/compiler/mod.rs b/tvix/eval/src/compiler/mod.rs index b1dec5df1..c9bbd8171 100644 --- a/tvix/eval/src/compiler/mod.rs +++ b/tvix/eval/src/compiler/mod.rs @@ -98,9 +98,14 @@ impl Compiler { &mut self.context_mut().scope } + /// Push a single instruction to the current bytecode chunk. + fn push_op(&mut self, data: OpCode) -> CodeIdx { + self.chunk().push_op(data) + } + fn emit_constant(&mut self, value: Value) { let idx = self.chunk().push_constant(value); - self.chunk().push_op(OpCode::OpConstant(idx)); + self.push_op(OpCode::OpConstant(idx)); } } @@ -211,7 +216,7 @@ impl Compiler { } if count != 1 { - self.chunk().push_op(OpCode::OpInterpolate(Count(count))); + self.push_op(OpCode::OpInterpolate(Count(count))); } } @@ -224,7 +229,7 @@ impl Compiler { ast::UnaryOpKind::Negate => OpCode::OpNegate, }; - self.chunk().push_op(opcode); + self.push_op(opcode); } fn compile_binop(&mut self, slot: Option, op: ast::BinOp) { @@ -252,21 +257,21 @@ impl Compiler { self.emit_force(); match op.operator().unwrap() { - BinOpKind::Add => self.chunk().push_op(OpCode::OpAdd), - BinOpKind::Sub => self.chunk().push_op(OpCode::OpSub), - BinOpKind::Mul => self.chunk().push_op(OpCode::OpMul), - BinOpKind::Div => self.chunk().push_op(OpCode::OpDiv), - BinOpKind::Update => self.chunk().push_op(OpCode::OpAttrsUpdate), - BinOpKind::Equal => self.chunk().push_op(OpCode::OpEqual), - BinOpKind::Less => self.chunk().push_op(OpCode::OpLess), - BinOpKind::LessOrEq => self.chunk().push_op(OpCode::OpLessOrEq), - BinOpKind::More => self.chunk().push_op(OpCode::OpMore), - BinOpKind::MoreOrEq => self.chunk().push_op(OpCode::OpMoreOrEq), - BinOpKind::Concat => self.chunk().push_op(OpCode::OpConcat), + BinOpKind::Add => self.push_op(OpCode::OpAdd), + BinOpKind::Sub => self.push_op(OpCode::OpSub), + BinOpKind::Mul => self.push_op(OpCode::OpMul), + BinOpKind::Div => self.push_op(OpCode::OpDiv), + BinOpKind::Update => self.push_op(OpCode::OpAttrsUpdate), + BinOpKind::Equal => self.push_op(OpCode::OpEqual), + BinOpKind::Less => self.push_op(OpCode::OpLess), + BinOpKind::LessOrEq => self.push_op(OpCode::OpLessOrEq), + BinOpKind::More => self.push_op(OpCode::OpMore), + BinOpKind::MoreOrEq => self.push_op(OpCode::OpMoreOrEq), + BinOpKind::Concat => self.push_op(OpCode::OpConcat), BinOpKind::NotEqual => { - self.chunk().push_op(OpCode::OpEqual); - self.chunk().push_op(OpCode::OpInvert) + self.push_op(OpCode::OpEqual); + self.push_op(OpCode::OpInvert) } // Handled by separate branch above. @@ -289,17 +294,17 @@ impl Compiler { // If this value is false, jump over the right-hand side - the // whole expression is false. - let end_idx = self.chunk().push_op(OpCode::OpJumpIfFalse(JumpOffset(0))); + let end_idx = self.push_op(OpCode::OpJumpIfFalse(JumpOffset(0))); // Otherwise, remove the previous value and leave the // right-hand side on the stack. Its result is now the value // of the whole expression. - self.chunk().push_op(OpCode::OpPop); + self.push_op(OpCode::OpPop); self.compile(slot, node.rhs().unwrap()); self.emit_force(); self.patch_jump(end_idx); - self.chunk().push_op(OpCode::OpAssertBool); + self.push_op(OpCode::OpAssertBool); } fn compile_or(&mut self, slot: Option, node: ast::BinOp) { @@ -315,13 +320,13 @@ impl Compiler { // Opposite of above: If this value is **true**, we can // short-circuit the right-hand side. - let end_idx = self.chunk().push_op(OpCode::OpJumpIfTrue(JumpOffset(0))); - self.chunk().push_op(OpCode::OpPop); + let end_idx = self.push_op(OpCode::OpJumpIfTrue(JumpOffset(0))); + self.push_op(OpCode::OpPop); self.compile(slot, node.rhs().unwrap()); self.emit_force(); self.patch_jump(end_idx); - self.chunk().push_op(OpCode::OpAssertBool); + self.push_op(OpCode::OpAssertBool); } fn compile_implication(&mut self, slot: Option, node: ast::BinOp) { @@ -334,16 +339,16 @@ impl Compiler { // Leave left-hand side value on the stack and invert it. self.compile(slot, node.lhs().unwrap()); self.emit_force(); - self.chunk().push_op(OpCode::OpInvert); + self.push_op(OpCode::OpInvert); // Exactly as `||` (because `a -> b` = `!a || b`). - let end_idx = self.chunk().push_op(OpCode::OpJumpIfTrue(JumpOffset(0))); - self.chunk().push_op(OpCode::OpPop); + let end_idx = self.push_op(OpCode::OpJumpIfTrue(JumpOffset(0))); + self.push_op(OpCode::OpPop); self.compile(slot, node.rhs().unwrap()); self.emit_force(); self.patch_jump(end_idx); - self.chunk().push_op(OpCode::OpAssertBool); + self.push_op(OpCode::OpAssertBool); } fn compile_has_attr(&mut self, slot: Option, node: ast::HasAttr) { @@ -354,7 +359,7 @@ impl Compiler { // next nested element, for all fragments except the last one. for (count, fragment) in node.attrpath().unwrap().attrs().enumerate() { if count > 0 { - self.chunk().push_op(OpCode::OpAttrsTrySelect); + self.push_op(OpCode::OpAttrsTrySelect); } self.compile_attr(slot, fragment); @@ -362,7 +367,7 @@ impl Compiler { // After the last fragment, emit the actual instruction that // leaves a boolean on the stack. - self.chunk().push_op(OpCode::OpAttrsIsSet); + self.push_op(OpCode::OpAttrsIsSet); } fn compile_attr(&mut self, slot: Option, node: ast::Attr) { @@ -395,7 +400,7 @@ impl Compiler { self.compile(slot, item); } - self.chunk().push_op(OpCode::OpList(Count(count))); + self.push_op(OpCode::OpList(Count(count))); } // Compile attribute set literals into equivalent bytecode. @@ -437,7 +442,7 @@ impl Compiler { self.compile(slot, from.expr().unwrap()); self.emit_force(); self.emit_literal_ident(&ident); - self.chunk().push_op(OpCode::OpAttrsSelect); + self.push_op(OpCode::OpAttrsSelect); } } @@ -473,7 +478,7 @@ impl Compiler { // otherwise we need to emit an instruction to construct // the attribute path. if key_count > 1 { - self.chunk().push_op(OpCode::OpAttrPath(Count(key_count))); + self.push_op(OpCode::OpAttrPath(Count(key_count))); } // The value is just compiled as normal so that its @@ -482,7 +487,7 @@ impl Compiler { self.compile(slot, kv.value().unwrap()); } - self.chunk().push_op(OpCode::OpAttrs(Count(count))); + self.push_op(OpCode::OpAttrs(Count(count))); } fn compile_select(&mut self, slot: Option, node: ast::Select) { @@ -504,7 +509,7 @@ impl Compiler { // nested selects. for fragment in path.attrs() { self.compile_attr(slot, fragment); - self.chunk().push_op(OpCode::OpAttrsSelect); + self.push_op(OpCode::OpAttrsSelect); } } @@ -550,14 +555,14 @@ impl Compiler { for fragment in path.attrs() { self.compile_attr(slot, fragment); - self.chunk().push_op(OpCode::OpAttrsTrySelect); + self.push_op(OpCode::OpAttrsTrySelect); jumps.push( self.chunk() .push_op(OpCode::OpJumpIfNotFound(JumpOffset(0))), ); } - let final_jump = self.chunk().push_op(OpCode::OpJump(JumpOffset(0))); + let final_jump = self.push_op(OpCode::OpJump(JumpOffset(0))); for jump in jumps { self.patch_jump(jump); @@ -572,7 +577,7 @@ impl Compiler { fn compile_assert(&mut self, slot: Option, node: ast::Assert) { // Compile the assertion condition to leave its value on the stack. self.compile(slot, node.condition().unwrap()); - self.chunk().push_op(OpCode::OpAssert); + self.push_op(OpCode::OpAssert); // The runtime will abort evaluation at this point if the // assertion failed, if not the body simply continues on like @@ -593,15 +598,15 @@ impl Compiler { fn compile_if_else(&mut self, slot: Option, node: ast::IfElse) { self.compile(slot, node.condition().unwrap()); - let then_idx = self.chunk().push_op(OpCode::OpJumpIfFalse(JumpOffset(0))); + let then_idx = self.push_op(OpCode::OpJumpIfFalse(JumpOffset(0))); - self.chunk().push_op(OpCode::OpPop); // discard condition value + self.push_op(OpCode::OpPop); // discard condition value self.compile(slot, node.body().unwrap()); - let else_idx = self.chunk().push_op(OpCode::OpJump(JumpOffset(0))); + let else_idx = self.push_op(OpCode::OpJump(JumpOffset(0))); self.patch_jump(then_idx); // patch jump *to* else_body - self.chunk().push_op(OpCode::OpPop); // discard condition value + self.push_op(OpCode::OpPop); // discard condition value self.compile(slot, node.else_body().unwrap()); self.patch_jump(else_idx); // patch jump *over* else body @@ -652,7 +657,7 @@ impl Compiler { self.emit_force(); self.emit_literal_ident(&ident); - self.chunk().push_op(OpCode::OpAttrsSelect); + self.push_op(OpCode::OpAttrsSelect); let idx = self.declare_local( ident.syntax().clone(), ident.ident_token().unwrap().text(), @@ -712,7 +717,7 @@ impl Compiler { for idx in indices { if self.scope()[idx].needs_finaliser { let stack_idx = self.scope().stack_index(idx); - self.chunk().push_op(OpCode::OpFinalise(stack_idx)); + self.push_op(OpCode::OpFinalise(stack_idx)); } } @@ -737,7 +742,7 @@ impl Compiler { LocalPosition::Unknown => { // Are we possibly dealing with an upvalue? if let Some(idx) = self.resolve_upvalue(self.contexts.len() - 1, ident.text()) { - self.chunk().push_op(OpCode::OpGetUpvalue(idx)); + self.push_op(OpCode::OpGetUpvalue(idx)); return; } @@ -750,11 +755,11 @@ impl Compiler { // both in this scope, and in the upvalues. if self.scope().has_with() { self.emit_constant(Value::String(ident.text().into())); - self.chunk().push_op(OpCode::OpResolveWithOrUpvalue(idx)); + self.push_op(OpCode::OpResolveWithOrUpvalue(idx)); return; } - self.chunk().push_op(OpCode::OpGetUpvalue(idx)); + self.push_op(OpCode::OpGetUpvalue(idx)); return; } @@ -766,12 +771,12 @@ impl Compiler { // Variable needs to be dynamically resolved at // runtime. self.emit_constant(Value::String(ident.text().into())); - self.chunk().push_op(OpCode::OpResolveWith); + self.push_op(OpCode::OpResolveWith); } LocalPosition::Known(idx) => { let stack_idx = self.scope().stack_index(idx); - self.chunk().push_op(OpCode::OpGetLocal(stack_idx)); + self.push_op(OpCode::OpGetLocal(stack_idx)); } // This identifier is referring to a value from the same @@ -801,11 +806,11 @@ impl Compiler { self.scope_mut().push_with(); - self.chunk().push_op(OpCode::OpPushWith(with_idx)); + self.push_op(OpCode::OpPushWith(with_idx)); self.compile(slot, node.body().unwrap()); - self.chunk().push_op(OpCode::OpPopWith); + self.push_op(OpCode::OpPopWith); self.scope_mut().pop_with(); self.end_scope(); } @@ -862,7 +867,7 @@ impl Compiler { .chunk() .push_constant(Value::Blueprint(Rc::new(compiled.lambda))); - self.chunk().push_op(OpCode::OpClosure(blueprint_idx)); + self.push_op(OpCode::OpClosure(blueprint_idx)); self.emit_upvalue_data(slot, compiled.scope.upvalues); } @@ -873,7 +878,7 @@ impl Compiler { // to enter the function call straight away. self.compile(slot, node.argument().unwrap()); self.compile(slot, node.lambda().unwrap()); - self.chunk().push_op(OpCode::OpCall); + self.push_op(OpCode::OpCall); } /// Compile an expression into a runtime thunk which should be @@ -907,7 +912,7 @@ impl Compiler { .chunk() .push_constant(Value::Blueprint(Rc::new(thunk.lambda))); - self.chunk().push_op(OpCode::OpThunk(blueprint_idx)); + self.push_op(OpCode::OpThunk(blueprint_idx)); self.emit_upvalue_data(slot, thunk.scope.upvalues); } @@ -918,7 +923,7 @@ impl Compiler { match upvalue { Upvalue::Local(idx) if slot.is_none() => { let stack_idx = self.scope().stack_index(idx); - self.chunk().push_op(OpCode::DataLocalIdx(stack_idx)); + self.push_op(OpCode::DataLocalIdx(stack_idx)); } Upvalue::Local(idx) => { @@ -929,21 +934,21 @@ impl Compiler { // deferred until the scope is fully initialised // and can be finalised. if slot.unwrap() < idx { - self.chunk().push_op(OpCode::DataDeferredLocal(stack_idx)); + self.push_op(OpCode::DataDeferredLocal(stack_idx)); self.scope_mut().mark_needs_finaliser(slot.unwrap()); } else { - self.chunk().push_op(OpCode::DataLocalIdx(stack_idx)); + self.push_op(OpCode::DataLocalIdx(stack_idx)); } } Upvalue::Upvalue(idx) => { - self.chunk().push_op(OpCode::DataUpvalueIdx(idx)); + self.push_op(OpCode::DataUpvalueIdx(idx)); } Upvalue::Dynamic { name, up } => { let idx = self.chunk().push_constant(Value::String(name.into())); - self.chunk().push_op(OpCode::DataDynamicIdx(idx)); + self.push_op(OpCode::DataDynamicIdx(idx)); if let Some(up) = up { - self.chunk().push_op(OpCode::DataDynamicAncestor(up)); + self.push_op(OpCode::DataDynamicAncestor(up)); } } }; @@ -1021,7 +1026,7 @@ impl Compiler { } if pops > 0 { - self.chunk().push_op(OpCode::OpCloseScope(Count(pops))); + self.push_op(OpCode::OpCloseScope(Count(pops))); } } @@ -1156,7 +1161,7 @@ impl Compiler { } fn emit_force(&mut self) { - self.chunk().push_op(OpCode::OpForce); + self.push_op(OpCode::OpForce); } fn emit_warning(&mut self, node: rnix::SyntaxNode, kind: WarningKind) {