feat(tvix/eval): insert `import` into the builtins itself r/5048

Adding `import` to builtins causes causes a bootstrap cycle because
the `import` builtin needs to be initialised with the set of globals
before being inserted into the globals, which also must contain
itself.

To break out of the cycle this hack wraps the builtins passed to the
compiler in an `Rc` (probably sensible anyways, as they will end up
getting cloned a bunch), containing a RefCell which gives us mutable
access to the builtins.

This opens up a potentially dangerous footgun in which we could mutate
the builtins at runtime leading to different compiler invocations
seeing different builtins, so it'd be nice to have some kind of
"finalised" status for them or some such, but I'm not sure how to
represent that atm.

Change-Id: I25f8d4d2a7e8472d401c8ba2f4bbf9d86ab2abcb
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6867
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>

1 files changed, 9 insertions, 5 deletions

diff --git a/tvix/eval/src/compiler/mod.rs b/tvix/eval/src/compiler/mod.rs
index eb617b227351..472f4aaf363d 100644
--- a/tvix/eval/src/compiler/mod.rs
+++ b/tvix/eval/src/compiler/mod.rs
@@ -21,6 +21,7 @@ use codemap::Span;
 use path_clean::PathClean;
 use rnix::ast::{self, AstToken};
 use smol_str::SmolStr;
+use std::cell::RefCell;
 use std::collections::HashMap;
 use std::path::{Path, PathBuf};
 use std::rc::Rc;
@@ -103,7 +104,7 @@ impl<'observer> Compiler<'observer> {
     pub(crate) fn new(
         location: Option<PathBuf>,
         file: Arc<codemap::File>,
-        globals: HashMap<&'static str, Value>,
+        globals: Rc<RefCell<HashMap<&'static str, Value>>>,
         observer: &'observer mut dyn CompilerObserver,
     ) -> EvalResult<Self> {
         let mut root_dir = match location {
@@ -124,11 +125,13 @@ impl<'observer> Compiler<'observer> {
             root_dir.pop();
         }
 
+        let globals = globals.borrow();
+
         Ok(Self {
             root_dir,
             file,
             observer,
-            globals: prepare_globals(globals),
+            globals: prepare_globals(&globals),
             contexts: vec![LambdaCtx::new()],
             warnings: vec![],
             errors: vec![],
@@ -1103,7 +1106,7 @@ fn optimise_tail_call(chunk: &mut Chunk) {
 /// Note that all builtin functions are *not* considered part of the
 /// language in this sense and MUST be supplied as additional global
 /// values, including the `builtins` set itself.
-fn prepare_globals(additional: HashMap<&'static str, Value>) -> GlobalsMap {
+fn prepare_globals(additional: &HashMap<&'static str, Value>) -> GlobalsMap {
     let mut globals: GlobalsMap = HashMap::new();
 
     globals.insert(
@@ -1127,7 +1130,8 @@ fn prepare_globals(additional: HashMap<&'static str, Value>) -> GlobalsMap {
         }),
     );
 
-    for (ident, value) in additional.into_iter() {
+    for (ident, value) in additional.iter() {
+        let value: Value = value.clone();
         globals.insert(
             ident,
             Rc::new(move |compiler, span| compiler.emit_constant(value.clone(), &span)),
@@ -1141,7 +1145,7 @@ pub fn compile(
     expr: &ast::Expr,
     location: Option<PathBuf>,
     file: Arc<codemap::File>,
-    globals: HashMap<&'static str, Value>,
+    globals: Rc<RefCell<HashMap<&'static str, Value>>>,
     observer: &mut dyn CompilerObserver,
 ) -> EvalResult<CompilationOutput> {
     let mut c = Compiler::new(location, file, globals, observer)?;