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 <sternenseemann@systemli.org>
This commit is contained in:
parent
26acc2e636
commit
7c2fdefcd8
1 changed files with 5 additions and 13 deletions
|
@ -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
|
||||
|
|
Loading…
Reference in a new issue