From 3d238c350b4c0b4430d694b15e89319a150af889 Mon Sep 17 00:00:00 2001 From: Vincent Ambo Date: Tue, 3 Jan 2023 22:30:49 +0300 Subject: refactor(tvix/eval): streamline construction of globals/builtins Previously the construction of globals (a compiler-only concept) and builtins (a (now) user-facing API) was intermingled between multiple different modules, and kind of difficult to understand. The complexity of this had grown in large part due to the implementation of `builtins.import`, which required the notorious "knot-tying" trick using Rc::new_cyclic (see cl/7097) for constructing the set of globals. As part of the new `Evaluation` API users should have the ability to bring their own builtins, and control explicitly whether or not impure builtins are available (regardless of whether they're compiled in or not). To streamline the construction and allow the new API features to work, this commit restructures things by making these changes: 1. The `tvix_eval::builtins` module is now only responsible for exporting sets of builtins. It no longer has any knowledge of whether or not certain sets (e.g. only pure, or pure+impure) are enabled, and it has no control over which builtins are globally available (this is now handled in the compiler). 2. The compiler module is now responsible for both constructing the final attribute set of builtins from the set of builtins supplied by a user, as well as for populating its globals (that is identifiers which are available at the top-level scope). 3. The `Evaluation` API now carries a `builtins` field which is populated with the pure builtins by default, and can be extended by users. 4. The `import` feature has been moved into the compiler, as a special case. In general, builtins no longer have the ability to reference the "fix point" of the globals set. This should not change any functionality, and in fact preserves minor differences between Tvix/Nix that we already had (such as `builtins.builtins` not existing). Change-Id: Icdf5dd50eb81eb9260d89269d6e08b1e67811a2c Reviewed-on: https://cl.tvl.fyi/c/depot/+/7738 Reviewed-by: sterni Autosubmit: tazjin Tested-by: BuildkiteCI Reviewed-by: flokli --- tvix/eval/src/builtins/impure.rs | 111 ++++----------------------------- tvix/eval/src/builtins/mod.rs | 130 ++++++++++++--------------------------- 2 files changed, 50 insertions(+), 191 deletions(-) (limited to 'tvix/eval/src/builtins') diff --git a/tvix/eval/src/builtins/impure.rs b/tvix/eval/src/builtins/impure.rs index d371d877977d..e8c032cc77e6 100644 --- a/tvix/eval/src/builtins/impure.rs +++ b/tvix/eval/src/builtins/impure.rs @@ -2,20 +2,17 @@ use builtin_macros::builtins; use smol_str::SmolStr; use std::{ - collections::BTreeMap, env, - rc::{Rc, Weak}, + rc::Rc, time::{SystemTime, UNIX_EPOCH}, }; use crate::{ - compiler::GlobalsMap, errors::ErrorKind, io::FileType, - observer::NoOpObserver, - value::{Builtin, BuiltinArgument, NixAttrs, Thunk}, + value::{NixAttrs, Thunk}, vm::VM, - SourceCode, Value, + Value, }; #[builtins] @@ -67,13 +64,13 @@ mod impure_builtins { /// 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<&'static str, Value> { - let mut map: BTreeMap<&'static str, Value> = impure_builtins::builtins() +pub fn impure_builtins() -> Vec<(&'static str, Value)> { + let mut result = impure_builtins::builtins() .into_iter() - .map(|b| (b.name(), Value::Builtin(b))) - .collect(); + .map(super::builtin_tuple) + .collect::>(); - map.insert( + result.push(( "storeDir", Value::Thunk(Thunk::new_suspended_native(Rc::new(Box::new( |vm: &mut VM| match vm.io().store_dir() { @@ -81,7 +78,7 @@ pub(super) fn builtins() -> BTreeMap<&'static str, Value> { Some(dir) => Ok(Value::String(dir.into())), }, )))), - ); + )); // currentTime pins the time at which evaluation was started { @@ -92,94 +89,8 @@ pub(super) fn builtins() -> BTreeMap<&'static str, Value> { Err(err) => -(err.duration().as_secs() as i64), }; - map.insert("currentTime", Value::Integer(seconds)); + result.push(("currentTime", Value::Integer(seconds))); } - map -} - -/// Constructs and inserts the `import` builtin. This builtin is special in that -/// 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: &Weak, 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", - &[BuiltinArgument { - strict: true, - name: "path", - }], - None, - move |mut args: Vec, vm: &mut VM| { - let mut path = super::coerce_value_to_path(&args.pop().unwrap(), vm)?; - if path.is_dir() { - path.push("default.nix"); - } - - let current_span = vm.current_light_span(); - - if let Some(cached) = vm.import_cache.get(&path) { - return Ok(cached.clone()); - } - - let contents = vm.io().read_to_string(path.clone())?; - - let parsed = rnix::ast::Root::parse(&contents); - let errors = parsed.errors(); - - let file = source.add_file(path.to_string_lossy().to_string(), contents); - - if !errors.is_empty() { - return Err(ErrorKind::ImportParseError { - path, - file, - errors: errors.to_vec(), - }); - } - - let result = crate::compiler::compile( - &parsed.tree().expr().unwrap(), - Some(path.clone()), - file, - // 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 { - path: path.clone(), - errors: vec![err], - })?; - - if !result.errors.is_empty() { - return Err(ErrorKind::ImportCompilerError { - path, - errors: result.errors, - }); - } - - // Compilation succeeded, we can construct a thunk from whatever it spat - // out and return that. - let res = Value::Thunk(Thunk::new_suspended(result.lambda, current_span)); - - vm.import_cache.insert(path, res.clone()); - - for warning in result.warnings { - vm.push_warning(warning); - } - - Ok(res) - }, - ) + result } diff --git a/tvix/eval/src/builtins/mod.rs b/tvix/eval/src/builtins/mod.rs index 2e043a1b104f..01ef1678c7ee 100644 --- a/tvix/eval/src/builtins/mod.rs +++ b/tvix/eval/src/builtins/mod.rs @@ -3,17 +3,15 @@ //! 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 crate::value::BuiltinArgument; use std::cmp::{self, Ordering}; -use std::collections::{BTreeMap, HashMap, HashSet}; +use std::collections::{BTreeMap, HashSet}; use std::path::PathBuf; -use std::rc::Rc; use builtin_macros::builtins; use regex::Regex; +use crate::arithmetic_op; +use crate::value::BuiltinArgument; use crate::warnings::WarningKind; use crate::{ errors::{ErrorKind, EvalResult}, @@ -21,13 +19,18 @@ use crate::{ vm::VM, }; -use crate::arithmetic_op; - use self::versions::{VersionPart, VersionPartsIter}; +mod versions; + #[cfg(feature = "impure")] -pub mod impure; -pub mod versions; +mod impure; + +#[cfg(feature = "impure")] +pub use impure::impure_builtins; + +// we set TVIX_CURRENT_SYSTEM in build.rs +pub const CURRENT_PLATFORM: &str = env!("TVIX_CURRENT_SYSTEM"); /// Coerce a Nix Value to a plain path, e.g. in order to access the /// file it points to via either `builtins.toPath` or an impure @@ -942,15 +945,37 @@ mod pure_builtins { } } -pub use pure_builtins::builtins as pure_builtins; +fn builtin_tuple(builtin: Builtin) -> (&'static str, Value) { + (builtin.name(), Value::Builtin(builtin)) +} + +/// The set of standard pure builtins in Nix, mostly concerned with +/// data structure manipulation (string, attrs, list, etc. functions). +pub fn pure_builtins() -> Vec<(&'static str, Value)> { + let mut result = pure_builtins::builtins() + .into_iter() + .map(builtin_tuple) + .collect::>(); + + // Pure-value builtins + result.push(("nixVersion", Value::String("2.3-compat-tvix-0.1".into()))); + result.push(("langVersion", Value::Integer(6))); + + result.push(( + "currentSystem", + crate::systems::llvm_triple_to_nix_double(CURRENT_PLATFORM).into(), + )); + + result +} /// Placeholder builtins that technically have a function which we do /// not yet implement, but which is also not easily observable from /// within a pure evaluation context. /// /// These are used as a crutch to make progress on nixpkgs evaluation. -fn placeholders() -> Vec { - vec![ +pub fn placeholders() -> Vec<(&'static str, Value)> { + let ph = vec![ Builtin::new( "addErrorContext", &[ @@ -1041,84 +1066,7 @@ fn placeholders() -> Vec { Ok(Value::Attrs(Box::new(attrs))) }, ), - ] -} -// we set TVIX_CURRENT_SYSTEM in build.rs -pub const CURRENT_PLATFORM: &str = env!("TVIX_CURRENT_SYSTEM"); - -/// Set of Nix builtins that are globally available. -pub fn global_builtins(source: SourceCode) -> GlobalsMapFunc { - Box::new(move |globals: &std::rc::Weak| { - let mut map: BTreeMap<&'static str, Value> = BTreeMap::new(); - - // Pure-value builtins - map.insert("nixVersion", Value::String("2.3-compat-tvix-0.1".into())); - - map.insert("langVersion", Value::Integer(6)); - - map.insert( - "currentSystem", - crate::systems::llvm_triple_to_nix_double(CURRENT_PLATFORM).into(), - ); - - let mut add_builtins = |builtins: Vec| { - 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)); - - map.insert("import", import); - }; - - let mut globals: GlobalsMap = HashMap::new(); - - let builtins = Rc::new(NixAttrs::from_iter(map.into_iter())); - - // 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", - Rc::new(move |c, s| c.emit_constant(Value::attrs(builtins.as_ref().clone()), &s)), - ); + ]; - globals - }) + ph.into_iter().map(builtin_tuple).collect() } -- cgit 1.4.1