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 <wpcarro@gmail.com> Reviewed-by: tazjin <tazjin@tvl.su>
This commit is contained in:
		
							parent
							
								
									55459f02fc
								
							
						
					
					
						commit
						64d3efcc2c
					
				
					 10 changed files with 32 additions and 28 deletions
				
			
		|  | @ -52,11 +52,11 @@ pub fn coerce_value_to_path(v: &Value, vm: &mut VM) -> Result<PathBuf, ErrorKind | ||||||
| /// WASM).
 | /// WASM).
 | ||||||
| fn pure_builtins() -> Vec<Builtin> { | fn pure_builtins() -> Vec<Builtin> { | ||||||
|     vec![ |     vec![ | ||||||
|         Builtin::new("add", &[true, true], |mut args, _| { |         Builtin::new( | ||||||
|             let b = args.pop().unwrap(); |             "add", | ||||||
|             let a = args.pop().unwrap(); |             &[false, false], | ||||||
|             arithmetic_op!(a, b, +) |             |args, vm| arithmetic_op!(&*args[0].force(vm)?, &*args[1].force(vm)?, +), | ||||||
|         }), |         ), | ||||||
|         Builtin::new("abort", &[true], |args, _| { |         Builtin::new("abort", &[true], |args, _| { | ||||||
|             return Err(ErrorKind::Abort(args[0].to_str()?.to_string())); |             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)), |                 std::cmp::Ordering::Greater => Ok(Value::Integer(1)), | ||||||
|             } |             } | ||||||
|         }), |         }), | ||||||
|         Builtin::new("div", &[true, true], |mut args, _| { |         Builtin::new( | ||||||
|             let b = args.pop().unwrap(); |             "div", | ||||||
|             let a = args.pop().unwrap(); |             &[false, false], | ||||||
|             arithmetic_op!(a, b, /) |             |args, vm| arithmetic_op!(&*args[0].force(vm)?, &*args[1].force(vm)?, /), | ||||||
|         }), |         ), | ||||||
|         Builtin::new("elemAt", &[true, true], |args, _| { |         Builtin::new("elemAt", &[true, true], |args, _| { | ||||||
|             let xs = args[0].to_list()?; |             let xs = args[0].to_list()?; | ||||||
|             let i = args[1].as_int()?; |             let i = args[1].as_int()?; | ||||||
|  | @ -215,11 +215,11 @@ fn pure_builtins() -> Vec<Builtin> { | ||||||
|             let value = args[0].force(vm)?; |             let value = args[0].force(vm)?; | ||||||
|             Ok(Value::Bool(matches!(*value, Value::String(_)))) |             Ok(Value::Bool(matches!(*value, Value::String(_)))) | ||||||
|         }), |         }), | ||||||
|         Builtin::new("mul", &[true, true], |mut args, _| { |         Builtin::new( | ||||||
|             let b = args.pop().unwrap(); |             "mul", | ||||||
|             let a = args.pop().unwrap(); |             &[false, false], | ||||||
|             arithmetic_op!(a, b, *) |             |args, vm| arithmetic_op!(&*args[0].force(vm)?, &*args[1].force(vm)?, *), | ||||||
|         }), |         ), | ||||||
|         Builtin::new("splitVersion", &[true], |args, _| { |         Builtin::new("splitVersion", &[true], |args, _| { | ||||||
|             let s = args[0].to_str()?; |             let s = args[0].to_str()?; | ||||||
|             let s = VersionPartsIter::new(s.as_str()); |             let s = VersionPartsIter::new(s.as_str()); | ||||||
|  | @ -234,11 +234,11 @@ fn pure_builtins() -> Vec<Builtin> { | ||||||
|                 .collect::<Vec<Value>>(); |                 .collect::<Vec<Value>>(); | ||||||
|             Ok(Value::List(NixList::construct(parts.len(), parts))) |             Ok(Value::List(NixList::construct(parts.len(), parts))) | ||||||
|         }), |         }), | ||||||
|         Builtin::new("sub", &[true, true], |mut args, _| { |         Builtin::new( | ||||||
|             let b = args.pop().unwrap(); |             "sub", | ||||||
|             let a = args.pop().unwrap(); |             &[false, false], | ||||||
|             arithmetic_op!(a, b, -) |             |args, vm| arithmetic_op!(&*args[0].force(vm)?, &*args[1].force(vm)?, -), | ||||||
|         }), |         ), | ||||||
|         Builtin::new("substring", &[true, true, true], |args, _| { |         Builtin::new("substring", &[true, true, true], |args, _| { | ||||||
|             let beg = args[0].as_int()?; |             let beg = args[0].as_int()?; | ||||||
|             let len = args[1].as_int()?; |             let len = args[1].as_int()?; | ||||||
|  |  | ||||||
|  | @ -1 +1 @@ | ||||||
| [ 18 18.9 18.9 19.1 19 ] | [ 18 18.9 18.9 19.1 19 42 ] | ||||||
|  |  | ||||||
|  | @ -4,4 +4,5 @@ | ||||||
|   (builtins.add 7 11.9) |   (builtins.add 7 11.9) | ||||||
|   (builtins.add 7.2 11.9) |   (builtins.add 7.2 11.9) | ||||||
|   (builtins.add 7.1 11.9) |   (builtins.add 7.1 11.9) | ||||||
|  |   (builtins.add (builtins.add 21 10) 11) | ||||||
| ] | ] | ||||||
|  |  | ||||||
|  | @ -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 ] | ||||||
|  |  | ||||||
|  | @ -7,4 +7,5 @@ | ||||||
|   (builtins.div 1.0 2) |   (builtins.div 1.0 2) | ||||||
|   (builtins.div 1 2.0) |   (builtins.div 1 2.0) | ||||||
|   (builtins.div 1.0 2.0) |   (builtins.div 1.0 2.0) | ||||||
|  |   (builtins.div (builtins.div 84 4) 0.5) | ||||||
| ] | ] | ||||||
|  |  | ||||||
|  | @ -1 +1 @@ | ||||||
| [ 36 0 0 14 ] | [ 36 0 0 14 42 ] | ||||||
|  |  | ||||||
|  | @ -3,4 +3,5 @@ | ||||||
|   (builtins.mul 0 7) |   (builtins.mul 0 7) | ||||||
|   (builtins.mul 7 0) |   (builtins.mul 7 0) | ||||||
|   (builtins.mul 7 2) |   (builtins.mul 7 2) | ||||||
|  |   (builtins.mul (builtins.mul 4 0.5) 21) | ||||||
| ] | ] | ||||||
|  |  | ||||||
|  | @ -1 +1 @@ | ||||||
| [ -4 -3.1 -4.9 -4.7 -4 ] | [ -4 -3.1 -4.9 -4.7 -4 42 ] | ||||||
|  |  | ||||||
|  | @ -4,4 +4,5 @@ | ||||||
|   (builtins.sub 7 11.9) |   (builtins.sub 7 11.9) | ||||||
|   (builtins.sub 7.2 11.9) |   (builtins.sub 7.2 11.9) | ||||||
|   (builtins.sub 7.9 11.9) |   (builtins.sub 7.9 11.9) | ||||||
|  |   (builtins.sub (builtins.sub 123 23) 58) | ||||||
| ] | ] | ||||||
|  |  | ||||||
|  | @ -71,7 +71,7 @@ macro_rules! arithmetic_op { | ||||||
|     ( $self:ident, $op:tt ) => {{ |     ( $self:ident, $op:tt ) => {{ | ||||||
|         let b = $self.pop(); |         let b = $self.pop(); | ||||||
|         let a = $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); |         $self.push(result); | ||||||
|     }}; |     }}; | ||||||
| 
 | 
 | ||||||
