From afcfa7836401d90bbb131b170a3a69b3cf794b1c Mon Sep 17 00:00:00 2001
From: Vincent Ambo <mail@tazj.in>
Date: Wed, 31 Aug 2022 04:47:54 +0300
Subject: [PATCH] feat(tvix/eval): semi-strictly evaluate output values of the
 VM

This essentially makes the VM behave like `nix-instantiate --eval
--strict`, i.e. data structures are traversed strictly and thunks are
forced. Thunks embedded in closures are not forced.

This allows us to re-enable tests that were disabled because they
needed to output nested thunk contents, but is overall a behaviour
that must be configurable later on, as it is not cmopatible with e.g.
an evaluation of nixpkgs.

Change-Id: I5303a5c8e4322feab1384fdb7712fecb950afca5
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6372
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
---
 .../disabled-identity-nested-attrs.nix        |  3 --
 .../tvix_tests/identity-nested-attrs.nix      |  1 +
 tvix/eval/src/vm.rs                           | 36 ++++++++++++++++++-
 3 files changed, 36 insertions(+), 4 deletions(-)
 delete mode 100644 tvix/eval/src/tests/tvix_tests/disabled-identity-nested-attrs.nix
 create mode 100644 tvix/eval/src/tests/tvix_tests/identity-nested-attrs.nix

diff --git a/tvix/eval/src/tests/tvix_tests/disabled-identity-nested-attrs.nix b/tvix/eval/src/tests/tvix_tests/disabled-identity-nested-attrs.nix
deleted file mode 100644
index f8dacf3e0..000000000
--- a/tvix/eval/src/tests/tvix_tests/disabled-identity-nested-attrs.nix
+++ /dev/null
@@ -1,3 +0,0 @@
-# TODO: temporarily disabled because need "strict output" (b is
-# thunked)
-{ a = { b = null; }; }
diff --git a/tvix/eval/src/tests/tvix_tests/identity-nested-attrs.nix b/tvix/eval/src/tests/tvix_tests/identity-nested-attrs.nix
new file mode 100644
index 000000000..6a139452e
--- /dev/null
+++ b/tvix/eval/src/tests/tvix_tests/identity-nested-attrs.nix
@@ -0,0 +1 @@
+{ a = { b = null; }; }
diff --git a/tvix/eval/src/vm.rs b/tvix/eval/src/vm.rs
index d85f31a3c..091a4ebb3 100644
--- a/tvix/eval/src/vm.rs
+++ b/tvix/eval/src/vm.rs
@@ -602,6 +602,38 @@ impl VM {
 
         Ok(())
     }
+
+    /// Strictly evaluate the supplied value for outputting it. This
+    /// will ensure that lists and attribute sets do not contain
+    /// chunks which, for users, are displayed in a strange and often
+    /// unexpected way.
+    fn force_for_output(&mut self, value: &Value) -> EvalResult<()> {
+        match value {
+            Value::Attrs(attrs) => {
+                for (_, value) in attrs.iter() {
+                    self.force_for_output(value)?;
+                }
+                Ok(())
+            }
+
+            Value::List(list) => list.iter().try_for_each(|elem| self.force_for_output(elem)),
+
+            Value::Thunk(thunk) => {
+                thunk.force(self)?;
+                self.force_for_output(&thunk.value())
+            }
+
+            // If any of these internal values are encountered here a
+            // critical error has happened (likely a compiler bug).
+            Value::AttrPath(_)
+            | Value::AttrNotFound
+            | Value::DynamicUpvalueMissing(_)
+            | Value::Blueprint(_)
+            | Value::DeferredUpvalue(_) => panic!("tvix bug: internal value left on stack"),
+
+            _ => Ok(()),
+        }
+    }
 }
 
 // TODO: use Rc::unwrap_or_clone once it is stabilised.
@@ -618,5 +650,7 @@ pub fn run_lambda(lambda: Lambda) -> EvalResult<Value> {
     };
 
     vm.call(Rc::new(lambda), vec![], 0);
-    vm.run()
+    let value = vm.run()?;
+    vm.force_for_output(&value)?;
+    Ok(value)
 }