feat(tvix/eval): nonrecursive coerce_to_string() r/7176

After this commit, the only non-builtins uses of generators are:

  - coerce_to_string() uses generators::request_enter_lambda()
  - Thunk::force() uses generators::request_enter_lambda()

That's it!  Once those two are taken care of, GenCo can become an
implementation detail of `builtins::BuiltinGen`.  No more crazy
nonlocal flow control within the interpreter: if you've got a GenCo
floating around in your code it's because you're writing a builtin,
which isn't part of the core interpreter.  The interpreter won't
need GenCos to talk to itself anymore.

Technically generators::request_path_import() is also used by
coerce_to_string(), but that's just because the io_handle happens to
be part of the VM.  There's no recursion-depth issue there, so the
call doesn't need to go through the generator mechanism
(request_path_import() doesn't call back to the interpreter!)

Change-Id: I83ce5774d49b88fdafdd61160975b4937a435bb0
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10256
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
Autosubmit: Adam Joseph <adam@westernsemico.com>

1 files changed, 103 insertions, 83 deletions

diff --git a/tvix/eval/src/value/mod.rs b/tvix/eval/src/value/mod.rs
index 596dddba520b..003567bfd440 100644
--- a/tvix/eval/src/value/mod.rs
+++ b/tvix/eval/src/value/mod.rs
@@ -289,102 +289,122 @@ impl Value {
         }
     }
 
-    // TODO(amjoseph): de-asyncify this (when called directly by the VM)
+    pub async fn coerce_to_string(
+        self,
+        co: GenCo,
+        kind: CoercionKind,
+        span: LightSpan,
+    ) -> Result<Value, ErrorKind> {
+        self.coerce_to_string_(&co, kind, span).await
+    }
+
     /// Coerce a `Value` to a string. See `CoercionKind` for a rundown of what
     /// input types are accepted under what circumstances.
