feat(tvix/eval): implement correct toString behavior r/4857

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 <sternenseemann@systemli.org>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI

6 files changed, 196 insertions, 14 deletions

diff --git a/tvix/eval/src/builtins/mod.rs b/tvix/eval/src/builtins/mod.rs
index 0fadb738d5..74215cd37c 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> {
             ));
         }),
         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 39b105de98..086cc9d905 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<Error>),
 
+    /// 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 0dda9a6827..a148ebc3b5 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 05b2917a0f..e4dc18ac96 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 6baa8b666e..f8fb9c7b40 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<NixString, ErrorKind> {
+        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 d886bf24b9..6fe94184b8 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);