From 64d3efcc2ce055ffe45034ed169569ece961f04d Mon Sep 17 00:00:00 2001 From: sterni Date: Wed, 21 Sep 2022 22:20:37 +0200 Subject: fix(tvix/eval): handle thunks in arithmetic builtins 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 Reviewed-by: tazjin --- tvix/eval/src/builtins/mod.rs | 40 +++++++++++----------- .../tests/tvix_tests/eval-okay-builtins-add.exp | 2 +- .../tests/tvix_tests/eval-okay-builtins-add.nix | 1 + .../tests/tvix_tests/eval-okay-builtins-div.exp | 2 +- .../tests/tvix_tests/eval-okay-builtins-div.nix | 1 + .../tests/tvix_tests/eval-okay-builtins-mul.exp | 2 +- .../tests/tvix_tests/eval-okay-builtins-mul.nix | 1 + .../tests/tvix_tests/eval-okay-builtins-sub.exp | 2 +- .../tests/tvix_tests/eval-okay-builtins-sub.nix | 1 + tvix/eval/src/vm.rs | 8 ++--- 10 files changed, 32 insertions(+), 28 deletions(-) diff --git a/tvix/eval/src/builtins/mod.rs b/tvix/eval/src/builtins/mod.rs index 650cfe674f52..1a6155a82352 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 Vec { 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 { 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 { 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 { .collect::>(); 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 25c0a9cc0fba..c3ac813de6b9 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 6b2a29d7982c..b04b1d1fa6ba 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 d71bbb7a060b..73e9bc33b083 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 622ce889daa9..98b8b74bdf2b 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 5ed8b8e2bd83..e3e0f03a8af5 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 5b61e387a622..2a8d6c4214c3 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 c47a17fa2709..51842eccfac5 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 68b1913a0097..2929c4dddd81 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 52627a1fea14..d6a24ebf86d4 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) -- cgit 1.4.1