about summary refs log tree commit diff
path: root/tvix/eval
diff options
context:
space:
mode:
Diffstat (limited to 'tvix/eval')
-rw-r--r--tvix/eval/src/builtins/impure.rs38
-rw-r--r--tvix/eval/src/builtins/mod.rs135
-rw-r--r--tvix/eval/src/compiler/mod.rs96
-rw-r--r--tvix/eval/src/eval.rs20
-rw-r--r--tvix/eval/src/lib.rs2
5 files changed, 154 insertions, 137 deletions
diff --git a/tvix/eval/src/builtins/impure.rs b/tvix/eval/src/builtins/impure.rs
index 5edd81999a7c..8433c04c419d 100644
--- a/tvix/eval/src/builtins/impure.rs
+++ b/tvix/eval/src/builtins/impure.rs
@@ -1,17 +1,17 @@
 use std::{
-    cell::RefCell,
-    collections::{BTreeMap, HashMap},
+    collections::BTreeMap,
     env,
     fs::File,
     io::{self, Read},
-    rc::Rc,
+    rc::{Rc, Weak},
     time::{SystemTime, UNIX_EPOCH},
 };
 
 use crate::{
+    compiler::GlobalsMap,
     errors::ErrorKind,
     observer::NoOpObserver,
-    value::{Builtin, NixAttrs, NixString, Thunk},
+    value::{Builtin, NixAttrs, Thunk},
     vm::VM,
     SourceCode, Value,
 };
