maurice/snix
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

fix(tvix/eval): instantiate *new* closures from blueprints each time

Browse source 
The previous closure refactoring introduced a bug in which the same
closure object would get mutated constantly for each instance of a
closure, which is incorrect behaviour.

This commit instead introduces an explicit new Value variant for the
internal "blueprint" that the compiler generates (essentially just the
lambda) and uses this variant to construct the closure at runtime.

If the blueprint ever leaks out to a user somehow that is a critical
bug and tvix-eval will panic.

As a ~treat~ test for this, the fibonacci function is being used as it
is a self-recursive closure (i.e. different instantiations of the same
"blueprint") getting called with different values and it's good to
have it around.

Change-Id: I485de675e9bb0c599ed7d5dc0f001eb34ab4c15f
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6323
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
This commit is contained in:
Vincent Ambo 2022-08-28 06:07:20 +03:00 • committed by tazjin
parent ccfb971dc5
commit 776120de05
6 changed files with 33 additions and 17 deletions
Download patch file Download diff file Expand all files Collapse all files

7
tvix/eval/src/compiler/mod.rs
View file

@ -805,16 +805,17 @@ impl Compiler {
// If the function is not a closure, just emit it directly and
// move on.
if compiled.lambda.upvalue_count == 0 {
self.emit_constant(Value::Closure(Closure::new(compiled.lambda)));
self.emit_constant(Value::Closure(Closure::new(Rc::new(compiled.lambda))));
return;
}
// If the function is a closure, we need to emit the variable
// number of operands that allow the runtime to close over the
// upvalues.
// upvalues and leave a blueprint in the constant index from
// which the runtime closure can be constructed.
let closure_idx = self
.chunk()
.push_constant(Value::Closure(Closure::new(compiled.lambda)));
.push_constant(Value::Blueprint(Rc::new(compiled.lambda)));
self.chunk().push_op(OpCode::OpClosure(closure_idx));
for upvalue in compiled.scope.upvalues {