refactor(tvix/eval): take owned ast::Expr in Compiler::compile

This adds a very minimal amount of additional Rc-increments (~1 per
compilation), but makes it a lot easier to add an AST-optimising
compiler pass without incurring a lot of extra cost.

Change-Id: I57208bdfc8882e3ae21c5850e14aa380d3ccea36
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7765
Reviewed-by: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
This commit is contained in:
Vincent Ambo 2023-01-05 14:35:49 +03:00 committed by tazjin
parent 0e421a16c5
commit 36e5a4cc07
2 changed files with 34 additions and 34 deletions

View file

@ -585,7 +585,7 @@ impl Compiler<'_> {
// Create a thunk wrapping value (which may be one as well)
// to avoid forcing the from expr too early.
self.thunk(binding.value_slot, &namespace, |c, s| {
c.compile(s, &namespace);
c.compile(s, namespace.clone());
c.emit_force(&namespace);
c.emit_constant(Value::String(name.into()), &span);
@ -595,7 +595,7 @@ impl Compiler<'_> {
// Binding is "just" a plain expression that needs to be
// compiled.
Binding::Plain { expr } => self.compile(binding.value_slot, &expr),
Binding::Plain { expr } => self.compile(binding.value_slot, expr),
// Binding is a merged or nested attribute set, and needs to be
// recursively compiled as another binding.
@ -651,7 +651,7 @@ impl Compiler<'_> {
self.compile_bindings(slot, BindingsKind::LetIn, node);
// Deal with the body, then clean up the locals afterwards.
self.compile(slot, &node.body().unwrap());
self.compile(slot, node.body().unwrap());
self.cleanup_scope(node);
}

View file

@ -229,8 +229,8 @@ impl Compiler<'_> {
// Actual code-emitting AST traversal methods.
impl Compiler<'_> {
fn compile(&mut self, slot: LocalIdx, expr: &ast::Expr) {
match expr {
fn compile(&mut self, slot: LocalIdx, expr: ast::Expr) {
match &expr {
ast::Expr::Literal(literal) => self.compile_literal(literal),
ast::Expr::Path(path) => self.compile_path(slot, path),
ast::Expr::Str(s) => self.compile_str(slot, s),
@ -277,7 +277,7 @@ impl Compiler<'_> {
// Parenthesized expressions are simply unwrapped, leaving
// their value on the stack.
ast::Expr::Paren(paren) => self.compile(slot, &paren.expr().unwrap()),
ast::Expr::Paren(paren) => self.compile(slot, paren.expr().unwrap()),
ast::Expr::LegacyLet(legacy_let) => self.compile_legacy_let(slot, legacy_let),
@ -372,7 +372,7 @@ impl Compiler<'_> {
// the final string. We need to coerce them here,
// so OpInterpolate definitely has a string to consume.
ast::InterpolPart::Interpolation(ipol) => {
self.compile(slot, &ipol.expr().unwrap());
self.compile(slot, ipol.expr().unwrap());
// implicitly forces as well
self.push_op(OpCode::OpCoerceToString, ipol);
}
@ -406,7 +406,7 @@ impl Compiler<'_> {
}
fn compile_unary_op(&mut self, slot: LocalIdx, op: &ast::UnaryOp) {
self.compile(slot, &op.expr().unwrap());
self.compile(slot, op.expr().unwrap());
self.emit_force(op);
let opcode = match op.operator().unwrap() {
@ -435,10 +435,10 @@ impl Compiler<'_> {
// For all other operators, the two values need to be left on
// the stack in the correct order before pushing the
// instruction for the operation itself.
self.compile(slot, &op.lhs().unwrap());
self.compile(slot, op.lhs().unwrap());
self.emit_force(&op.lhs().unwrap());
self.compile(slot, &op.rhs().unwrap());
self.compile(slot, op.rhs().unwrap());
self.emit_force(&op.rhs().unwrap());
match op.operator().unwrap() {
@ -474,7 +474,7 @@ impl Compiler<'_> {
);
// Leave left-hand side value on the stack.
self.compile(slot, &node.lhs().unwrap());
self.compile(slot, node.lhs().unwrap());
self.emit_force(&node.lhs().unwrap());
// If this value is false, jump over the right-hand side - the
@ -485,7 +485,7 @@ impl Compiler<'_> {
// right-hand side on the stack. Its result is now the value
// of the whole expression.
self.push_op(OpCode::OpPop, node);
self.compile(slot, &node.rhs().unwrap());
self.compile(slot, node.rhs().unwrap());
self.emit_force(&node.rhs().unwrap());
self.patch_jump(end_idx);
@ -500,14 +500,14 @@ impl Compiler<'_> {
);
// Leave left-hand side value on the stack
self.compile(slot, &node.lhs().unwrap());
self.compile(slot, node.lhs().unwrap());
self.emit_force(&node.lhs().unwrap());
// Opposite of above: If this value is **true**, we can
// short-circuit the right-hand side.
let end_idx = self.push_op(OpCode::OpJumpIfTrue(JumpOffset(0)), node);
self.push_op(OpCode::OpPop, node);
self.compile(slot, &node.rhs().unwrap());
self.compile(slot, node.rhs().unwrap());
self.emit_force(&node.rhs().unwrap());
self.patch_jump(end_idx);
@ -522,14 +522,14 @@ impl Compiler<'_> {
);
// Leave left-hand side value on the stack and invert it.
self.compile(slot, &node.lhs().unwrap());
self.compile(slot, node.lhs().unwrap());
self.emit_force(&node.lhs().unwrap());
self.push_op(OpCode::OpInvert, node);
// Exactly as `||` (because `a -> b` = `!a || b`).
let end_idx = self.push_op(OpCode::OpJumpIfTrue(JumpOffset(0)), node);
self.push_op(OpCode::OpPop, node);
self.compile(slot, &node.rhs().unwrap());
self.compile(slot, node.rhs().unwrap());
self.emit_force(&node.rhs().unwrap());
self.patch_jump(end_idx);
@ -563,7 +563,7 @@ impl Compiler<'_> {
};
count += 1;
self.compile(item_slot, &item);
self.compile(item_slot, item);
self.scope_mut().mark_initialised(item_slot);
}
@ -574,7 +574,7 @@ impl Compiler<'_> {
fn compile_attr(&mut self, slot: LocalIdx, node: &ast::Attr) {
match node {
ast::Attr::Dynamic(dynamic) => {
self.compile(slot, &dynamic.expr().unwrap());
self.compile(slot, dynamic.expr().unwrap());
self.emit_force(&dynamic.expr().unwrap());
}
@ -589,7 +589,7 @@ impl Compiler<'_> {
fn compile_has_attr(&mut self, slot: LocalIdx, node: &ast::HasAttr) {
// Put the attribute set on the stack.
self.compile(slot, &node.expr().unwrap());
self.compile(slot, node.expr().unwrap());
self.emit_force(node);
// Push all path fragments with an operation for fetching the
@ -618,7 +618,7 @@ impl Compiler<'_> {
}
// Push the set onto the stack
self.compile(slot, &set);
self.compile(slot, set);
// Compile each key fragment and emit access instructions.
//
@ -669,7 +669,7 @@ impl Compiler<'_> {
path: ast::Attrpath,
default: ast::Expr,
) {
self.compile(slot, &set);
self.compile(slot, set);
let mut jumps = vec![];
for fragment in path.attrs() {
@ -687,7 +687,7 @@ impl Compiler<'_> {
// Compile the default value expression and patch the final
// jump to point *beyond* it.
self.compile(slot, &default);
self.compile(slot, default);
self.patch_jump(final_jump);
}
@ -705,12 +705,12 @@ impl Compiler<'_> {
/// ```
fn compile_assert(&mut self, slot: LocalIdx, node: &ast::Assert) {
// 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());
let then_idx = self.push_op(OpCode::OpJumpIfFalse(JumpOffset(0)), node);
self.push_op(OpCode::OpPop, node);
self.compile(slot, &node.body().unwrap());
self.compile(slot, node.body().unwrap());
let else_idx = self.push_op(OpCode::OpJump(JumpOffset(0)), node);
@ -734,7 +734,7 @@ impl Compiler<'_> {
/// └────────────────────┘
/// ```
fn compile_if_else(&mut self, slot: LocalIdx, node: &ast::IfElse) {
self.compile(slot, &node.condition().unwrap());
self.compile(slot, node.condition().unwrap());
self.emit_force(&node.condition().unwrap());
let then_idx = self.push_op(
@ -743,13 +743,13 @@ impl Compiler<'_> {
);
self.push_op(OpCode::OpPop, node); // discard condition value
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); // patch jump *to* else_body
self.push_op(OpCode::OpPop, node); // discard condition value
self.compile(slot, &node.else_body().unwrap());
self.compile(slot, node.else_body().unwrap());
self.patch_jump(else_idx); // patch jump *over* else body
}
@ -762,7 +762,7 @@ impl Compiler<'_> {
// TODO: Detect if the namespace is just an identifier, and
// resolve that directly (thus avoiding duplication on the
// stack).
self.compile(slot, &node.namespace().unwrap());
self.compile(slot, node.namespace().unwrap());
let span = self.span_for(&node.namespace().unwrap());
@ -778,7 +778,7 @@ impl Compiler<'_> {
self.push_op(OpCode::OpPushWith(with_idx), &node.namespace().unwrap());
self.compile(slot, &node.body().unwrap());
self.compile(slot, node.body().unwrap());
self.push_op(OpCode::OpPopWith, node);
self.scope_mut().pop_with();
@ -865,7 +865,7 @@ impl Compiler<'_> {
let jump_over_default = self.push_op(OpCode::OpJump(JumpOffset(0)), &default_expr);
self.patch_jump(jump_to_default);
self.compile(idx, &default_expr);
self.compile(idx, default_expr);
self.patch_jump(jump_over_default);
} else {
self.push_op(OpCode::OpAttrsSelect, &entry.ident().unwrap());
@ -909,7 +909,7 @@ impl Compiler<'_> {
}
};
self.compile(slot, &node.body().unwrap());
self.compile(slot, node.body().unwrap());
self.context_mut().lambda.formals = formals;
}
@ -1026,8 +1026,8 @@ impl Compiler<'_> {
// followed by the function expression itself, and then emit a
// call instruction. This way, the stack is perfectly laid out
// to enter the function call straight away.
self.compile(slot, &node.argument().unwrap());
self.compile(slot, &node.lambda().unwrap());
self.compile(slot, node.argument().unwrap());
self.compile(slot, node.lambda().unwrap());
self.emit_force(&node.lambda().unwrap());
self.push_op(OpCode::OpCall, node);
}
@ -1295,7 +1295,7 @@ pub fn compile(
let root_span = c.span_for(expr);
let root_slot = c.scope_mut().declare_phantom(root_span, false);
c.compile(root_slot, expr);
c.compile(root_slot, expr.clone());
// The final operation of any top-level Nix program must always be
// `OpForce`. A thunk should not be returned to the user in an