refactor(tvix/eval): Construct globals in EvaluationBuilder::build

Construct the Rc<GlobalsMap> for the evaluation as part of
EvaluiationBuilder::build, rather than deferring it until we actually
compile. This changes nothing functionally, but gets us one step closer
to sharing this globals map across evaluations.

Change-Id: Id92e9fb88d974d763056d4f15ce61962ab776e84
Reviewed-on: https://cl.tvl.fyi/c/depot/+/11957
Tested-by: BuildkiteCI
Autosubmit: aspen <root@gws.fyi>
Reviewed-by: flokli <flokli@flokli.de>
This commit is contained in:
Aspen Smith 2024-07-05 20:50:56 -04:00 committed by clbot
parent dfe137786c
commit 3a79f93795
2 changed files with 47 additions and 58 deletions

View file

@ -44,11 +44,6 @@ pub struct CompilationOutput {
pub lambda: Rc<Lambda>, pub lambda: Rc<Lambda>,
pub warnings: Vec<EvalWarning>, pub warnings: Vec<EvalWarning>,
pub errors: Vec<Error>, pub errors: Vec<Error>,
// This field must outlive the rc::Weak reference which breaks the
// builtins -> import -> builtins reference cycle. For this
// reason, it must be passed to the VM.
pub globals: Rc<GlobalsMap>,
} }
/// Represents the lambda currently being compiled. /// Represents the lambda currently being compiled.
@ -1689,6 +1684,5 @@ pub fn compile(
lambda, lambda,
warnings: c.warnings, warnings: c.warnings,
errors: c.errors, errors: c.errors,
globals,
}) })
} }

View file

