From 0b06d946062f254d8e097f7a0a0ad308b159d739 Mon Sep 17 00:00:00 2001
From: Griffin Smith <root@gws.fyi>
Date: Sun, 9 Oct 2022 13:18:32 -0400
Subject: refactor(tvix/eval): Abstract away calling functions

The process of calling a function from a builtin, especially if it's got
more than 1 arrgument, is reasonably involved and easy to get wrong due
to having to interact directly with the stack - instead of having that
done entirely manually in builtins, this wraps it up in a new
`call_with` function which handles pushing arguments onto the stack and
recursively calling the (partially applied) function.

Change-Id: I14700c639a0deca53b9a060f6d70dbc7762e9007
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6910
Autosubmit: grfn <grfn@gws.fyi>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
---
 tvix/eval/src/builtins/mod.rs | 34 +++++++---------------------------
 tvix/eval/src/vm.rs           | 25 +++++++++++++++++++++++++
 2 files changed, 32 insertions(+), 27 deletions(-)

diff --git a/tvix/eval/src/builtins/mod.rs b/tvix/eval/src/builtins/mod.rs
index f32a29a20660..5819796236cd 100644
--- a/tvix/eval/src/builtins/mod.rs
+++ b/tvix/eval/src/builtins/mod.rs
@@ -60,10 +60,7 @@ fn pure_builtins() -> Vec<Builtin> {
         }),
         Builtin::new("all", &[true, true], |args: Vec<Value>, vm: &mut VM| {
             for value in args[1].to_list()?.into_iter() {
-                let pred_result = {
-                    vm.push(value);
-                    vm.call_value(&args[0])
-                }?;
+                let pred_result = vm.call_with(&args[0], [value])?;
 
                 if !pred_result.force(vm)?.as_bool()? {
                     return Ok(Value::Bool(false));
@@ -74,10 +71,7 @@ fn pure_builtins() -> Vec<Builtin> {
         }),
         Builtin::new("any", &[true, true], |args: Vec<Value>, vm: &mut VM| {
             for value in args[1].to_list()?.into_iter() {
-                let pred_result = {
-                    vm.push(value);
-                    vm.call_value(&args[0])
-                }?;
+                let pred_result = vm.call_with(&args[0], [value])?;
 
                 if pred_result.force(vm)?.as_bool()? {
                     return Ok(Value::Bool(true));
@@ -170,8 +164,7 @@ fn pure_builtins() -> Vec<Builtin> {
                 let list = args[1].to_list()?;
                 let mut res = Vec::new();
                 for val in list {
-                    vm.push(val);
-                    res.extend(vm.call_value(&args[0])?.force(vm)?.to_list()?);
+                    res.extend(vm.call_with(&args[0], [val])?.force(vm)?.to_list()?);
                 }
                 Ok(Value::List(res.into()))
             },
@@ -198,9 +191,7 @@ fn pure_builtins() -> Vec<Builtin> {
 
             list.into_iter()
                 .filter_map(|elem| {
-                    vm.push(elem.clone());
-
-                    let result = match vm.call_value(&args[0]) {
+                    let result = match vm.call_with(&args[0], [elem.clone()]) {
                         Err(err) => return Some(Err(err)),
                         Ok(result) => result,
                     };
@@ -231,10 +222,7 @@ fn pure_builtins() -> Vec<Builtin> {
                 let op = args.pop().unwrap();
                 for val in list {
                     val.force(vm)?;
-                    vm.push(val);
-                    vm.push(res);
-                    let partial = vm.call_value(&op)?;
-                    res = vm.call_value(&partial)?;
+                    res = vm.call_with(&op, [val, res])?;
                 }
 
                 Ok(res)
@@ -243,10 +231,7 @@ fn pure_builtins() -> Vec<Builtin> {
         Builtin::new("genList", &[true, true], |args: Vec<Value>, vm: &mut VM| {
             let len = args[1].as_int()?;
             (0..len)
-                .map(|i| {
-                    vm.push(i.into());
-                    vm.call_value(&args[0])
-                })
+                .map(|i| vm.call_with(&args[0], [i.into()]))
                 .collect::<Result<Vec<Value>, _>>()
                 .map(|list| Value::List(NixList::from(list)))
                 .map_err(Into::into)
@@ -269,12 +254,7 @@ fn pure_builtins() -> Vec<Builtin> {
             let list: NixList = args[1].to_list()?;
 
             list.into_iter()
-                .map(|val| {
-                    // Leave the argument on the stack before calling the
-                    // function.
-                    vm.push(val);
-                    vm.call_value(&args[0])
-                })
+                .map(|val| vm.call_with(&args[0], [val]))
                 .collect::<Result<Vec<Value>, _>>()
                 .map(|list| Value::List(NixList::from(list)))
                 .map_err(Into::into)
diff --git a/tvix/eval/src/vm.rs b/tvix/eval/src/vm.rs
index 9d28c7be1b93..b89b61f7626a 100644
--- a/tvix/eval/src/vm.rs
+++ b/tvix/eval/src/vm.rs
@@ -229,6 +229,31 @@ impl<'o> VM<'o> {
         }
     }
 
+    /// Call the given `callable` value with the given list of `args`
+    ///
+    /// # Panics
+    ///
+    /// Panics if the passed list of `args` is empty
+    #[track_caller]
+    pub fn call_with<I>(&mut self, callable: &Value, args: I) -> EvalResult<Value>
+    where
+        I: IntoIterator<Item = Value>,
+    {
+        let mut num_args = 0_usize;
+        for arg in args {
+            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)?;
+        for _ in 0..(num_args - 1) {
+            res = self.call_value(&res)?;
+        }
+        Ok(res)
+    }
+
     fn tail_call_value(&mut self, callable: Value) -> EvalResult<()> {
         match callable {
             Value::Builtin(builtin) => self.call_builtin(builtin),
-- 
cgit 1.4.1