From da1d71a4e8e3afee09d8f72b79b6042d4ccbfc2d Mon Sep 17 00:00:00 2001 From: sterni Date: Tue, 13 Sep 2022 15:37:19 +0200 Subject: feat(tvix/eval): implement correct toString behavior Implement C++ Nix's `EvalState::coerceToString` minus some of the Path / store handling. This is currently only used for `toString` which does all possible coercions, but we've already prepared the weaker coercion variant which is e.g. used for builtins that expect string arguments. `EvalState::coerceToPath` is still missing for builtins that need a path, but it'll be easy to build on top of this. Change-Id: I78d15576b18921791d04b6b1e964b951fdef22c6 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6571 Autosubmit: sterni Reviewed-by: tazjin Tested-by: BuildkiteCI --- tvix/eval/src/builtins/mod.rs | 8 +- tvix/eval/src/errors.rs | 23 ++++ .../tvix_tests/eval-okay-builtins-toString.exp | 2 +- .../tvix_tests/eval-okay-builtins-toString.nix | 29 ++++- tvix/eval/src/value/mod.rs | 142 +++++++++++++++++++++ tvix/eval/src/vm.rs | 6 +- 6 files changed, 196 insertions(+), 14 deletions(-) (limited to 'tvix') diff --git a/tvix/eval/src/builtins/mod.rs b/tvix/eval/src/builtins/mod.rs index 0fadb738d529..74215cd37c92 100644 --- a/tvix/eval/src/builtins/mod.rs +++ b/tvix/eval/src/builtins/mod.rs @@ -10,7 +10,7 @@ use std::{ use crate::{ errors::ErrorKind, - value::{Builtin, NixAttrs, NixList, NixString, Value}, + value::{Builtin, CoercionKind, NixAttrs, NixList, NixString, Value}, }; use crate::arithmetic_op; @@ -121,9 +121,9 @@ fn pure_builtins() -> Vec { )); }), Builtin::new("toString", 1, |args, vm| { - force!(vm, &args[0], value, { - Ok(Value::String(format!("{}", value).into())) - }) + args[0] + .coerce_to_string(CoercionKind::Strong, vm) + .map(|s| Value::String(s)) }), Builtin::new("typeOf", 1, |args, vm| { force!(vm, &args[0], value, { diff --git a/tvix/eval/src/errors.rs b/tvix/eval/src/errors.rs index 39b105de9812..086cc9d9052c 100644 --- a/tvix/eval/src/errors.rs +++ b/tvix/eval/src/errors.rs @@ -1,3 +1,4 @@ +use crate::value::CoercionKind; use std::fmt::Display; use codemap::{CodeMap, Span}; @@ -61,6 +62,12 @@ pub enum ErrorKind { /// chained up. ThunkForce(Box), + /// Given type can't be coerced to a string in the respective context + NotCoercibleToString { + from: &'static str, + kind: CoercionKind, + }, + /// Tvix internal warning for features triggered by users that are /// not actually implemented yet, and without which eval can not /// proceed. @@ -158,6 +165,21 @@ to a missing value in the attribute set(s) included via `with`."#, // delegating to the inner error ErrorKind::ThunkForce(err) => err.message(codemap), + ErrorKind::NotCoercibleToString { kind, from } => { + let kindly = match kind { + CoercionKind::Strong => "strongly", + CoercionKind::Weak => "weakly", + }; + + let hint = if *from == "set" { + ", missing a `__toString` or `outPath` attribute" + } else { + "" + }; + + format!("cannot ({kindly}) coerce {from} to a string{hint}") + } + ErrorKind::NotImplemented(feature) => { format!("feature not yet implemented in Tvix: {}", feature) } @@ -185,6 +207,7 @@ to a missing value in the attribute set(s) included via `with`."#, ErrorKind::ParseErrors(_) => "E015", ErrorKind::DuplicateAttrsKey { .. } => "E016", ErrorKind::ThunkForce(_) => "E017", + ErrorKind::NotCoercibleToString { .. } => "E018", ErrorKind::NotImplemented(_) => "E999", } } diff --git a/tvix/eval/src/tests/tvix_tests/eval-okay-builtins-toString.exp b/tvix/eval/src/tests/tvix_tests/eval-okay-builtins-toString.exp index 0dda9a682704..a148ebc3b53f 100644 --- a/tvix/eval/src/tests/tvix_tests/eval-okay-builtins-toString.exp +++ b/tvix/eval/src/tests/tvix_tests/eval-okay-builtins-toString.exp @@ -1 +1 @@ -[ "1" ] +[ "1" "4.200000" "" "" "1" "foo" "/etc" "Hello World" "Hello World" "1" "out" "2" "1 4.200000 1 foo /etc Hello World Hello World 1 out 2" ] diff --git a/tvix/eval/src/tests/tvix_tests/eval-okay-builtins-toString.nix b/tvix/eval/src/tests/tvix_tests/eval-okay-builtins-toString.nix index 05b2917a0fb3..e4dc18ac96a7 100644 --- a/tvix/eval/src/tests/tvix_tests/eval-okay-builtins-toString.nix +++ b/tvix/eval/src/tests/tvix_tests/eval-okay-builtins-toString.nix @@ -1,6 +1,23 @@ -# TODO: add some examples for the "weird" types -[ - (toString 1) - # TODO: floats must be padded to 6 digits - # (toString 4.2) -] +let + toStringableSet = { + __toString = self: self.content; + content = "Hello World"; + }; + + toStringExamples = [ + (toString 1) + (toString 4.2) + (toString null) + (toString false) + (toString true) + (toString "foo") + (toString /etc) + (toString toStringableSet) + (toString { __toString = _: toStringableSet; }) + (toString { __toString = _: true; }) + (toString { outPath = "out"; }) + (toString { outPath = { outPath = { __toString = _: 2; }; }; }) + ]; +in + +toStringExamples ++ [ (toString toStringExamples) ] diff --git a/tvix/eval/src/value/mod.rs b/tvix/eval/src/value/mod.rs index 6baa8b666ed6..f8fb9c7b40b7 100644 --- a/tvix/eval/src/value/mod.rs +++ b/tvix/eval/src/value/mod.rs @@ -12,6 +12,8 @@ mod thunk; use crate::errors::ErrorKind; use crate::opcode::StackIdx; +use crate::upvalues::UpvalueCarrier; +use crate::vm::VM; pub use attrs::NixAttrs; pub use builtin::Builtin; pub use function::{Closure, Lambda}; @@ -78,7 +80,147 @@ macro_rules! gen_is { }; } +/// Describes what input types are allowed when coercing a `Value` to a string +#[derive(Clone, Copy, Debug)] +pub enum CoercionKind { + /// Only coerce already "stringly" types like strings and paths, but also + /// coerce sets that have a `__toString` attribute. Equivalent to + /// `!coerceMore` in C++ Nix. + Weak, + /// Coerce all value types included by `Weak`, but also coerce `null`, + /// booleans, integers, floats and lists of coercible types. Equivalent to + /// `coerceMore` in C++ Nix. + Strong, +} + impl Value { + /// Coerce a `Value` to a string. See `CoercionKind` for a rundown of what + /// input types are accepted under what circumstances. + pub fn coerce_to_string( + &self, + kind: CoercionKind, + vm: &mut VM, + ) -> Result { + if let Value::Thunk(t) = self { + t.force(vm)?; + } + + match (self, kind) { + // deal with thunks + (Value::Thunk(t), _) => t.value().coerce_to_string(kind, vm), + + // coercions that are always done + (Value::String(s), _) => Ok(s.clone()), + // TODO(sterni): Think about proper encoding handling here. This needs + // general consideration anyways, since one current discrepancy between + // C++ Nix and Tvix is that the former's strings are arbitrary byte + // sequences without NUL bytes, whereas Tvix only allows valid + // Unicode. See also b/189. + (Value::Path(p), _) => Ok(p.to_string_lossy().into_owned().into()), + + // Attribute sets can be converted to strings if they either have an + // `__toString` attribute which holds a function that receives the + // set itself or an `outPath` attribute which should be a string. + // `__toString` is preferred. + (Value::Attrs(attrs), _) => { + match (attrs.select("__toString"), attrs.select("outPath")) { + (None, None) => Err(ErrorKind::NotCoercibleToString { + from: "set", + kind: kind, + }), + + (Some(f), _) => { + // use a closure here to deal with the thunk borrow we need to do below + let call_to_string = |value: &Value, vm: &mut VM| { + // TODO(sterni): calling logic should be extracted into a helper + let result = match value { + Value::Closure(c) => { + vm.push(self.clone()); + vm.call(c.lambda(), c.upvalues().clone(), 1) + .map_err(|e| e.kind) + } + + Value::Builtin(b) => { + vm.push(self.clone()); + vm.call_builtin(b.clone()).map_err(|e| e.kind)?; + Ok(vm.pop()) + } + + _ => Err(ErrorKind::NotCallable), + }?; + + match result { + Value::String(s) => Ok(s), + // Attribute set coercion actually works + // recursively, e.g. you can even return + // /another/ set with a __toString attr. + _ => result.coerce_to_string(kind, vm), + } + }; + + if let Value::Thunk(t) = f { + t.force(vm)?; + let guard = t.value(); + call_to_string(&*guard, vm) + } else { + call_to_string(&f, vm) + } + } + + // Similarly to `__toString` we also coerce recursively for `outPath` + (None, Some(s)) => s.coerce_to_string(kind, vm), + } + } + + // strong coercions + (Value::Null, CoercionKind::Strong) | (Value::Bool(false), CoercionKind::Strong) => { + Ok("".into()) + } + (Value::Bool(true), CoercionKind::Strong) => Ok("1".into()), + + (Value::Integer(i), CoercionKind::Strong) => Ok(format!("{i}").into()), + (Value::Float(f), CoercionKind::Strong) => { + // contrary to normal Display, coercing a float to a string will + // result in unconditional 6 decimal places + Ok(format!("{:.6}", f).into()) + } + + // Lists are coerced by coercing their elements and interspersing spaces + (Value::List(l), CoercionKind::Strong) => { + // TODO(sterni): use intersperse when it becomes available? + // https://github.com/rust-lang/rust/issues/79524 + l.iter() + .map(|v| v.coerce_to_string(kind, vm)) + .reduce(|acc, string| { + let a = acc?; + let s = &string?; + Ok(a.concat(&" ".into()).concat(s)) + }) + // None from reduce indicates empty iterator + .unwrap_or(Ok("".into())) + } + + (Value::Closure(_), _) + | (Value::Builtin(_), _) + | (Value::Null, _) + | (Value::Bool(_), _) + | (Value::Integer(_), _) + | (Value::Float(_), _) + | (Value::List(_), _) => Err(ErrorKind::NotCoercibleToString { + from: self.type_of(), + kind: kind, + }), + + (Value::AttrPath(_), _) + | (Value::AttrNotFound, _) + | (Value::DynamicUpvalueMissing(_), _) + | (Value::Blueprint(_), _) + | (Value::DeferredUpvalue(_), _) => { + panic!("tvix bug: .coerce_to_string() called on internal value") + } + } + } + pub fn type_of(&self) -> &'static str { match self { Value::Null => "null", diff --git a/tvix/eval/src/vm.rs b/tvix/eval/src/vm.rs index d886bf24b939..6fe94184b8a2 100644 --- a/tvix/eval/src/vm.rs +++ b/tvix/eval/src/vm.rs @@ -148,11 +148,11 @@ impl<'o> VM<'o> { op } - fn pop(&mut self) -> Value { + pub fn pop(&mut self) -> Value { self.stack.pop().expect("runtime stack empty") } - fn push(&mut self, value: Value) { + pub fn push(&mut self, value: Value) { self.stack.push(value) } @@ -726,7 +726,7 @@ impl<'o> VM<'o> { } } - fn call_builtin(&mut self, builtin: Builtin) -> EvalResult<()> { + pub fn call_builtin(&mut self, builtin: Builtin) -> EvalResult<()> { let builtin_name = builtin.name(); self.observer.observe_enter_builtin(builtin_name); -- cgit 1.4.1