-    pub async fn coerce_to_string(self, co: GenCo, kind: CoercionKind) -> Result<Value, ErrorKind> {
-        let value = generators::request_force(&co, self).await;
-
-        match (value, kind) {
-            // coercions that are always done
-            tuple @ (Value::String(_), _) => Ok(tuple.0),
-
-            // 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), _) => {
-                // TODO(tazjin): there are cases where coerce_to_string does not import
-                let imported = generators::request_path_import(&co, *p).await;
-                Ok(imported.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), kind) => {
-                if let Some(s) = attrs.try_to_string(&co, kind).await {
-                    return Ok(Value::String(s));
+    pub async fn coerce_to_string_(
+        self,
+        co: &GenCo,
+        kind: CoercionKind,
+        span: LightSpan,
+    ) -> Result<Value, ErrorKind> {
+        let mut result = String::new();
+        let mut vals = vec![self];
+        loop {
+            let value = if let Some(v) = vals.pop() {
+                v.force(co, span.clone()).await?
+            } else {
+                return Ok(Value::String(result.into()));
+            };
+            let coerced = match (value, kind) {
+                // coercions that are always done
+                (Value::String(s), _) => Ok(s.as_str().to_owned()),
+
+                // 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), _) => {
+                    // TODO(tazjin): there are cases where coerce_to_string does not import
+                    let imported = generators::request_path_import(co, *p).await;
+                    Ok(imported.to_string_lossy().into_owned())
                 }
 
-                if let Some(out_path) = attrs.select("outPath") {
-                    return match generators::request_string_coerce(&co, out_path.clone(), kind)
-                        .await
-                    {
-                        Ok(s) => Ok(Value::String(s)),
-                        Err(c) => Ok(Value::Catchable(c)),
-                    };
+                // 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), kind) => {
+                    if let Some(to_string) = attrs.select("__toString") {
+                        let callable = to_string.clone().force(co, span.clone()).await?;
+
+                        // Leave the attribute set on the stack as an argument
+                        // to the function call.
+                        generators::request_stack_push(co, Value::Attrs(attrs.clone())).await;
+
+                        // Call the callable ...
+                        let result = generators::request_call(co, callable).await;
+
+                        // Recurse on the result, as attribute set coercion
+                        // actually works recursively, e.g. you can even return
+                        // /another/ set with a __toString attr.
+                        vals.push(result);
+                        continue;
+                    } else if let Some(out_path) = attrs.select("outPath") {
+                        vals.push(out_path.clone());
+                        continue;
+                    } else {
+                        return Err(ErrorKind::NotCoercibleToString { from: "set", kind });
+                    }
                 }
 
-                Err(ErrorKind::NotCoercibleToString { from: "set", kind })
-            }
-
-            // 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(list), CoercionKind::Strong) => {
-                let mut out = String::new();
+                // strong coercions
+                (Value::Null, CoercionKind::Strong)
+                | (Value::Bool(false), CoercionKind::Strong) => Ok("".to_owned()),
+                (Value::Bool(true), CoercionKind::Strong) => Ok("1".to_owned()),
 
-                for (idx, elem) in list.into_iter().enumerate() {
-                    if idx > 0 {
-                        out.push(' ');
-                    }
+                (Value::Integer(i), CoercionKind::Strong) => Ok(format!("{i}")),
+                (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))
+                }
 
-                    match generators::request_string_coerce(&co, elem, kind).await {
-                        Ok(s) => out.push_str(s.as_str()),
-                        Err(c) => return Ok(Value::Catchable(c)),
+                // Lists are coerced by coercing their elements and interspersing spaces
+                (Value::List(list), CoercionKind::Strong) => {
+                    for elem in list.into_iter().rev() {
+                        vals.push(elem);
                     }
+                    continue;
                 }
 
-                Ok(Value::String(out.into()))
-            }
+                (Value::Thunk(_), _) => panic!("Tvix bug: force returned unforced thunk"),
+
+                val @ (Value::Closure(_), _)
+                | val @ (Value::Builtin(_), _)
+                | val @ (Value::Null, _)
+                | val @ (Value::Bool(_), _)
+                | val @ (Value::Integer(_), _)
+                | val @ (Value::Float(_), _)
+                | val @ (Value::List(_), _) => Err(ErrorKind::NotCoercibleToString {
+                    from: val.0.type_of(),
+                    kind,
+                }),
 
-            (Value::Thunk(_), _) => panic!("Tvix bug: force returned unforced thunk"),
-
-            val @ (Value::Closure(_), _)
-            | val @ (Value::Builtin(_), _)
-            | val @ (Value::Null, _)
-            | val @ (Value::Bool(_), _)
-            | val @ (Value::Integer(_), _)
-            | val @ (Value::Float(_), _)
-            | val @ (Value::List(_), _) => Err(ErrorKind::NotCoercibleToString {
-                from: val.0.type_of(),
-                kind,
-            }),
-
-            (c @ Value::Catchable(_), _) => Ok(c),
-
-            (Value::AttrNotFound, _)
-            | (Value::Blueprint(_), _)
-            | (Value::DeferredUpvalue(_), _)
-            | (Value::UnresolvedPath(_), _)
-            | (Value::Json(_), _)
-            | (Value::FinaliseRequest(_), _) => {
-                panic!("tvix bug: .coerce_to_string() called on internal value")
+                (c @ Value::Catchable(_), _) => return Ok(c),
+
+                (Value::AttrNotFound, _)
+                | (Value::Blueprint(_), _)
+                | (Value::DeferredUpvalue(_), _)
+                | (Value::UnresolvedPath(_), _)
+                | (Value::Json(_), _)
+                | (Value::FinaliseRequest(_), _) => {
+                    panic!("tvix bug: .coerce_to_string() called on internal value")
+                }
+            };
+            if !result.is_empty() {
+                result.push(' ');
             }
+            result.push_str(&coerced?);
         }
     }