@ -87,6 +87,43 @@ pub struct EvaluationBuilder<'co, 'ro, 'env, IO> {
runtime_observer: Option<&'ro mut dyn RuntimeObserver>, runtime_observer: Option<&'ro mut dyn RuntimeObserver>,
} }
impl<'co, 'ro, 'env, IO> EvaluationBuilder<'co, 'ro, 'env, IO>
where
IO: AsRef<dyn EvalIO> + 'static,
{
/// Build an [`Evaluation`] based on the configuration in this builder.
///
/// This:
///
/// - Adds a `"storeDir"` builtin containing the store directory of the configured IO handle
/// - Sets up globals based on the configured builtins
/// - Copies all other configured fields to the [`Evaluation`]
pub fn build(mut self) -> Evaluation<'co, 'ro, 'env, IO> {
// Insert a storeDir builtin *iff* a store directory is present.
if let Some(store_dir) = self.io_handle.as_ref().store_dir() {
self.builtins.push(("storeDir", store_dir.into()));
}
let globals = crate::compiler::prepare_globals(
self.builtins,
self.src_builtins,
self.source_map.clone(),
self.enable_import,
);
Evaluation {
source_map: self.source_map,
globals,
env: self.env,
io_handle: self.io_handle,
strict: self.strict,
nix_path: self.nix_path,
compiler_observer: self.compiler_observer,
runtime_observer: self.runtime_observer,
}
}
}
// NOTE(aspen): The methods here are intentionally incomplete; feel free to add new ones (ideally // NOTE(aspen): The methods here are intentionally incomplete; feel free to add new ones (ideally
// with similar naming conventions to the ones already present) but don't expose fields publically! // with similar naming conventions to the ones already present) but don't expose fields publically!
impl<'co, 'ro, 'env, IO> EvaluationBuilder<'co, 'ro, 'env, IO> { impl<'co, 'ro, 'env, IO> EvaluationBuilder<'co, 'ro, 'env, IO> {
@ -108,21 +145,6 @@ impl<'co, 'ro, 'env, IO> EvaluationBuilder<'co, 'ro, 'env, IO> {
} }
} }
pub fn build(self) -> Evaluation<'co, 'ro, 'env, IO> {
Evaluation {
source_map: self.source_map,
builtins: self.builtins,
src_builtins: self.src_builtins,
env: self.env,
io_handle: self.io_handle,
enable_import: self.enable_import,
strict: self.strict,
nix_path: self.nix_path,
compiler_observer: self.compiler_observer,
runtime_observer: self.runtime_observer,
}
}
pub fn io_handle<IO2>(self, io_handle: IO2) -> EvaluationBuilder<'co, 'ro, 'env, IO2> { pub fn io_handle<IO2>(self, io_handle: IO2) -> EvaluationBuilder<'co, 'ro, 'env, IO2> {
EvaluationBuilder { EvaluationBuilder {
io_handle, io_handle,
@ -260,16 +282,8 @@ pub struct Evaluation<'co, 'ro, 'env, IO> {
/// Source code map used for error reporting. /// Source code map used for error reporting.
source_map: SourceCode, source_map: SourceCode,
/// Set of all builtins that should be available during the /// Set of all global values available at the top-level scope
/// evaluation. globals: Rc<GlobalsMap>,
///
/// This defaults to all pure builtins. Users might want to add
/// the set of impure builtins, or other custom builtins.
builtins: Vec<(&'static str, Value)>,
/// Set of builtins that are implemented in Nix itself and should
/// be compiled and inserted in the builtins set.
src_builtins: Vec<(&'static str, &'static str)>,
/// Top-level variables to define in the evaluation /// Top-level variables to define in the evaluation
env: Option<&'env HashMap<SmolStr, Value>>, env: Option<&'env HashMap<SmolStr, Value>>,
@ -280,11 +294,6 @@ pub struct Evaluation<'co, 'ro, 'env, IO> {
/// Defaults to [`DummyIO`] if not set explicitly. /// Defaults to [`DummyIO`] if not set explicitly.
io_handle: IO, io_handle: IO,
/// Determines whether the `import` builtin should be made
/// available. Note that this depends on the `io_handle` being
/// able to read the files specified as arguments to `import`.
enable_import: bool,
/// Determines whether the returned value should be strictly /// Determines whether the returned value should be strictly
/// evaluated, that is whether its list and attribute set elements /// evaluated, that is whether its list and attribute set elements
/// should be forced recursively. /// should be forced recursively.
@ -378,10 +387,8 @@ where
file, file,
location, location,
source, source,
self.builtins, self.globals,
self.src_builtins,
self.env, self.env,
self.enable_import,
compiler_observer, compiler_observer,
); );
@ -409,21 +416,14 @@ where
let mut noop_observer = observer::NoOpObserver::default(); let mut noop_observer = observer::NoOpObserver::default();
let compiler_observer = self.compiler_observer.take().unwrap_or(&mut noop_observer); let compiler_observer = self.compiler_observer.take().unwrap_or(&mut noop_observer);
// Insert a storeDir builtin *iff* a store directory is present. let lambda = match parse_compile_internal(
if let Some(store_dir) = self.io_handle.as_ref().store_dir() {
self.builtins.push(("storeDir", store_dir.into()));
}
let (lambda, globals) = match parse_compile_internal(
&mut result, &mut result,
code.as_ref(), code.as_ref(),
file.clone(), file.clone(),
location, location,
source.clone(), source.clone(),
self.builtins, self.globals.clone(),
self.src_builtins,
self.env, self.env,
self.enable_import,
compiler_observer, compiler_observer,
) { ) {
None => return result, None => return result,
@ -455,7 +455,7 @@ where
self.io_handle, self.io_handle,
runtime_observer, runtime_observer,
source.clone(), source.clone(),
globals, self.globals,
lambda, lambda,
self.strict, self.strict,
); );
@ -492,12 +492,10 @@ fn parse_compile_internal(
file: Arc<codemap::File>, file: Arc<codemap::File>,
location: Option<PathBuf>, location: Option<PathBuf>,
source: SourceCode, source: SourceCode,
builtins: Vec<(&'static str, Value)>, globals: Rc<GlobalsMap>,
src_builtins: Vec<(&'static str, &'static str)>,
env: Option<&HashMap<SmolStr, Value>>, env: Option<&HashMap<SmolStr, Value>>,
enable_import: bool,
compiler_observer: &mut dyn CompilerObserver, compiler_observer: &mut dyn CompilerObserver,
) -> Option<(Rc<Lambda>, Rc<GlobalsMap>)> { ) -> Option<Rc<Lambda>> {
let parsed = rnix::ast::Root::parse(code); let parsed = rnix::ast::Root::parse(code);
let parse_errors = parsed.errors(); let parse_errors = parsed.errors();
@ -515,13 +513,10 @@ fn parse_compile_internal(
// the result, in case the caller needs it for something. // the result, in case the caller needs it for something.
result.expr = parsed.tree().expr(); result.expr = parsed.tree().expr();
let builtins =
crate::compiler::prepare_globals(builtins, src_builtins, source.clone(), enable_import);
let compiler_result = match compiler::compile( let compiler_result = match compiler::compile(
result.expr.as_ref().unwrap(), result.expr.as_ref().unwrap(),
location, location,
builtins, globals,
env, env,
&source, &source,
&file, &file,
@ -545,5 +540,5 @@ fn parse_compile_internal(
// Return the lambda (for execution) and the globals map (to // Return the lambda (for execution) and the globals map (to
// ensure the invariant that the globals outlive the runtime). // ensure the invariant that the globals outlive the runtime).
Some((compiler_result.lambda, compiler_result.globals)) Some(compiler_result.lambda)
} }