From 1f84d9081130eb55b911a2542ac4781ee1438fc4 Mon Sep 17 00:00:00 2001 From: Vincent Ambo Date: Tue, 11 Oct 2022 02:14:47 +0300 Subject: refactor(tvix/eval): after calling, the caller has to pop Previously the various call functions either returned `EvalResult<()>` or `EvalResult`, which was confusing. Now only vm::call_with returns a Value directly, and other parts of the API just leave the stack top in the post-call state. This makes it easier to reason about what's going on in non-tail-call cases (which are making a comeback). Change-Id: I264ffc683a11aca72dd06e2220a5ff6e7c5fc2b0 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6936 Tested-by: BuildkiteCI Reviewed-by: grfn --- tvix/eval/src/value/mod.rs | 3 ++- tvix/eval/src/value/thunk.rs | 7 +++---- tvix/eval/src/vm.rs | 40 ++++++++++++++++++++++++---------------- 3 files changed, 29 insertions(+), 21 deletions(-) diff --git a/tvix/eval/src/value/mod.rs b/tvix/eval/src/value/mod.rs index fd5b5255c5d8..3791aaf56b9b 100644 --- a/tvix/eval/src/value/mod.rs +++ b/tvix/eval/src/value/mod.rs @@ -163,7 +163,8 @@ impl Value { let call_to_string = |value: &Value, vm: &mut VM| { // Leave self on the stack as an argument to the function call. vm.push(self.clone()); - let result = vm.call_value(value)?; + vm.call_value(value)?; + let result = vm.pop(); match result { Value::String(s) => Ok(s), diff --git a/tvix/eval/src/value/thunk.rs b/tvix/eval/src/value/thunk.rs index ed994ebd7d57..7cb79dedc84a 100644 --- a/tvix/eval/src/value/thunk.rs +++ b/tvix/eval/src/value/thunk.rs @@ -87,10 +87,9 @@ impl Thunk { std::mem::replace(&mut *thunk_mut, ThunkRepr::Blackhole) { drop(thunk_mut); - let evaluated = ThunkRepr::Evaluated( - vm.call(lambda, upvalues, 0) - .map_err(|e| ErrorKind::ThunkForce(Box::new(e)))?, - ); + vm.enter_frame(lambda, upvalues, 0) + .map_err(|e| ErrorKind::ThunkForce(Box::new(e)))?; + let evaluated = ThunkRepr::Evaluated(vm.pop()); (*self.0.borrow_mut()) = evaluated; } } diff --git a/tvix/eval/src/vm.rs b/tvix/eval/src/vm.rs index 44ddda5a1f63..924e8a9ebdae 100644 --- a/tvix/eval/src/vm.rs +++ b/tvix/eval/src/vm.rs @@ -219,14 +219,11 @@ impl<'o> VM<'o> { /// The stack of the VM must be prepared with all required /// arguments before calling this and the value must have already /// been forced. - pub fn call_value(&mut self, callable: &Value) -> EvalResult { + pub fn call_value(&mut self, callable: &Value) -> EvalResult<()> { match callable { - Value::Closure(c) => self.call(c.lambda(), c.upvalues().clone(), 1), + Value::Closure(c) => self.enter_frame(c.lambda(), c.upvalues().clone(), 1), - Value::Builtin(b) => { - self.call_builtin(b.clone())?; - Ok(self.pop()) - } + Value::Builtin(b) => self.call_builtin(b.clone()), Value::Thunk(t) => self.call_value(&t.value()), @@ -250,13 +247,19 @@ impl<'o> VM<'o> { num_args += 1; self.push(arg); } + if num_args == 0 { panic!("call_with called with an empty list of args"); } - let mut res = self.call_value(callable)?; + + self.call_value(callable)?; + let mut res = self.pop(); + for _ in 0..(num_args - 1) { - res = self.call_value(&res)?; + self.call_value(&res)?; + res = self.pop(); } + Ok(res) } @@ -287,7 +290,8 @@ impl<'o> VM<'o> { // synthetic (i.e. there is no corresponding OpCall for the // first call in the bytecode.) self.push(callable.clone()); - let primed = self.call_value(functor)?; + self.call_value(functor)?; + let primed = self.pop(); self.tail_call_value(primed) } }, @@ -298,12 +302,12 @@ impl<'o> VM<'o> { /// Execute the given lambda in this VM's context, returning its /// value after its stack frame completes. - pub fn call( + pub fn enter_frame( &mut self, lambda: Rc, upvalues: Upvalues, arg_count: usize, - ) -> EvalResult { + ) -> EvalResult<()> { self.observer .observe_enter_frame(arg_count, &lambda, self.frames.len() + 1); @@ -322,16 +326,19 @@ impl<'o> VM<'o> { result } - /// Run the VM's current stack frame to completion and return the - /// value. - fn run(&mut self) -> EvalResult { + /// Run the VM's current call frame to completion. + /// + /// On successful return, the top of the stack is the value that + /// the frame evaluated to. The frame itself is popped off. It is + /// up to the caller to consume the value. + fn run(&mut self) -> EvalResult<()> { loop { // Break the loop if this call frame has already run to // completion, pop it off, and return the value to the // caller. if self.frame().ip.0 == self.chunk().code.len() { self.frames.pop(); - return Ok(self.pop()); + return Ok(()); } let op = self.inc_ip(); @@ -853,7 +860,8 @@ pub fn run_lambda( lambda: Rc, ) -> EvalResult { let mut vm = VM::new(nix_path, observer); - let value = vm.call(lambda, Upvalues::with_capacity(0), 0)?; + vm.enter_frame(lambda, Upvalues::with_capacity(0), 0)?; + let value = vm.pop(); vm.force_for_output(&value)?; Ok(RuntimeResult { -- cgit 1.4.1