|  | @ -79,8 +79,8 @@ macro_rules! arithmetic_op { | ||||||
|         match ($a, $b) { |         match ($a, $b) { | ||||||
|             (Value::Integer(i1), Value::Integer(i2)) => Ok(Value::Integer(i1 $op i2)), |             (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::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::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::Float(f1), Value::Integer(i2)) => Ok(Value::Float(f1 $op *i2 as f64)), | ||||||
| 
 | 
 | ||||||
|             (v1, v2) => Err(ErrorKind::TypeError { |             (v1, v2) => Err(ErrorKind::TypeError { | ||||||
|                 expected: "number (either int or float)", |                 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) { |                     let result = if let (Value::String(s1), Value::String(s2)) = (&a, &b) { | ||||||
|                         Value::String(s1.concat(s2)) |                         Value::String(s1.concat(s2)) | ||||||
|                     } else { |                     } else { | ||||||
|                         fallible!(self, arithmetic_op!(a, b, +)) |                         fallible!(self, arithmetic_op!(&a, &b, +)) | ||||||
|                     }; |                     }; | ||||||
| 
 | 
 | ||||||
|                     self.push(result) |                     self.push(result) | ||||||
|  |  | ||||||
		Loading…
	
	Add table
		Add a link
		
	
		Reference in a new issue