fix(tvix/eval): thunk string interpolation r/4859

If we have multiple string parts, we need to thunk assembling the
string. If we have a single literal, it is strict (like all literals),
but a single interpolation part may compile to a thunk, depending on how
the expression inside is compiled – we can avoid forcing to early here
compared to the previous behavior.

Note that this CL retains the bug that `"${x}"` is erroneously
translated to `x`, implying e.g. `"${12}" == 12`.

The use of `parts.len()` is unproblematic, since normalized_parts()
builds a `Vec` instead of returning an iterator.

Change-Id: I3aecbfefef65cc627b1b8a65be27cbaeada3582b
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6580
Autosubmit: sterni <sternenseemann@systemli.org>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI

1 files changed, 32 insertions, 18 deletions

diff --git a/tvix/eval/src/compiler/mod.rs b/tvix/eval/src/compiler/mod.rs
index 3830c01712f2..a624cf11c74d 100644
--- a/tvix/eval/src/compiler/mod.rs
+++ b/tvix/eval/src/compiler/mod.rs
@@ -258,34 +258,48 @@ impl Compiler<'_, '_> {
     }
 
     fn compile_str(&mut self, slot: LocalIdx, node: ast::Str) {
-        // TODO: thunk string construction if it is not a literal
-        let mut count = 0;
+        let parts = node.normalized_parts();
+        let count = parts.len();
 
-        // The string parts are produced in literal order, however
-        // they need to be reversed on the stack in order to
-        // efficiently create the real string in case of
-        // interpolation.
-        for part in node.normalized_parts().into_iter().rev() {
-            count += 1;
+        if count != 1 {
+            self.thunk(slot, &node, |c, n, s| {
+                // The string parts are produced in literal order, however
+                // they need to be reversed on the stack in order to
+                // efficiently create the real string in case of
+                // interpolation.
+                for part in parts.into_iter().rev() {
+                    match part {
+                        // Interpolated expressions are compiled as normal and
+                        // dealt with by the VM before being assembled into
+                        // the final string. We need to force them here,
+                        // so OpInterpolate definitely has a string to consume.
+                        // TODO(sterni): coerce to string
+                        ast::InterpolPart::Interpolation(ipol) => {
+                            c.compile(s, ipol.expr().unwrap());
+                            c.emit_force(&ipol);
+                        }
+
+                        ast::InterpolPart::Literal(lit) => {
+                            c.emit_constant(Value::String(lit.into()), n);
+                        }
+                    }
+                }
 
-            match part {
-                // Interpolated expressions are compiled as normal and
-                // dealt with by the VM before being assembled into
-                // the final string.
+                c.push_op(OpCode::OpInterpolate(Count(count)), n);
+            });
+        } else {
+            match &parts[0] {
+                // Since we only have a single part, it is okay if this yields a thunk
+                // TODO(sterni): coerce to string
                 ast::InterpolPart::Interpolation(node) => {
                     self.compile(slot, node.expr().unwrap());
-                    self.emit_force(&node);
                 }
 
                 ast::InterpolPart::Literal(lit) => {
-                    self.emit_constant(Value::String(lit.into()), &node);
+                    self.emit_constant(Value::String(lit.as_str().into()), &node);
                 }
             }
         }
-
-        if count != 1 {
-            self.push_op(OpCode::OpInterpolate(Count(count)), &node);
-        }
     }
 
     fn compile_unary_op(&mut self, slot: LocalIdx, op: ast::UnaryOp) {