fix(tvix/eval): handle thunks in arithmetic builtins r/4960

The simplest solution seems to be to pass references to arithmetic_op!()
which avoids the moving annoyance we had to deal with in the
builtins (no more popping!). We then use .force() to force the values
and dereference any Thunks (which arithmetic_op! doesn't do for us).

Change-Id: I0eb8ad60e80a0b3ba9d9f411e973ef8bcf136989
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6724
Tested-by: BuildkiteCI
Reviewed-by: wpcarro <wpcarro@gmail.com>
Reviewed-by: tazjin <tazjin@tvl.su>

10 files changed, 32 insertions, 28 deletions

diff --git a/tvix/eval/src/builtins/mod.rs b/tvix/eval/src/builtins/mod.rs
index 650cfe674f..1a6155a823 100644
--- a/tvix/eval/src/builtins/mod.rs
+++ b/tvix/eval/src/builtins/mod.rs
@@ -52,11 +52,11 @@ pub fn coerce_value_to_path(v: &Value, vm: &mut VM) -> Result<PathBuf, ErrorKind
 /// WASM).
 fn pure_builtins() -> Vec<Builtin> {
     vec![
-        Builtin::new("add", &[true, true], |mut args, _| {
-            let b = args.pop().unwrap();
-            let a = args.pop().unwrap();
-            arithmetic_op!(a, b, +)
-        }),
+        Builtin::new(
+            "add",
+            &[false, false],
+            |args, vm| arithmetic_op!(&*args[0].force(vm)?, &*args[1].force(vm)?, +),
+        ),
         Builtin::new("abort", &[true], |args, _| {
             return Err(ErrorKind::Abort(args[0].to_str()?.to_string()));
         }),
@@ -115,11 +115,11 @@ fn pure_builtins() -> Vec<Builtin> {
                 std::cmp::Ordering::Greater => Ok(Value::Integer(1)),
             }
         }),
-        Builtin::new("div", &[true, true], |mut args, _| {
-            let b = args.pop().unwrap();
-            let a = args.pop().unwrap();
-            arithmetic_op!(a, b, /)
-        }),
+        Builtin::new(
+            "div",
+            &[false, false],
+            |args, vm| arithmetic_op!(&*args[0].force(vm)?, &*args[1].force(vm)?, /),
+        ),
         Builtin::new("elemAt", &[true, true], |args, _| {
             let xs = args[0].to_list()?;
             let i = args[1].as_int()?;
@@ -215,11 +215,11 @@ fn pure_builtins() -> Vec<Builtin> {
             let value = args[0].force(vm)?;
             Ok(Value::Bool(matches!(*value, Value::String(_))))
         }),
-        Builtin::new("mul", &[true, true], |mut args, _| {
-            let b = args.pop().unwrap();
-            let a = args.pop().unwrap();
-            arithmetic_op!(a, b, *)
-        }),
+        Builtin::new(
+            "mul",
+            &[false, false],
+            |args, vm| arithmetic_op!(&*args[0].force(vm)?, &*args[1].force(vm)?, *),
+        ),
         Builtin::new("splitVersion", &[true], |args, _| {
             let s = args[0].to_str()?;
             let s = VersionPartsIter::new(s.as_str());
@@ -234,11 +234,11 @@ fn pure_builtins() -> Vec<Builtin> {
                 .collect::<Vec<Value>>();
             Ok(Value::List(NixList::construct(parts.len(), parts)))
         }),
-        Builtin::new("sub", &[true, true], |mut args, _| {
-            let b = args.pop().unwrap();
-            let a = args.pop().unwrap();
-            arithmetic_op!(a, b, -)
-        }),
+        Builtin::new(
+            "sub",
+            &[false, false],
+            |args, vm| arithmetic_op!(&*args[0].force(vm)?, &*args[1].force(vm)?, -),
+        ),
         Builtin::new("substring", &[true, true, true], |args, _| {
             let beg = args[0].as_int()?;
             let len = args[1].as_int()?;
diff --git a/tvix/eval/src/tests/tvix_tests/eval-okay-builtins-add.exp b/tvix/eval/src/tests/tvix_tests/eval-okay-builtins-add.exp
index 25c0a9cc0f..c3ac813de6 100644
--- a/tvix/eval/src/tests/tvix_tests/eval-okay-builtins-add.exp
+++ b/tvix/eval/src/tests/tvix_tests/eval-okay-builtins-add.exp
@@ -1 +1 @@
-[ 18 18.9 18.9 19.1 19 ]
+[ 18 18.9 18.9 19.1 19 42 ]
diff --git a/tvix/eval/src/tests/tvix_tests/eval-okay-builtins-add.nix b/tvix/eval/src/tests/tvix_tests/eval-okay-builtins-add.nix
index 6b2a29d798..b04b1d1fa6 100644
--- a/tvix/eval/src/tests/tvix_tests/eval-okay-builtins-add.nix
+++ b/tvix/eval/src/tests/tvix_tests/eval-okay-builtins-add.nix
@@ -4,4 +4,5 @@
   (builtins.add 7 11.9)
   (builtins.add 7.2 11.9)
   (builtins.add 7.1 11.9)
+  (builtins.add (builtins.add 21 10) 11)
 ]
diff --git a/tvix/eval/src/tests/tvix_tests/eval-okay-builtins-div.exp b/tvix/eval/src/tests/tvix_tests/eval-okay-builtins-div.exp
index d71bbb7a06..73e9bc33b0 100644
--- a/tvix/eval/src/tests/tvix_tests/eval-okay-builtins-div.exp
+++ b/tvix/eval/src/tests/tvix_tests/eval-okay-builtins-div.exp
@@ -1 +1 @@
-[ 3 7 0 1 0 0.5 0.5 0.5 ]
+[ 3 7 0 1 0 0.5 0.5 0.5 42 ]
diff --git a/tvix/eval/src/tests/tvix_tests/eval-okay-builtins-div.nix b/tvix/eval/src/tests/tvix_tests/eval-okay-builtins-div.nix
index 622ce889da..98b8b74bdf 100644
--- a/tvix/eval/src/tests/tvix_tests/eval-okay-builtins-div.nix
+++ b/tvix/eval/src/tests/tvix_tests/eval-okay-builtins-div.nix
@@ -7,4 +7,5 @@
   (builtins.div 1.0 2)
   (builtins.div 1 2.0)
   (builtins.div 1.0 2.0)
+  (builtins.div (builtins.div 84 4) 0.5)
 ]
diff --git a/tvix/eval/src/tests/tvix_tests/eval-okay-builtins-mul.exp b/tvix/eval/src/tests/tvix_tests/eval-okay-builtins-mul.exp
index 5ed8b8e2bd..e3e0f03a8a 100644
--- a/tvix/eval/src/tests/tvix_tests/eval-okay-builtins-mul.exp
+++ b/tvix/eval/src/tests/tvix_tests/eval-okay-builtins-mul.exp
@@ -1 +1 @@
-[ 36 0 0 14 ]
+[ 36 0 0 14 42 ]
diff --git a/tvix/eval/src/tests/tvix_tests/eval-okay-builtins-mul.nix b/tvix/eval/src/tests/tvix_tests/eval-okay-builtins-mul.nix
index 5b61e387a6..2a8d6c4214 100644
--- a/tvix/eval/src/tests/tvix_tests/eval-okay-builtins-mul.nix
+++ b/tvix/eval/src/tests/tvix_tests/eval-okay-builtins-mul.nix
@@ -3,4 +3,5 @@
   (builtins.mul 0 7)
   (builtins.mul 7 0)
   (builtins.mul 7 2)
+  (builtins.mul (builtins.mul 4 0.5) 21)
 ]
diff --git a/tvix/eval/src/tests/tvix_tests/eval-okay-builtins-sub.exp b/tvix/eval/src/tests/tvix_tests/eval-okay-builtins-sub.exp
index c47a17fa27..51842eccfa 100644
--- a/tvix/eval/src/tests/tvix_tests/eval-okay-builtins-sub.exp
+++ b/tvix/eval/src/tests/tvix_tests/eval-okay-builtins-sub.exp
@@ -1 +1 @@
-[ -4 -3.1 -4.9 -4.7 -4 ]
+[ -4 -3.1 -4.9 -4.7 -4 42 ]
diff --git a/tvix/eval/src/tests/tvix_tests/eval-okay-builtins-sub.nix b/tvix/eval/src/tests/tvix_tests/eval-okay-builtins-sub.nix
index 68b1913a00..2929c4dddd 100644
--- a/tvix/eval/src/tests/tvix_tests/eval-okay-builtins-sub.nix
+++ b/tvix/eval/src/tests/tvix_tests/eval-okay-builtins-sub.nix
@@ -4,4 +4,5 @@
   (builtins.sub 7 11.9)
   (builtins.sub 7.2 11.9)
   (builtins.sub 7.9 11.9)
+  (builtins.sub (builtins.sub 123 23) 58)
 ]
diff --git a/tvix/eval/src/vm.rs b/tvix/eval/src/vm.rs
index 52627a1fea..d6a24ebf86 100644
--- a/tvix/eval/src/vm.rs
+++ b/tvix/eval/src/vm.rs
@@ -71,7 +71,7 @@ macro_rules! arithmetic_op {
     ( $self:ident, $op:tt ) => {{
         let b = $self.pop();
         let a = $self.pop();
-        let result = fallible!($self, arithmetic_op!(a, b, $op));
+        let result = fallible!($self, arithmetic_op!(&a, &b, $op));
         $self.push(result);
     }};
 
@@ -79,8 +79,8 @@ macro_rules! arithmetic_op {
         match ($a, $b) {
             (Value::Integer(i1), Value::Integer(i2)) => Ok(Value::Integer(i1 $op i2)),
             (Value::Float(f1), Value::Float(f2)) => Ok(Value::Float(f1 $op f2)),
-            (Value::Integer(i1), Value::Float(f2)) => Ok(Value::Float(i1 as f64 $op f2)),
-            (Value::Float(f1), Value::Integer(i2)) => Ok(Value::Float(f1 $op i2 as f64)),
+            (Value::Integer(i1), Value::Float(f2)) => Ok(Value::Float(*i1 as f64 $op f2)),
+            (Value::Float(f1), Value::Integer(i2)) => Ok(Value::Float(f1 $op *i2 as f64)),
 
             (v1, v2) => Err(ErrorKind::TypeError {
                 expected: "number (either int or float)",
@@ -264,7 +264,7 @@ impl<'o> VM<'o> {
                     let result = if let (Value::String(s1), Value::String(s2)) = (&a, &b) {
                         Value::String(s1.concat(s2))
                     } else {
-                        fallible!(self, arithmetic_op!(a, b, +))
+                        fallible!(self, arithmetic_op!(&a, &b, +))
                     };
 
                     self.push(result)