feat(tvix/eval): Allow passing in an env to evaluation

Allow passing in a top-level env, a map from name to value, to
evaluation. The intent is to support bound identifiers in the REPL just
like upstream nix does.

Getting this working involves mucking around a bit with internals - most
notably, locals now only optionally have a Span (since locals don't have
an easy span we can use) - and getting that working requires propagating
some minor hacks to places where we currently *need* a span (and which
would require too much changing now to make spans optional; my guess is
that that would essentially end up making spans optional throughout the
codebase).

Also, some extra care has to be taken to close out the scope in the case
that we do pass in an env, to avoid breaking our assumptions about the
size of the stack when we return from the toplevel

Change-Id: Ie475b2d3dfc72ccbf298d2a3ea28c63ac877d653
Reviewed-on: https://cl.tvl.fyi/c/depot/+/11953
Tested-by: BuildkiteCI
Autosubmit: aspen <root@gws.fyi>
Reviewed-by: flokli <flokli@flokli.de>
This commit is contained in:
Aspen Smith 2024-07-04 23:39:51 -04:00 committed by clbot
parent af933c177a
commit ac3d717944
7 changed files with 99 additions and 15 deletions

View file

@ -9,6 +9,8 @@ use std::iter::Peekable;
use rnix::ast::HasEntry; use rnix::ast::HasEntry;
use rowan::ast::AstChildren; use rowan::ast::AstChildren;
use crate::spans::{EntireFile, OrEntireFile};
use super::*; use super::*;
type PeekableAttrs = Peekable<AstChildren<ast::Attr>>; type PeekableAttrs = Peekable<AstChildren<ast::Attr>>;
@ -556,6 +558,15 @@ impl Compiler<'_, '_> {
self.scope_mut().end_scope(); self.scope_mut().end_scope();
} }
/// Emit definitions for all variables in the top-level global env passed to the evaluation (eg
/// local variables in the REPL)
pub(super) fn compile_env(&mut self, env: &HashMap<SmolStr, Value>) {
for (name, value) in env {
self.scope_mut().declare_constant(name.to_string());
self.emit_constant(value.clone(), &EntireFile);
}
}
/// Actually binds all tracked bindings by emitting the bytecode that places /// Actually binds all tracked bindings by emitting the bytecode that places
/// them in their stack slots. /// them in their stack slots.
fn bind_values(&mut self, bindings: TrackedBindings) { fn bind_values(&mut self, bindings: TrackedBindings) {
@ -569,7 +580,7 @@ impl Compiler<'_, '_> {
KeySlot::Static { slot, name } => { KeySlot::Static { slot, name } => {
let span = self.scope()[slot].span; let span = self.scope()[slot].span;
self.emit_constant(name.as_str().into(), &span); self.emit_constant(name.as_str().into(), &OrEntireFile(span));
self.scope_mut().mark_initialised(slot); self.scope_mut().mark_initialised(slot);
} }
@ -621,7 +632,7 @@ impl Compiler<'_, '_> {
if self.scope()[idx].needs_finaliser { if self.scope()[idx].needs_finaliser {
let stack_idx = self.scope().stack_index(idx); let stack_idx = self.scope().stack_index(idx);
let span = self.scope()[idx].span; let span = self.scope()[idx].span;
self.push_op(OpCode::OpFinalise(stack_idx), &span); self.push_op(OpCode::OpFinalise(stack_idx), &OrEntireFile(span));
} }
} }
} }

View file

