fix(tvix/eval/value): correctly emit spaces when coercing lists r/7264

r/7176 introduced an incorrect assumption was the benefit of the
nonrecursive coercion algorithm, namely that a coercion operation always
returns a non empty string. This allows to detect whether we are
coercing a list or not by checking if the intermediate result is empty
or not. Unfortunately, coercing null and false yields an empty string,
so we need to explicitly track whether we are coercing a list.

Updated the test case to hopefully catch similar bugs in the future. I'm
not a hundred percent certain I have not introduced a new edge case with
this, so it may be interesting to add a prop test case for this to
nix_oracle down the line. At least lists are the only nested data
structures that can be serialized as nested data structures, so the
problem is kind of limited.

Change-Id: Ia41e904356f1c41a9d35e4e65ec02f2fe5a4100e
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10418
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Autosubmit: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI

3 files changed, 36 insertions, 16 deletions

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 a148ebc3b5..cd5a6c0d54 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" "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" ]
+[ "" " " " /deep/thought" " 2  3" " flat" "1" "4.200000" "" "" "1" "foo" "/etc" "Hello World" "Hello World" "1" "out" "2" "    /deep/thought  2  3  flat 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 e4dc18ac96..eb8011158f 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
@@ -5,19 +5,24 @@ let
   };
 
   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; }; }; })
+    null
+    [ null false ]
+    [ null /deep/thought ]
+    [ [ null 2 ] null 3 ]
+    [ false "flat" ]
+    1
+    4.2
+    null
+    false
+    true
+    "foo"
+    /etc
+    toStringableSet
+    { __toString = _: toStringableSet; }
+    { __toString = _: true; }
+    { outPath = "out"; }
+    { outPath = { outPath = { __toString = _: 2; }; }; }
   ];
 in
 
-toStringExamples ++ [ (toString toStringExamples) ]
+(builtins.map toString toStringExamples) ++ [ (toString toStringExamples) ]
diff --git a/tvix/eval/src/value/mod.rs b/tvix/eval/src/value/mod.rs
index 0007735cc0..567a2f3df2 100644
--- a/tvix/eval/src/value/mod.rs
+++ b/tvix/eval/src/value/mod.rs
@@ -315,6 +315,9 @@ impl Value {
     ) -> Result<Value, ErrorKind> {
         let mut result = String::new();
         let mut vals = vec![self];
+        // Track if we are coercing the first value of a list to correctly emit
+        // separating white spaces.
+        let mut is_list_head = None;
         loop {
             let value = if let Some(v) = vals.pop() {
                 v.force(co, span.clone()).await?
@@ -392,6 +395,12 @@ impl Value {
                     for elem in list.into_iter().rev() {
                         vals.push(elem);
                     }
+                    // In case we are coercing a list within a list we don't want
+                    // to touch this. Since the algorithm is nonrecursive, the
+                    // space would not have been created yet (due to continue).
+                    if is_list_head.is_none() {
+                        is_list_head = Some(true);
+                    }
                     continue;
                 }
 
@@ -419,9 +428,15 @@ impl Value {
                     panic!("tvix bug: .coerce_to_string() called on internal value")
                 }
             };
-            if !result.is_empty() {
-                result.push(' ');
+
+            if let Some(head) = is_list_head {
+                if !head {
+                    result.push(' ');
+                } else {
+                    is_list_head = Some(false);
+                }
             }
+
             result.push_str(&coerced?);
         }
     }