refactor(tvix/eval): keep globals alive through VM struct r/5715

This forces users to pass the fully constructed set of globals to the
VM, making it harder to accidentally "lose" the set while weak
references to it still exist.

This doesn't modify any functionality, but is laying the foundation
for simplifying some of the builtins behaviour that has grown more
complex again.

Change-Id: I5120f97861c65dc46d90b8a4e2c92ad32cc53e03
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7877
Autosubmit: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>

4 files changed, 59 insertions, 10 deletions

diff --git a/tvix/eval/src/lib.rs b/tvix/eval/src/lib.rs
index a759e0c0ab2c..0e0f12059188 100644
--- a/tvix/eval/src/lib.rs
+++ b/tvix/eval/src/lib.rs
@@ -213,7 +213,7 @@ impl<'code, 'co, 'ro> Evaluation<'code, 'co, 'ro> {
         let mut noop_observer = observer::NoOpObserver::default();
         let compiler_observer = self.compiler_observer.take().unwrap_or(&mut noop_observer);
 
-        let (lambda, _globals) = match parse_compile_internal(
+        let (lambda, globals) = match parse_compile_internal(
             &mut result,
             self.code,
             self.file.clone(),
@@ -246,7 +246,7 @@ impl<'code, 'co, 'ro> Evaluation<'code, 'co, 'ro> {
             .unwrap_or_default();
 
         let runtime_observer = self.runtime_observer.take().unwrap_or(&mut noop_observer);
-        let vm_result = run_lambda(nix_path, self.io_handle, runtime_observer, lambda);
+        let vm_result = run_lambda(nix_path, self.io_handle, runtime_observer, globals, lambda);
 
         match vm_result {
             Ok(mut runtime_result) => {
diff --git a/tvix/eval/src/value/attrs/tests.rs b/tvix/eval/src/value/attrs/tests.rs
index 2039c509fd56..ccf8dc7c10e0 100644
--- a/tvix/eval/src/value/attrs/tests.rs
+++ b/tvix/eval/src/value/attrs/tests.rs
@@ -10,7 +10,12 @@ mod nix_eq {
     #[proptest(ProptestConfig { cases: 2, ..Default::default() })]
     fn reflexive(x: NixAttrs) {
         let mut observer = NoOpObserver {};
-        let mut vm = VM::new(Default::default(), Box::new(crate::DummyIO), &mut observer);
+        let mut vm = VM::new(
+            Default::default(),
+            Box::new(crate::DummyIO),
+            &mut observer,
+            Default::default(),
+        );
 
         assert!(x.nix_eq(&x, &mut vm).unwrap())
     }
@@ -18,7 +23,12 @@ mod nix_eq {
     #[proptest(ProptestConfig { cases: 2, ..Default::default() })]
     fn symmetric(x: NixAttrs, y: NixAttrs) {
         let mut observer = NoOpObserver {};
-        let mut vm = VM::new(Default::default(), Box::new(crate::DummyIO), &mut observer);
+        let mut vm = VM::new(
+            Default::default(),
+            Box::new(crate::DummyIO),
+            &mut observer,
+            Default::default(),
+        );
 
         assert_eq!(
             x.nix_eq(&y, &mut vm).unwrap(),
@@ -29,7 +39,12 @@ mod nix_eq {
     #[proptest(ProptestConfig { cases: 2, ..Default::default() })]
     fn transitive(x: NixAttrs, y: NixAttrs, z: NixAttrs) {
         let mut observer = NoOpObserver {};
-        let mut vm = VM::new(Default::default(), Box::new(crate::DummyIO), &mut observer);
+        let mut vm = VM::new(
+            Default::default(),
+            Box::new(crate::DummyIO),
+            &mut observer,
+            Default::default(),
+        );
 
         if x.nix_eq(&y, &mut vm).unwrap() && y.nix_eq(&z, &mut vm).unwrap() {
             assert!(x.nix_eq(&z, &mut vm).unwrap())
diff --git a/tvix/eval/src/value/mod.rs b/tvix/eval/src/value/mod.rs
index 9ab23043c523..7a6bbcafb30f 100644
--- a/tvix/eval/src/value/mod.rs
+++ b/tvix/eval/src/value/mod.rs
@@ -578,7 +578,12 @@ mod tests {
         #[proptest(ProptestConfig { cases: 5, ..Default::default() })]
         fn reflexive(x: Value) {
             let mut observer = NoOpObserver {};
-            let mut vm = VM::new(Default::default(), Box::new(crate::DummyIO), &mut observer);
+            let mut vm = VM::new(
+                Default::default(),
+                Box::new(crate::DummyIO),
+                &mut observer,
+                Default::default(),
+            );
 
             assert!(x.nix_eq(&x, &mut vm).unwrap())
         }
@@ -586,7 +591,12 @@ mod tests {
         #[proptest(ProptestConfig { cases: 5, ..Default::default() })]
         fn symmetric(x: Value, y: Value) {
             let mut observer = NoOpObserver {};
-            let mut vm = VM::new(Default::default(), Box::new(crate::DummyIO), &mut observer);
+            let mut vm = VM::new(
+                Default::default(),
+                Box::new(crate::DummyIO),
+                &mut observer,
+                Default::default(),
+            );
 
             assert_eq!(
                 x.nix_eq(&y, &mut vm).unwrap(),
@@ -597,7 +607,12 @@ mod tests {
         #[proptest(ProptestConfig { cases: 5, ..Default::default() })]
         fn transitive(x: Value, y: Value, z: Value) {
             let mut observer = NoOpObserver {};
-            let mut vm = VM::new(Default::default(), Box::new(crate::DummyIO), &mut observer);
+            let mut vm = VM::new(
+                Default::default(),
+                Box::new(crate::DummyIO),
+                &mut observer,
+                Default::default(),
+            );
 
             if x.nix_eq(&y, &mut vm).unwrap() && y.nix_eq(&z, &mut vm).unwrap() {
                 assert!(x.nix_eq(&z, &mut vm).unwrap())
@@ -607,7 +622,12 @@ mod tests {
         #[test]
         fn list_int_float_fungibility() {
             let mut observer = NoOpObserver {};
-            let mut vm = VM::new(Default::default(), Box::new(crate::DummyIO), &mut observer);
+            let mut vm = VM::new(
+                Default::default(),
+                Box::new(crate::DummyIO),
+                &mut observer,
+                Default::default(),
+            );
 
             let v1 = Value::List(NixList::from(vector![Value::Integer(1)]));
             let v2 = Value::List(NixList::from(vector![Value::Float(1.0)]));
diff --git a/tvix/eval/src/vm.rs b/tvix/eval/src/vm.rs
index b42b47c2dc01..06ead175eeb1 100644
--- a/tvix/eval/src/vm.rs
+++ b/tvix/eval/src/vm.rs
@@ -6,6 +6,7 @@ use std::{cmp::Ordering, collections::BTreeMap, ops::DerefMut, path::PathBuf, rc
 
 use crate::{
     chunk::Chunk,
+    compiler::GlobalsMap,
     errors::{Error, ErrorKind, EvalResult},
     io::EvalIO,
     nix_search_path::NixSearchPath,
@@ -124,6 +125,16 @@ pub struct VM<'o> {
 
     /// Runtime observer which can print traces of runtime operations.
     observer: &'o mut dyn RuntimeObserver,
+
+    /// Strong reference to the globals, guaranteeing that they are
+    /// kept alive for the duration of evaluation.
+    ///
+    /// This is important because recursive builtins (specifically
+    /// `import`) hold a weak reference to the builtins, while the
+    /// original strong reference is held by the compiler which does
+    /// not exist anymore at runtime.
+    #[allow(dead_code)]
+    globals: Rc<GlobalsMap>,
 }
 
 /// The result of a VM's runtime evaluation.
@@ -207,6 +218,7 @@ impl<'o> VM<'o> {
         nix_search_path: NixSearchPath,
         io_handle: Box<dyn EvalIO>,
         observer: &'o mut dyn RuntimeObserver,
+        globals: Rc<GlobalsMap>,
     ) -> Self {
         // Backtrace-on-stack-overflow is some seriously weird voodoo and
         // very unsafe.  This double-guard prevents it from accidentally
@@ -221,6 +233,7 @@ impl<'o> VM<'o> {
             nix_search_path,
             io_handle,
             observer,
+            globals,
             frames: vec![],
             stack: vec![],
             with_stack: vec![],
@@ -1180,9 +1193,10 @@ pub fn run_lambda(
     nix_search_path: NixSearchPath,
     io_handle: Box<dyn EvalIO>,
     observer: &mut dyn RuntimeObserver,
+    globals: Rc<GlobalsMap>,
     lambda: Rc<Lambda>,
 ) -> EvalResult<RuntimeResult> {
-    let mut vm = VM::new(nix_search_path, io_handle, observer);
+    let mut vm = VM::new(nix_search_path, io_handle, observer, globals);
 
     // Retain the top-level span of the expression in this lambda, as
     // synthetic "calls" in deep_force will otherwise not have a span