@ -65,6 +65,7 @@ async fn import_impl(
globals globals
.upgrade() .upgrade()
.expect("globals dropped while still in use"), .expect("globals dropped while still in use"),
None,
&source, &source,
&file, &file,
&mut NoOpObserver::default(), &mut NoOpObserver::default(),

View file

@ -192,6 +192,7 @@ impl<'source, 'observer> Compiler<'source, 'observer> {
pub(crate) fn new( pub(crate) fn new(
location: Option<PathBuf>, location: Option<PathBuf>,
globals: Rc<GlobalsMap>, globals: Rc<GlobalsMap>,
env: Option<&HashMap<SmolStr, Value>>,
source: &'source SourceCode, source: &'source SourceCode,
file: &'source codemap::File, file: &'source codemap::File,
observer: &'observer mut dyn CompilerObserver, observer: &'observer mut dyn CompilerObserver,
@ -227,7 +228,7 @@ impl<'source, 'observer> Compiler<'source, 'observer> {
#[cfg(not(target_arch = "wasm32"))] #[cfg(not(target_arch = "wasm32"))]
debug_assert!(root_dir.is_absolute()); debug_assert!(root_dir.is_absolute());
Ok(Self { let mut compiler = Self {
root_dir, root_dir,
source, source,
file, file,
@ -237,7 +238,13 @@ impl<'source, 'observer> Compiler<'source, 'observer> {
warnings: vec![], warnings: vec![],
errors: vec![], errors: vec![],
dead_scope: 0, dead_scope: 0,
}) };
if let Some(env) = env {
compiler.compile_env(env);
}
Ok(compiler)
} }
} }
@ -1548,6 +1555,7 @@ fn compile_src_builtin(
&parsed.tree().expr().unwrap(), &parsed.tree().expr().unwrap(),
None, None,
weak.upgrade().unwrap(), weak.upgrade().unwrap(),
None,
&source, &source,
&file, &file,
&mut crate::observer::NoOpObserver {}, &mut crate::observer::NoOpObserver {},
@ -1651,11 +1659,12 @@ pub fn compile(
expr: &ast::Expr, expr: &ast::Expr,
location: Option<PathBuf>, location: Option<PathBuf>,
globals: Rc<GlobalsMap>, globals: Rc<GlobalsMap>,
env: Option<&HashMap<SmolStr, Value>>,
source: &SourceCode, source: &SourceCode,
file: &codemap::File, file: &codemap::File,
observer: &mut dyn CompilerObserver, observer: &mut dyn CompilerObserver,
) -> EvalResult<CompilationOutput> { ) -> EvalResult<CompilationOutput> {
let mut c = Compiler::new(location, globals.clone(), source, file, observer)?; let mut c = Compiler::new(location, globals.clone(), env, source, file, observer)?;
let root_span = c.span_for(expr); let root_span = c.span_for(expr);
let root_slot = c.scope_mut().declare_phantom(root_span, false); let root_slot = c.scope_mut().declare_phantom(root_span, false);
@ -1666,6 +1675,11 @@ pub fn compile(
// unevaluated state (though in practice, a value *containing* a // unevaluated state (though in practice, a value *containing* a
// thunk might be returned). // thunk might be returned).
c.emit_force(expr); c.emit_force(expr);
if let Some(env) = env {
if !env.is_empty() {
c.push_op(OpCode::OpCloseScope(Count(env.len())), &root_span);
}
}
c.push_op(OpCode::OpReturn, &root_span); c.push_op(OpCode::OpReturn, &root_span);
let lambda = Rc::new(c.contexts.pop().unwrap().lambda); let lambda = Rc::new(c.contexts.pop().unwrap().lambda);

View file

@ -38,7 +38,7 @@ pub struct Local {
name: LocalName, name: LocalName,
/// Source span at which this local was declared. /// Source span at which this local was declared.
pub span: codemap::Span, pub span: Option<codemap::Span>,
/// Scope depth of this local. /// Scope depth of this local.
pub depth: usize, pub depth: usize,
@ -73,6 +73,10 @@ impl Local {
LocalName::Phantom => false, LocalName::Phantom => false,
} }
} }
pub fn is_used(&self) -> bool {
self.depth == 0 || self.used || self.is_ignored()
}
} }
/// Represents the current position of an identifier as resolved in a scope. /// Represents the current position of an identifier as resolved in a scope.
@ -240,7 +244,7 @@ impl Scope {
let idx = self.locals.len(); let idx = self.locals.len();
self.locals.push(Local { self.locals.push(Local {
initialised, initialised,
span, span: Some(span),
name: LocalName::Phantom, name: LocalName::Phantom,
depth: self.scope_depth, depth: self.scope_depth,
needs_finaliser: false, needs_finaliser: false,
@ -263,7 +267,7 @@ impl Scope {
let idx = LocalIdx(self.locals.len()); let idx = LocalIdx(self.locals.len());
self.locals.push(Local { self.locals.push(Local {
name: LocalName::Ident(name.clone()), name: LocalName::Ident(name.clone()),
span, span: Some(span),
depth: self.scope_depth, depth: self.scope_depth,
initialised: false, initialised: false,
needs_finaliser: false, needs_finaliser: false,
@ -286,6 +290,23 @@ impl Scope {
(idx, shadowed) (idx, shadowed)
} }
pub fn declare_constant(&mut self, name: String) -> LocalIdx {
let idx = LocalIdx(self.locals.len());
self.locals.push(Local {
name: LocalName::Ident(name.clone()),
span: None,
depth: 0,
initialised: true,
used: false,
needs_finaliser: false,
must_thunk: false,
});
// We don't need to worry about shadowing for constants; they're defined at the toplevel
// always
self.by_name.insert(name, ByName::Single(idx));
idx
}
/// Mark local as initialised after compiling its expression. /// Mark local as initialised after compiling its expression.
pub fn mark_initialised(&mut self, idx: LocalIdx) { pub fn mark_initialised(&mut self, idx: LocalIdx) {
self.locals[idx.0].initialised = true; self.locals[idx.0].initialised = true;
@ -348,8 +369,8 @@ impl Scope {
// lifetime, and emit a warning otherwise (unless the // lifetime, and emit a warning otherwise (unless the
// user explicitly chose to ignore it by prefixing the // user explicitly chose to ignore it by prefixing the
// identifier with `_`) // identifier with `_`)
if !local.used && !local.is_ignored() { if local.is_used() {
unused_spans.push(local.span); unused_spans.extend(local.span);
} }
// remove the by-name index if this was a named local // remove the by-name index if this was a named local

View file

@ -109,7 +109,7 @@ pub enum ErrorKind {
UnknownDynamicVariable(String), UnknownDynamicVariable(String),
/// User is defining the same variable twice at the same depth. /// User is defining the same variable twice at the same depth.
VariableAlreadyDefined(Span), VariableAlreadyDefined(Option<Span>),
/// Attempt to call something that is not callable. /// Attempt to call something that is not callable.
NotCallable(&'static str), NotCallable(&'static str),

View file

@ -36,6 +36,7 @@ mod test_utils;
#[cfg(test)] #[cfg(test)]
mod tests; mod tests;
use std::collections::HashMap;
use std::path::PathBuf; use std::path::PathBuf;
use std::rc::Rc; use std::rc::Rc;
use std::str::FromStr; use std::str::FromStr;
@ -56,6 +57,7 @@ pub use crate::value::{NixContext, NixContextElement};
pub use crate::vm::generators; pub use crate::vm::generators;
pub use crate::warnings::{EvalWarning, WarningKind}; pub use crate::warnings::{EvalWarning, WarningKind};
pub use builtin_macros; pub use builtin_macros;
use smol_str::SmolStr;
pub use crate::value::{Builtin, CoercionKind, NixAttrs, NixList, NixString, Value}; pub use crate::value::{Builtin, CoercionKind, NixAttrs, NixList, NixString, Value};
@ -68,7 +70,7 @@ pub use crate::io::StdIO;
/// ///
/// Public fields are intended to be set by the caller. Setting all /// Public fields are intended to be set by the caller. Setting all
/// fields is optional. /// fields is optional.
pub struct Evaluation<'co, 'ro, IO> { 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,
@ -83,6 +85,9 @@ pub struct Evaluation<'co, 'ro, IO> {
/// be compiled and inserted in the builtins set. /// be compiled and inserted in the builtins set.
pub src_builtins: Vec<(&'static str, &'static str)>, pub src_builtins: Vec<(&'static str, &'static str)>,
/// Top-level variables to define in the evaluation
pub env: Option<&'env HashMap<SmolStr, Value>>,
/// Implementation of file-IO to use during evaluation, e.g. for /// Implementation of file-IO to use during evaluation, e.g. for
/// impure builtins. /// impure builtins.
/// ///
@ -131,7 +136,7 @@ pub struct EvaluationResult {
pub expr: Option<rnix::ast::Expr>, pub expr: Option<rnix::ast::Expr>,
} }
impl<'co, 'ro, IO> Evaluation<'co, 'ro, IO> impl<'co, 'ro, 'env, IO> Evaluation<'co, 'ro, 'env, IO>
where where
IO: AsRef<dyn EvalIO> + 'static, IO: AsRef<dyn EvalIO> + 'static,
{ {
@ -146,6 +151,7 @@ where
io_handle, io_handle,
builtins, builtins,
src_builtins: vec![], src_builtins: vec![],
env: None,
strict: false, strict: false,
nix_path: None, nix_path: None,
compiler_observer: None, compiler_observer: None,
@ -154,7 +160,7 @@ where
} }
} }
impl<'co, 'ro> Evaluation<'co, 'ro, Box<dyn EvalIO>> { impl<'co, 'ro, 'env> Evaluation<'co, 'ro, 'env, Box<dyn EvalIO>> {
/// Initialize an `Evaluation`, without the import statement available, and /// Initialize an `Evaluation`, without the import statement available, and
/// all IO operations stubbed out. /// all IO operations stubbed out.
pub fn new_pure() -> Self { pub fn new_pure() -> Self {
@ -188,7 +194,7 @@ impl<'co, 'ro> Evaluation<'co, 'ro, Box<dyn EvalIO>> {
} }
} }
impl<'co, 'ro, IO> Evaluation<'co, 'ro, IO> impl<'co, 'ro, 'env, IO> Evaluation<'co, 'ro, 'env, IO>
where where
IO: AsRef<dyn EvalIO> + 'static, IO: AsRef<dyn EvalIO> + 'static,
{ {
@ -229,6 +235,7 @@ where
source, source,
self.builtins, self.builtins,
self.src_builtins, self.src_builtins,
self.env,
self.enable_import, self.enable_import,
compiler_observer, compiler_observer,
); );
@ -270,6 +277,7 @@ where
source.clone(), source.clone(),
self.builtins, self.builtins,
self.src_builtins, self.src_builtins,
self.env,
self.enable_import, self.enable_import,
compiler_observer, compiler_observer,
) { ) {
@ -341,6 +349,7 @@ fn parse_compile_internal(
source: SourceCode, source: SourceCode,
builtins: Vec<(&'static str, Value)>, builtins: Vec<(&'static str, Value)>,
src_builtins: Vec<(&'static str, &'static str)>, src_builtins: Vec<(&'static str, &'static str)>,
env: Option<&HashMap<SmolStr, Value>>,
enable_import: bool, enable_import: bool,
compiler_observer: &mut dyn CompilerObserver, compiler_observer: &mut dyn CompilerObserver,
) -> Option<(Rc<Lambda>, Rc<GlobalsMap>)> { ) -> Option<(Rc<Lambda>, Rc<GlobalsMap>)> {
@ -368,6 +377,7 @@ fn parse_compile_internal(
result.expr.as_ref().unwrap(), result.expr.as_ref().unwrap(),
location, location,
builtins, builtins,
env,
&source, &source,
&file, &file,
compiler_observer, compiler_observer,

View file

@ -35,6 +35,33 @@ impl ToSpan for rnix::SyntaxNode {
} }
} }
/// A placeholder [`ToSpan`] implementation covering the entire source file.
#[derive(Debug, Clone, Copy)]
pub struct EntireFile;
impl ToSpan for EntireFile {
fn span_for(&self, file: &File) -> Span {
file.span
}
}
/// A placeholder [`ToSpan`] implementation which falls back to the entire file if its wrapped value
/// is [`None`]
#[derive(Debug, Clone, Copy)]
pub struct OrEntireFile<T>(pub Option<T>);
impl<T> ToSpan for OrEntireFile<T>
where
T: ToSpan,
{
fn span_for(&self, file: &File) -> Span {
match &self.0 {
Some(t) => t.span_for(file),
None => EntireFile.span_for(file),
}
}
}
/// Generates a `ToSpan` implementation for a type implementing /// Generates a `ToSpan` implementation for a type implementing
/// `rowan::AstNode`. This is impossible to do as a blanket /// `rowan::AstNode`. This is impossible to do as a blanket
/// implementation because `rustc` forbids these implementations for /// implementation because `rustc` forbids these implementations for