@@ -69,10 +69,10 @@ fn impure_builtins() -> Vec<Builtin> {
 
 /// Return all impure builtins, that is all builtins which may perform I/O
 /// outside of the VM and so cannot be used in all contexts (e.g. WASM).
-pub(super) fn builtins() -> BTreeMap<NixString, Value> {
-    let mut map: BTreeMap<NixString, Value> = impure_builtins()
+pub(super) fn builtins() -> BTreeMap<&'static str, Value> {
+    let mut map: BTreeMap<&'static str, Value> = impure_builtins()
         .into_iter()
-        .map(|b| (b.name().into(), Value::Builtin(b)))
+        .map(|b| (b.name(), Value::Builtin(b)))
         .collect();
 
     // currentTime pins the time at which evaluation was started
@@ -84,7 +84,7 @@ pub(super) fn builtins() -> BTreeMap<NixString, Value> {
             Err(err) => -(err.duration().as_secs() as i64),
         };
 
-        map.insert(NixString::from("currentTime"), Value::Integer(seconds));
+        map.insert("currentTime", Value::Integer(seconds));
     }
 
     map
@@ -94,10 +94,13 @@ pub(super) fn builtins() -> BTreeMap<NixString, Value> {
 /// it needs to capture the [crate::SourceCode] structure to correctly track
 /// source code locations while invoking a compiler.
 // TODO: need to be able to pass through a CompilationObserver, too.
-pub fn builtins_import(
-    globals: Rc<RefCell<HashMap<&'static str, Value>>>,
-    source: SourceCode,
-) -> Builtin {
+pub fn builtins_import(globals: &Weak<GlobalsMap>, source: SourceCode) -> Builtin {
+    // This (very cheap, once-per-compiler-startup) clone exists
+    // solely in order to keep the borrow checker happy.  It
+    // resolves the tension between the requirements of
+    // Rc::new_cyclic() and Builtin::new()
+    let globals = globals.clone();
+
     Builtin::new(
         "import",
         &[true],
@@ -126,11 +129,18 @@ pub fn builtins_import(
                 });
             }
 
-            let result = crate::compile(
+            let result = crate::compiler::compile(
                 &parsed.tree().expr().unwrap(),
                 Some(path.clone()),
                 file,
-                globals.clone(),
+                // The VM must ensure that a strong reference to the
+                // globals outlives any self-references (which are
+                // weak) embedded within the globals.  If the
+                // expect() below panics, it means that did not
+                // happen.
+                globals
+                    .upgrade()
+                    .expect("globals dropped while still in use"),
                 &mut NoOpObserver::default(),
             )
             .map_err(|err| ErrorKind::ImportCompilerError {
diff --git a/tvix/eval/src/builtins/mod.rs b/tvix/eval/src/builtins/mod.rs
index 91fe27aa4695..7dd7ee946d32 100644
--- a/tvix/eval/src/builtins/mod.rs
+++ b/tvix/eval/src/builtins/mod.rs
@@ -3,9 +3,12 @@
 //! See //tvix/eval/docs/builtins.md for a some context on the
 //! available builtins in Nix.
 
+use crate::compiler::{GlobalsMap, GlobalsMapFunc};
+use crate::source::SourceCode;
 use std::cmp::{self, Ordering};
 use std::collections::{BTreeMap, HashMap, HashSet};
 use std::path::PathBuf;
+use std::rc::Rc;
 
 use regex::Regex;
 
@@ -690,70 +693,84 @@ fn placeholders() -> Vec<Builtin> {
 // we set TVIX_CURRENT_SYSTEM in build.rs
 pub const CURRENT_PLATFORM: &str = env!("TVIX_CURRENT_SYSTEM");
 
-fn builtins_set() -> NixAttrs {
-    let mut map: BTreeMap<NixString, Value> = BTreeMap::new();
-
-    // Pure-value builtins
-    map.insert(
-        "nixVersion".into(),
-        Value::String("2.3-compat-tvix-0.1".into()),
-    );
-
-    map.insert("langVersion".into(), Value::Integer(6));
+/// Set of Nix builtins that are globally available.
+pub fn global_builtins(source: SourceCode) -> GlobalsMapFunc {
+    Box::new(move |globals: &std::rc::Weak<GlobalsMap>| {
+        let mut map: BTreeMap<&'static str, Value> = BTreeMap::new();
 
-    map.insert(
-        "currentSystem".into(),
-        crate::systems::llvm_triple_to_nix_double(CURRENT_PLATFORM).into(),
-    );
+        // Pure-value builtins
+        map.insert("nixVersion", Value::String("2.3-compat-tvix-0.1".into()));
 
-    let mut add_builtins = |builtins: Vec<Builtin>| {
-        for builtin in builtins {
-            map.insert(builtin.name().into(), Value::Builtin(builtin));
-        }
-    };
+        map.insert("langVersion", Value::Integer(6));
 
-    add_builtins(pure_builtins());
-    add_builtins(placeholders());
+        map.insert(
+            "currentSystem",
+            crate::systems::llvm_triple_to_nix_double(CURRENT_PLATFORM).into(),
+        );
 
-    #[cfg(feature = "impure")]
-    {
-        map.extend(impure::builtins());
-    }
-
-    NixAttrs::from_map(map)
-}
-
-/// Set of Nix builtins that are globally available.
-pub fn global_builtins() -> HashMap<&'static str, Value> {
-    let builtins = builtins_set();
-    let mut globals: HashMap<&'static str, Value> = HashMap::new();
-
-    // known global builtins from the builtins set.
-    for global in &[
-        "abort",
-        "baseNameOf",
-        "derivation",
-        "derivationStrict",
-        "dirOf",
-        "fetchGit",
-        "fetchMercurial",
-        "fetchTarball",
-        "fromTOML",
-        "import",
-        "isNull",
-        "map",
-        "placeholder",
-        "removeAttrs",
-        "scopedImport",
-        "throw",
-        "toString",
-    ] {
-        if let Some(builtin) = builtins.select(global) {
-            globals.insert(global, builtin.clone());
+        let mut add_builtins = |builtins: Vec<Builtin>| {
+            for builtin in builtins {
+                map.insert(builtin.name(), Value::Builtin(builtin));
+            }
+        };
+
+        add_builtins(pure_builtins());
+        add_builtins(placeholders());
+
+        #[cfg(feature = "impure")]
+        {
+            map.extend(impure::builtins());
+
+            // We need to insert import into the builtins, but the
+            // builtins passed to import must have import *in it*.
+            let import = Value::Builtin(crate::builtins::impure::builtins_import(
+                globals,
+                source.clone(),
+            ));
+
+            map.insert("import", import);
+        };
+
+        let mut globals: GlobalsMap = HashMap::new();
+
+        let builtins = Rc::new(NixAttrs::from_map(
+            map.into_iter().map(|(k, v)| (k.into(), v)).collect(),
+        ));
+
+        // known global builtins from the builtins set.
+        for global in &[
+            "abort",
+            "baseNameOf",
+            "derivation",
+            "derivationStrict",
+            "dirOf",
+            "fetchGit",
+            "fetchMercurial",
+            "fetchTarball",
+            "fromTOML",
+            "import",
+            "isNull",
+            "map",
+            "placeholder",
+            "removeAttrs",
+            "scopedImport",
+            "throw",
+            "toString",
+        ] {
+            if let Some(builtin) = builtins.select(global) {
+                let builtin = builtin.clone();
+                globals.insert(
+                    global,
+                    Rc::new(move |c, s| c.emit_constant(builtin.clone(), &s)),
+                );
+            }
         }
-    }
 
-    globals.insert("builtins", Value::attrs(builtins));
+        globals.insert(
+            "builtins",
+            Rc::new(move |c, s| c.emit_constant(Value::attrs(builtins.as_ref().clone()), &s)),
+        );
 
-    globals
+        globals
+    })
 }
diff --git a/tvix/eval/src/compiler/mod.rs b/tvix/eval/src/compiler/mod.rs
index 2985c7e90e0b..34a15330c227 100644
--- a/tvix/eval/src/compiler/mod.rs
+++ b/tvix/eval/src/compiler/mod.rs
@@ -19,10 +19,9 @@ mod scope;
 use codemap::Span;
 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;
+use std::rc::{Rc, Weak};
 use std::sync::Arc;
 
 use crate::chunk::Chunk;
@@ -42,6 +41,10 @@ pub struct CompilationOutput {
     pub lambda: Rc<Lambda>,
     pub warnings: Vec<EvalWarning>,
     pub errors: Vec<Error>,
+
+    // This field must outlive the rc::Weak reference which breaks
+    // the builtins -> import -> builtins reference cycle.
+    pub globals: Rc<GlobalsMap>,
 }
 
 /// Represents the lambda currently being compiled.
@@ -69,11 +72,19 @@ impl LambdaCtx {
     }
 }
 
-/// Alias for the map of globally available functions that should
-/// implicitly be resolvable in the global scope.
-type GlobalsMap = HashMap<&'static str, Rc<dyn Fn(&mut Compiler, Span)>>;
+/// The map of globally available functions that should implicitly
+/// be resolvable in the global scope.
+pub type GlobalsMap = HashMap<&'static str, Rc<dyn Fn(&mut Compiler, Span)>>;
+
+/// Functions with this type are used to construct a
+/// self-referential `builtins` object; it takes a weak reference to
+/// its own result, similar to how nixpkgs' overlays work.
+/// Rc::new_cyclic() is what "ties the knot".  The heap allocation
+/// (Box) and vtable (dyn) do not impair runtime or compile-time
+/// performance; they exist only during compiler startup.
+pub type GlobalsMapFunc = Box<dyn FnOnce(&Weak<GlobalsMap>) -> GlobalsMap>;
 
-struct Compiler<'observer> {
+pub struct Compiler<'observer> {
     contexts: Vec<LambdaCtx>,
     warnings: Vec<EvalWarning>,
     errors: Vec<Error>,
@@ -85,7 +96,7 @@ struct Compiler<'observer> {
     /// Each global has an associated token, which when encountered as
     /// an identifier is resolved against the scope poisoning logic,
     /// and a function that should emit code for the token.
-    globals: GlobalsMap,
+    globals: Rc<GlobalsMap>,
 
     /// File reference in the codemap contains all known source code
     /// and is used to track the spans from which instructions where
@@ -108,7 +119,7 @@ impl<'observer> Compiler<'observer> {
     pub(crate) fn new(
         location: Option<PathBuf>,
         file: Arc<codemap::File>,
-        globals: Rc<RefCell<HashMap<&'static str, Value>>>,
+        globals: Rc<GlobalsMap>,
         observer: &'observer mut dyn CompilerObserver,
     ) -> EvalResult<Self> {
         let mut root_dir = match location {
@@ -136,8 +147,6 @@ impl<'observer> Compiler<'observer> {
             root_dir.pop();
         }
 
-        let globals = globals.borrow();
-
         #[cfg(not(target_arch = "wasm32"))]
         debug_assert!(root_dir.is_absolute());
 
@@ -145,7 +154,7 @@ impl<'observer> Compiler<'observer> {
             root_dir,
             file,
             observer,
-            globals: prepare_globals(&globals),
+            globals,
             contexts: vec![LambdaCtx::new()],
             warnings: vec![],
             errors: vec![],
@@ -186,7 +195,7 @@ impl Compiler<'_> {
 
     /// Emit a single constant to the current bytecode chunk and track
     /// the source span from which it was compiled.
-    fn emit_constant<T: ToSpan>(&mut self, value: Value, node: &T) {
+    pub(super) fn emit_constant<T: ToSpan>(&mut self, value: Value, node: &T) {
         let idx = self.chunk().push_constant(value);
         self.push_op(OpCode::OpConstant(idx), node);
     }
@@ -1174,54 +1183,50 @@ fn optimise_tail_call(chunk: &mut Chunk) {
 
 /// Prepare the full set of globals from additional globals supplied
 /// by the caller of the compiler, as well as the built-in globals
-/// that are always part of the language.
+/// that are always part of the language.  This also "ties the knot"
+/// required in order for import to have a reference cycle back to
+/// the globals.
 ///
 /// 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 {
-    let mut globals: GlobalsMap = HashMap::new();
-
-    globals.insert(
-        "true",
-        Rc::new(|compiler, span| {
-            compiler.push_op(OpCode::OpTrue, &span);
-        }),
-    );
-
-    globals.insert(
-        "false",
-        Rc::new(|compiler, span| {
-            compiler.push_op(OpCode::OpFalse, &span);
-        }),
-    );
-
-    globals.insert(
-        "null",
-        Rc::new(|compiler, span| {
-            compiler.push_op(OpCode::OpNull, &span);
-        }),
-    );
-
-    for (ident, value) in additional.iter() {
-        let value: Value = value.clone();
+pub fn prepare_globals(additional: GlobalsMapFunc) -> Rc<GlobalsMap> {
+    Rc::new_cyclic(Box::new(|weak: &Weak<GlobalsMap>| {
+        let mut globals = additional(weak);
+
         globals.insert(
-            ident,
-            Rc::new(move |compiler, span| compiler.emit_constant(value.clone(), &span)),
+            "true",
+            Rc::new(|compiler, span| {
+                compiler.push_op(OpCode::OpTrue, &span);
+            }),
+        );
+
+        globals.insert(
+            "false",
+            Rc::new(|compiler, span| {
+                compiler.push_op(OpCode::OpFalse, &span);
+            }),
+        );
+
+        globals.insert(
+            "null",
+            Rc::new(|compiler, span| {
+                compiler.push_op(OpCode::OpNull, &span);
+            }),
         );
-    }
 
-    globals
+        globals
+    }))
 }
 
 pub fn compile(
     expr: &ast::Expr,
     location: Option<PathBuf>,
     file: Arc<codemap::File>,
-    globals: Rc<RefCell<HashMap<&'static str, Value>>>,
+    globals: Rc<GlobalsMap>,
     observer: &mut dyn CompilerObserver,
 ) -> EvalResult<CompilationOutput> {
-    let mut c = Compiler::new(location, file, globals, observer)?;
+    let mut c = Compiler::new(location, file, globals.clone(), observer)?;
 
     let root_span = c.span_for(expr);
     let root_slot = c.scope_mut().declare_phantom(root_span, false);
@@ -1240,5 +1245,6 @@ pub fn compile(
         lambda,
         warnings: c.warnings,
         errors: c.errors,
+        globals: globals,
     })
 }
diff --git a/tvix/eval/src/eval.rs b/tvix/eval/src/eval.rs
index df2c562cfa33..6a35630b3ff7 100644
--- a/tvix/eval/src/eval.rs
+++ b/tvix/eval/src/eval.rs
@@ -1,4 +1,4 @@
-use std::{cell::RefCell, path::PathBuf, rc::Rc};
+use std::path::PathBuf;
 
 use crate::{
     builtins::global_builtins,
@@ -80,23 +80,7 @@ pub fn interpret(code: &str, location: Option<PathBuf>, options: Options) -> Eva
         println!("{}", pretty_print_expr(&root_expr));
     }
 
-    // TODO: encapsulate this import weirdness in builtins
-
-    let builtins = Rc::new(RefCell::new(global_builtins()));
-
-    #[cfg(feature = "impure")]
-    {
-        // We need to insert import into the builtins, but the
-        // builtins passed to import must have import *in it*.
-        let import = Value::Builtin(crate::builtins::impure::builtins_import(
-            builtins.clone(),
-            source.clone(),
-        ));
-
-        builtins.borrow_mut().insert("import", import);
-        // TODO: also add it into the inner builtins set
-    };
-
+    let builtins = crate::compiler::prepare_globals(Box::new(global_builtins(source.clone())));
     let result = if options.dump_bytecode {
         crate::compiler::compile(
             &root_expr,
diff --git a/tvix/eval/src/lib.rs b/tvix/eval/src/lib.rs
index 70d2dbbe3891..6cf3aa212a18 100644
--- a/tvix/eval/src/lib.rs
+++ b/tvix/eval/src/lib.rs
@@ -24,7 +24,7 @@ mod tests;
 
 // Re-export the public interface used by other crates.
 pub use crate::builtins::global_builtins;
-pub use crate::compiler::compile;
+pub use crate::compiler::{compile, prepare_globals};
 pub use crate::errors::EvalResult;
 pub use crate::eval::{interpret, Options};
 pub use crate::pretty_ast::pretty_print_expr;