From bcd7e520f076fb7a6b26805663fac2d70c677bc8 Mon Sep 17 00:00:00 2001
From: sterni <sternenseemann@systemli.org>
Date: Wed, 14 Sep 2022 15:35:28 +0200
Subject: fix(tvix/eval): thunk string interpolation
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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
---
 tvix/eval/src/compiler/mod.rs                      | 50 ++++++++++++++--------
 .../eval-okay-thunked-string-interpolation.exp     |  1 +
 .../eval-okay-thunked-string-interpolation.nix     |  7 +++
 3 files changed, 40 insertions(+), 18 deletions(-)
 create mode 100644 tvix/eval/src/tests/tvix_tests/eval-okay-thunked-string-interpolation.exp
 create mode 100644 tvix/eval/src/tests/tvix_tests/eval-okay-thunked-string-interpolation.nix

(limited to 'tvix')

diff --git a/tvix/eval/src/compiler/mod.rs b/tvix/eval/src/compiler/mod.rs
index 3830c01712..a624cf11c7 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) {
diff --git a/tvix/eval/src/tests/tvix_tests/eval-okay-thunked-string-interpolation.exp b/tvix/eval/src/tests/tvix_tests/eval-okay-thunked-string-interpolation.exp
new file mode 100644
index 0000000000..fc2f21e930
--- /dev/null
+++ b/tvix/eval/src/tests/tvix_tests/eval-okay-thunked-string-interpolation.exp
@@ -0,0 +1 @@
+"strict literal"
diff --git a/tvix/eval/src/tests/tvix_tests/eval-okay-thunked-string-interpolation.nix b/tvix/eval/src/tests/tvix_tests/eval-okay-thunked-string-interpolation.nix
new file mode 100644
index 0000000000..bd3555bb24
--- /dev/null
+++ b/tvix/eval/src/tests/tvix_tests/eval-okay-thunked-string-interpolation.nix
@@ -0,0 +1,7 @@
+let
+  final = { text = "strict literal"; inherit x y; };
+  x = "lazy ${throw "interpolation"}";
+  y = "${throw "also lazy!"}";
+in
+
+final.text
-- 
cgit 1.4.1