From 7c2fdefcd8ff693be1afd15a08958f09d07a0e91 Mon Sep 17 00:00:00 2001 From: Vincent Ambo Date: Sat, 27 Aug 2022 02:59:03 +0300 Subject: fix(tvix/eval): pop with stack immediately after processing body Instead of tying the popping of the with stack to scope depth, clean up the stack immediately after processing a with body. The previous behaviour was actually incorrect, as it would leave things on the with-stack longer than they were supposed to be there. This could lead to false positive resolutions in some situations involving closures. Change-Id: I7b0638557503f1f71eb602e3d5ff193cdfcb67cc Reviewed-on: https://cl.tvl.fyi/c/depot/+/6297 Tested-by: BuildkiteCI Reviewed-by: sterni --- tvix/eval/src/compiler/mod.rs | 18 +++++------------- 1 file changed, 5 insertions(+), 13 deletions(-) diff --git a/tvix/eval/src/compiler/mod.rs b/tvix/eval/src/compiler/mod.rs index 0c41f06cbde1..efcb83b8a4d4 100644 --- a/tvix/eval/src/compiler/mod.rs +++ b/tvix/eval/src/compiler/mod.rs @@ -59,9 +59,7 @@ struct Local { /// Represents a stack offset containing keys which are currently /// in-scope through a with expression. #[derive(Debug)] -struct With { - depth: usize, -} +struct With {} #[derive(Debug, PartialEq)] enum Upvalue { @@ -811,13 +809,15 @@ impl Compiler { self.compile(node.namespace().unwrap()); self.declare_phantom(); - let depth = self.scope().scope_depth; - self.scope_mut().with_stack.push(With { depth }); + self.scope_mut().with_stack.push(With {}); let with_idx = self.scope().locals.len() - 1; self.chunk().push_op(OpCode::OpPushWith(StackIdx(with_idx))); self.compile(node.body().unwrap()); + + self.chunk().push_op(OpCode::OpPopWith); + self.scope_mut().with_stack.pop(); } fn compile_lambda(&mut self, node: ast::Lambda) { @@ -961,14 +961,6 @@ impl Compiler { if pops > 0 { self.chunk().push_op(OpCode::OpCloseScope(Count(pops))); } - - while !self.scope().with_stack.is_empty() - && self.scope().with_stack[self.scope().with_stack.len() - 1].depth - > self.scope().scope_depth - { - self.chunk().push_op(OpCode::OpPopWith); - self.scope_mut().with_stack.pop(); - } } /// Declare a local variable known in the scope that is being -- cgit 1.4.1