fix(tvix/eval): compile but don't emit dead code

This adds a mechanism to the compiler to compile an expression without
emitting any code. This allows for detected dead code to still be
compiled to detect errors & warnings inside of it.

Change-Id: Ie78479173570e9c819d8f32ae683ce34234a4c5d
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7767
Reviewed-by: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI

tvix/eval/src/compiler/mod.rs

@@ -133,6 +133,11 @@ pub struct Compiler<'observer> {
     /// Carry an observer for the compilation process, which is called
     /// whenever a chunk is emitted.
     observer: &'observer mut dyn CompilerObserver,
+
+    /// Carry a count of nested scopes which have requested the
+    /// compiler not to emit anything. This used for compiling dead
+    /// code branches to catch errors & warnings in them.
+    dead_scope: usize,
 }
 
 impl Compiler<'_> {
@@ -185,6 +190,7 @@ impl<'observer> Compiler<'observer> {
             contexts: vec![LambdaCtx::new()],
             warnings: vec![],
             errors: vec![],
+            dead_scope: 0,
         })
     }
 }
@@ -216,6 +222,10 @@ impl Compiler<'_> {
     /// Push a single instruction to the current bytecode chunk and
     /// track the source span from which it was compiled.
     fn push_op<T: ToSpan>(&mut self, data: OpCode, node: &T) -> CodeIdx {
+        if self.dead_scope > 0 {
+            return CodeIdx(0);
+        }
+
         let span = self.span_for(node);
         self.chunk().push_op(data, span)
     }
@@ -223,6 +233,10 @@ impl Compiler<'_> {
     /// Emit a single constant to the current bytecode chunk and track
     /// the source span from which it was compiled.
     pub(super) fn emit_constant<T: ToSpan>(&mut self, value: Value, node: &T) {
+        if self.dead_scope > 0 {
+            return;
+        }
+
         let idx = self.chunk().push_constant(value);
         self.push_op(OpCode::OpConstant(idx), node);
     }
@@ -231,7 +245,7 @@ impl Compiler<'_> {
 // Actual code-emitting AST traversal methods.
 impl Compiler<'_> {
     fn compile(&mut self, slot: LocalIdx, expr: ast::Expr) {
-        let expr = optimiser::optimise_expr(self, expr);
+        let expr = optimiser::optimise_expr(self, slot, expr);
 
         match &expr {
             ast::Expr::Literal(literal) => self.compile_literal(literal),
@@ -289,6 +303,16 @@ impl Compiler<'_> {
         }
     }
 
+    /// Compiles an expression, but does not emit any code for it as
+    /// it is considered dead. This will still catch errors and
+    /// warnings in that expression.
+    fn compile_dead_code(&mut self, slot: LocalIdx, node: ast::Expr) {
+        self.dead_scope += 1;
+        self.emit_warning(&node, WarningKind::DeadCode);
+        self.compile(slot, node);
+        self.dead_scope -= 1;
+    }
+
     fn compile_literal(&mut self, node: &ast::Literal) {
         let value = match node.kind() {
             ast::LiteralKind::Float(f) => Value::Float(f.value().unwrap()),
@@ -956,7 +980,9 @@ impl Compiler<'_> {
         let mut compiled = self.contexts.pop().unwrap();
 
         // Check if tail-call optimisation is possible and perform it.
-        optimise_tail_call(&mut compiled.lambda.chunk);
+        if self.dead_scope == 0 {
+            optimise_tail_call(&mut compiled.lambda.chunk);
+        }
 
         // Capturing the with stack counts as an upvalue, as it is
         // emitted as an upvalue data instruction.

tvix/eval/src/compiler/optimiser.rs

@@ -6,9 +6,9 @@ use super::*;
 use ast::Expr;
 
 /// Optimise the given expression where possible.
-pub(super) fn optimise_expr(c: &mut Compiler, expr: ast::Expr) -> ast::Expr {
+pub(super) fn optimise_expr(c: &mut Compiler, slot: LocalIdx, expr: ast::Expr) -> ast::Expr {
     match expr {
-        Expr::BinOp(_) => optimise_bin_op(c, expr),
+        Expr::BinOp(_) => optimise_bin_op(c, slot, expr),
         _ => expr.to_owned(),
     }
 }
@@ -33,7 +33,7 @@ fn is_lit_bool(expr: ast::Expr) -> LitBool {
 }
 
 /// Detect useless binary operations (i.e. useless bool comparisons).
-fn optimise_bin_op(c: &mut Compiler, expr: ast::Expr) -> ast::Expr {
+fn optimise_bin_op(c: &mut Compiler, slot: LocalIdx, expr: ast::Expr) -> ast::Expr {
     use ast::BinOpKind;
 
     // bail out of this check if the user has poisoned either `true`
@@ -82,8 +82,7 @@ fn optimise_bin_op(c: &mut Compiler, expr: ast::Expr) -> ast::Expr {
                     WarningKind::UselessBoolOperation("this expression is always true"),
                 );
 
-                // TODO: still compile other to get errors/warnings from there, but don't emit
-                c.emit_warning(&other, WarningKind::DeadCode);
+                c.compile_dead_code(slot, other);
 
                 return t;
             }
@@ -104,8 +103,7 @@ fn optimise_bin_op(c: &mut Compiler, expr: ast::Expr) -> ast::Expr {
                     WarningKind::UselessBoolOperation("this expression is always false"),
                 );
 
-                // TODO: still compile other to get errors/warnings from there, but don't emit
-                c.emit_warning(&other, WarningKind::DeadCode);
+                c.compile_dead_code(slot, other);
 
                 return f;
             }