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

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>

7 files changed, 99 insertions, 15 deletions

diff --git a/tvix/eval/src/compiler/bindings.rs b/tvix/eval/src/compiler/bindings.rs
index 634cc5402247..60203ba5d94b 100644
--- a/tvix/eval/src/compiler/bindings.rs
+++ b/tvix/eval/src/compiler/bindings.rs
@@ -9,6 +9,8 @@ use std::iter::Peekable;
 use rnix::ast::HasEntry;
 use rowan::ast::AstChildren;
 
+use crate::spans::{EntireFile, OrEntireFile};
+
 use super::*;
 
 type PeekableAttrs = Peekable<AstChildren<ast::Attr>>;
@@ -556,6 +558,15 @@ impl Compiler<'_, '_> {
         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
     /// them in their stack slots.
     fn bind_values(&mut self, bindings: TrackedBindings) {
@@ -569,7 +580,7 @@ impl Compiler<'_, '_> {
 
                 KeySlot::Static { slot, name } => {
                     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);
                 }
 
@@ -621,7 +632,7 @@ impl Compiler<'_, '_> {
             if self.scope()[idx].needs_finaliser {
                 let stack_idx = self.scope().stack_index(idx);
                 let span = self.scope()[idx].span;
-                self.push_op(OpCode::OpFinalise(stack_idx), &span);
+                self.push_op(OpCode::OpFinalise(stack_idx), &OrEntireFile(span));
             }
         }
     }
diff --git a/tvix/eval/src/compiler/import.rs b/tvix/eval/src/compiler/import.rs
index 9036eec81731..862e792df566 100644
--- a/tvix/eval/src/compiler/import.rs
+++ b/tvix/eval/src/compiler/import.rs
@@ -65,6 +65,7 @@ async fn import_impl(
         globals
             .upgrade()
             .expect("globals dropped while still in use"),
+        None,
         &source,
         &file,
         &mut NoOpObserver::default(),
diff --git a/tvix/eval/src/compiler/mod.rs b/tvix/eval/src/compiler/mod.rs
index 88018cce2fae..ebc59a0aa035 100644
--- a/tvix/eval/src/compiler/mod.rs
+++ b/tvix/eval/src/compiler/mod.rs
@@ -192,6 +192,7 @@ impl<'source, 'observer> Compiler<'source, 'observer> {
     pub(crate) fn new(
         location: Option<PathBuf>,
         globals: Rc<GlobalsMap>,
+        env: Option<&HashMap<SmolStr, Value>>,
         source: &'source SourceCode,
         file: &'source codemap::File,
         observer: &'observer mut dyn CompilerObserver,
@@ -227,7 +228,7 @@ impl<'source, 'observer> Compiler<'source, 'observer> {
         #[cfg(not(target_arch = "wasm32"))]
         debug_assert!(root_dir.is_absolute());
 
-        Ok(Self {
+        let mut compiler = Self {
             root_dir,
             source,
             file,
@@ -237,7 +238,13 @@ impl<'source, 'observer> Compiler<'source, 'observer> {
             warnings: vec![],
             errors: vec![],
             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(),
             None,
             weak.upgrade().unwrap(),
+            None,
             &source,
             &file,
             &mut crate::observer::NoOpObserver {},
@@ -1651,11 +1659,12 @@ pub fn compile(
     expr: &ast::Expr,
     location: Option<PathBuf>,
     globals: Rc<GlobalsMap>,
+    env: Option<&HashMap<SmolStr, Value>>,
     source: &SourceCode,
     file: &codemap::File,
     observer: &mut dyn CompilerObserver,
 ) -> 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_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
     // thunk might be returned).
     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);
 
     let lambda = Rc::new(c.contexts.pop().unwrap().lambda);
diff --git a/tvix/eval/src/compiler/scope.rs b/tvix/eval/src/compiler/scope.rs
index 892727c107c9..7b0a66004a7f 100644
--- a/tvix/eval/src/compiler/scope.rs
+++ b/tvix/eval/src/compiler/scope.rs
@@ -38,7 +38,7 @@ pub struct Local {
     name: LocalName,
 
     /// Source span at which this local was declared.
-    pub span: codemap::Span,
+    pub span: Option<codemap::Span>,
 
     /// Scope depth of this local.
     pub depth: usize,
@@ -73,6 +73,10 @@ impl Local {
             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.
@@ -240,7 +244,7 @@ impl Scope {
         let idx = self.locals.len();
         self.locals.push(Local {
             initialised,
-            span,
+            span: Some(span),
             name: LocalName::Phantom,
             depth: self.scope_depth,
             needs_finaliser: false,
@@ -263,7 +267,7 @@ impl Scope {
         let idx = LocalIdx(self.locals.len());
         self.locals.push(Local {
             name: LocalName::Ident(name.clone()),
-            span,
+            span: Some(span),
             depth: self.scope_depth,
             initialised: false,
             needs_finaliser: false,
@@ -286,6 +290,23 @@ impl Scope {
         (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.
     pub fn mark_initialised(&mut self, idx: LocalIdx) {
         self.locals[idx.0].initialised = true;
@@ -348,8 +369,8 @@ impl Scope {
                 // lifetime, and emit a warning otherwise (unless the
                 // user explicitly chose to ignore it by prefixing the
                 // identifier with `_`)
-                if !local.used && !local.is_ignored() {
-                    unused_spans.push(local.span);
+                if local.is_used() {
+                    unused_spans.extend(local.span);
                 }
 
                 // remove the by-name index if this was a named local
diff --git a/tvix/eval/src/errors.rs b/tvix/eval/src/errors.rs
index e32cfa04ed40..ee55552c7ac5 100644
--- a/tvix/eval/src/errors.rs
+++ b/tvix/eval/src/errors.rs
@@ -109,7 +109,7 @@ pub enum ErrorKind {
     UnknownDynamicVariable(String),
 
     /// User is defining the same variable twice at the same depth.
-    VariableAlreadyDefined(Span),
+    VariableAlreadyDefined(Option<Span>),
 
     /// Attempt to call something that is not callable.
     NotCallable(&'static str),
diff --git a/tvix/eval/src/lib.rs b/tvix/eval/src/lib.rs
index 398da4d6e22e..97690d13610e 100644
--- a/tvix/eval/src/lib.rs
+++ b/tvix/eval/src/lib.rs
@@ -36,6 +36,7 @@ mod test_utils;
 #[cfg(test)]
 mod tests;
 
+use std::collections::HashMap;
 use std::path::PathBuf;
 use std::rc::Rc;
 use std::str::FromStr;
@@ -56,6 +57,7 @@ pub use crate::value::{NixContext, NixContextElement};
 pub use crate::vm::generators;
 pub use crate::warnings::{EvalWarning, WarningKind};
 pub use builtin_macros;
+use smol_str::SmolStr;
 
 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
 /// fields is optional.
-pub struct Evaluation<'co, 'ro, IO> {
+pub struct Evaluation<'co, 'ro, 'env, IO> {
     /// Source code map used for error reporting.
     source_map: SourceCode,
 
@@ -83,6 +85,9 @@ pub struct Evaluation<'co, 'ro, IO> {
     /// be compiled and inserted in the builtins set.
     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
     /// impure builtins.
     ///
@@ -131,7 +136,7 @@ pub struct EvaluationResult {
     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
     IO: AsRef<dyn EvalIO> + 'static,
 {
@@ -146,6 +151,7 @@ where
             io_handle,
             builtins,
             src_builtins: vec![],
+            env: None,
             strict: false,
             nix_path: 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
     /// all IO operations stubbed out.
     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
     IO: AsRef<dyn EvalIO> + 'static,
 {
@@ -229,6 +235,7 @@ where
             source,
             self.builtins,
             self.src_builtins,
+            self.env,
             self.enable_import,
             compiler_observer,
         );
@@ -270,6 +277,7 @@ where
             source.clone(),
             self.builtins,
             self.src_builtins,
+            self.env,
             self.enable_import,
             compiler_observer,
         ) {
@@ -341,6 +349,7 @@ fn parse_compile_internal(
     source: SourceCode,
     builtins: Vec<(&'static str, Value)>,
     src_builtins: Vec<(&'static str, &'static str)>,
+    env: Option<&HashMap<SmolStr, Value>>,
     enable_import: bool,
     compiler_observer: &mut dyn CompilerObserver,
 ) -> Option<(Rc<Lambda>, Rc<GlobalsMap>)> {
@@ -368,6 +377,7 @@ fn parse_compile_internal(
         result.expr.as_ref().unwrap(),
         location,
         builtins,
+        env,
         &source,
         &file,
         compiler_observer,
diff --git a/tvix/eval/src/spans.rs b/tvix/eval/src/spans.rs
index 9998e438b220..df2b9a725562 100644
--- a/tvix/eval/src/spans.rs
+++ b/tvix/eval/src/spans.rs
@@ -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
 /// `rowan::AstNode`. This is impossible to do as a blanket
 /// implementation because `rustc` forbids these implementations for