From e8dcdceb34585dfe82d826978f0f1cd8a673b474 Mon Sep 17 00:00:00 2001 From: Vincent Ambo Date: Thu, 5 Jan 2023 15:25:06 +0300 Subject: 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 Tested-by: BuildkiteCI --- tvix/eval/src/compiler/mod.rs | 30 ++++++++++++++++++++++++++++-- tvix/eval/src/compiler/optimiser.rs | 12 +++++------- 2 files changed, 33 insertions(+), 9 deletions(-) diff --git a/tvix/eval/src/compiler/mod.rs b/tvix/eval/src/compiler/mod.rs index 2f4fc4da64b6..9fb24aa98ec9 100644 --- a/tvix/eval/src/compiler/mod.rs +++ b/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(&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(&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. diff --git a/tvix/eval/src/compiler/optimiser.rs b/tvix/eval/src/compiler/optimiser.rs index 533b6c540c25..615de32357e6 100644 --- a/tvix/eval/src/compiler/optimiser.rs +++ b/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; } -- cgit 1.4.1