refactor(tvix/eval): remove code and location from struct r/7289

Instead, it's passed in the evaluate/compile_only functions, which feels
more naturally. It lets us set up the Evaluation struct long before
we actually feed it with data to evaluate.

Now that Evaluation::new() would be accepting an empty list of
arguments, we can simply implement Default, making things a bit more
idiomatic.

Change-Id: I4369658634909a0c504fdffa18242a130daa0239
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10475
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
Autosubmit: flokli <flokli@flokli.de>

4 files changed, 58 insertions, 55 deletions

diff --git a/tvix/eval/src/lib.rs b/tvix/eval/src/lib.rs
index c7c628f70e96..37db2b7bf418 100644
--- a/tvix/eval/src/lib.rs
+++ b/tvix/eval/src/lib.rs
@@ -68,21 +68,10 @@ pub use crate::io::StdIO;
 ///
 /// Public fields are intended to be set by the caller. Setting all
 /// fields is optional.
-pub struct Evaluation<'code, 'co, 'ro> {
-    /// The Nix source code to be evaluated.
-    code: &'code str,
-
-    /// Optional location of the source code (i.e. path to the file it was read
-    /// from). Used for error reporting, and for resolving relative paths in
-    /// impure functions.
-    location: Option<PathBuf>,
-
+pub struct Evaluation<'co, 'ro> {
     /// Source code map used for error reporting.
     source_map: SourceCode,
 
-    /// Top-level file reference for this code inside the source map.
-    file: Arc<codemap::File>,
-
     /// Set of all builtins that should be available during the
     /// evaluation.
     ///
@@ -142,27 +131,15 @@ pub struct EvaluationResult {
     pub expr: Option<rnix::ast::Expr>,
 }
 
-impl<'code, 'co, 'ro> Evaluation<'code, 'co, 'ro> {
-    /// Initialise an `Evaluation` for the given Nix source code snippet, and
-    /// an optional code location.
-    pub fn new(code: &'code str, location: Option<PathBuf>) -> Self {
+impl<'co, 'ro> Default for Evaluation<'co, 'ro> {
+    fn default() -> Self {
         let source_map = SourceCode::default();
 
-        let location_str = location
-            .as_ref()
-            .map(|p| p.to_string_lossy().to_string())
-            .unwrap_or_else(|| "[code]".into());
-
-        let file = source_map.add_file(location_str, code.into());
-
         let mut builtins = builtins::pure_builtins();
         builtins.extend(builtins::placeholders()); // these are temporary
 
-        Evaluation {
-            code,
-            location,
+        Self {
             source_map,
-            file,
             builtins,
             src_builtins: vec![],
             io_handle: Box::new(DummyIO {}),
@@ -173,15 +150,21 @@ impl<'code, 'co, 'ro> Evaluation<'code, 'co, 'ro> {
             runtime_observer: None,
         }
     }
+}
 
+impl<'co, 'ro> Evaluation<'co, 'ro> {
     #[cfg(feature = "impure")]
     /// Initialise an `Evaluation` for the given snippet, with all
     /// impure features turned on by default.
-    pub fn new_impure(code: &'code str, location: Option<PathBuf>) -> Self {
-        let mut eval = Self::new(code, location);
-        eval.enable_import = true;
+    pub fn new_impure() -> Self {
+        let mut eval = Self {
+            enable_import: true,
+            io_handle: Box::new(StdIO),
+            ..Default::default()
+        };
+
         eval.builtins.extend(builtins::impure_builtins());
-        eval.io_handle = Box::new(StdIO);
+
         eval
     }
 
@@ -191,21 +174,30 @@ impl<'code, 'co, 'ro> Evaluation<'code, 'co, 'ro> {
         self.source_map.clone()
     }
 
-    /// Only compile the provided source code. This does not *run* the
-    /// code, it only provides analysis (errors and warnings) of the
-    /// compiler.
-    pub fn compile_only(mut self) -> EvaluationResult {
+    /// Only compile the provided source code, at an optional location of the
+    /// source code (i.e. path to the file it was read from; used for error
+    /// reporting, and for resolving relative paths in impure functions)
+    /// This does not *run* the code, it only provides analysis (errors and
+    /// warnings) of the compiler.
+    pub fn compile_only(mut self, code: &str, location: Option<PathBuf>) -> EvaluationResult {
         let mut result = EvaluationResult::default();
         let source = self.source_map();
 
+        let location_str = location
+            .as_ref()
+            .map(|p| p.to_string_lossy().to_string())
+            .unwrap_or_else(|| "[code]".into());
+
+        let file = source.add_file(location_str, code.into());
+
         let mut noop_observer = observer::NoOpObserver::default();
         let compiler_observer = self.compiler_observer.take().unwrap_or(&mut noop_observer);
 
         parse_compile_internal(
             &mut result,
-            self.code,
-            self.file.clone(),
-            self.location,
+            code,
+            file,
+            location,
             source,
             self.builtins,
             self.src_builtins,
@@ -216,11 +208,20 @@ impl<'code, 'co, 'ro> Evaluation<'code, 'co, 'ro> {
         result
     }
 
-    /// Evaluate the provided source code.
-    pub fn evaluate(mut self) -> EvaluationResult {
+    /// Evaluate the provided source code, at an optional location of the source
+    /// code (i.e. path to the file it was read from; used for error reporting,
+    /// and for resolving relative paths in impure functions)
+    pub fn evaluate(mut self, code: &str, location: Option<PathBuf>) -> EvaluationResult {
         let mut result = EvaluationResult::default();
         let source = self.source_map();
 
+        let location_str = location
+            .as_ref()
+            .map(|p| p.to_string_lossy().to_string())
+            .unwrap_or_else(|| "[code]".into());
+
+        let file = source.add_file(location_str, code.into());
+
         let mut noop_observer = observer::NoOpObserver::default();
         let compiler_observer = self.compiler_observer.take().unwrap_or(&mut noop_observer);
 
@@ -231,9 +232,9 @@ impl<'code, 'co, 'ro> Evaluation<'code, 'co, 'ro> {
 
         let (lambda, globals) = match parse_compile_internal(
             &mut result,
-            self.code,
-            self.file.clone(),
-            self.location,
+            code,
+            file.clone(),
+            location,
             source,
             self.builtins,
             self.src_builtins,
@@ -255,7 +256,7 @@ impl<'code, 'co, 'ro> Evaluation<'code, 'co, 'ro> {
                 Err(err) => {
                     result.warnings.push(EvalWarning {
                         kind: WarningKind::InvalidNixPath(err.to_string()),
-                        span: self.file.span,
+                        span: file.span,
                     });
                     None
                 }
diff --git a/tvix/eval/src/tests/mod.rs b/tvix/eval/src/tests/mod.rs
index b7bea6094d44..7ec702883465 100644
--- a/tvix/eval/src/tests/mod.rs
+++ b/tvix/eval/src/tests/mod.rs
@@ -53,11 +53,11 @@ fn eval_test(code_path: &str, expect_success: bool) {
         return;
     }
 
-    let mut eval = crate::Evaluation::new_impure(&code, Some(code_path.into()));
+    let mut eval = crate::Evaluation::new_impure();
     eval.strict = true;
     eval.builtins.extend(mock_builtins::builtins());
 
-    let result = eval.evaluate();
+    let result = eval.evaluate(&code, Some(code_path.into()));
     let failed = match result.value {
         Some(Value::Catchable(_)) => true,
         _ => !result.errors.is_empty(),
@@ -106,11 +106,13 @@ fn eval_test(code_path: &str, expect_success: bool) {
 fn identity(code_path: &str) {
     let code = std::fs::read_to_string(code_path).expect("should be able to read test code");
 
-    let mut eval = crate::Evaluation::new(&code, None);
-    eval.strict = true;
-    eval.io_handle = Box::new(crate::StdIO);
+    let eval = crate::Evaluation {
+        strict: true,
+        io_handle: Box::new(crate::StdIO),
+        ..Default::default()
+    };
 
-    let result = eval.evaluate();
+    let result = eval.evaluate(&code, None);
     assert!(
         result.errors.is_empty(),
         "evaluation of identity test failed: {:?}",
diff --git a/tvix/eval/src/tests/one_offs.rs b/tvix/eval/src/tests/one_offs.rs
index 23bc9465d6bb..6024058a1296 100644
--- a/tvix/eval/src/tests/one_offs.rs
+++ b/tvix/eval/src/tests/one_offs.rs
@@ -5,10 +5,10 @@ fn test_source_builtin() {
     // Test an evaluation with a source-only builtin. The test ensures
     // that the artificially constructed thunking is correct.
 
-    let mut eval = Evaluation::new_impure("builtins.testSourceBuiltin", None);
+    let mut eval = Evaluation::new_impure();
     eval.src_builtins.push(("testSourceBuiltin", "42"));
 
-    let result = eval.evaluate();
+    let result = eval.evaluate("builtins.testSourceBuiltin", None);
     assert!(
         result.errors.is_empty(),
         "evaluation failed: {:?}",
@@ -25,7 +25,7 @@ fn test_source_builtin() {
 
 #[test]
 fn skip_broken_bytecode() {
-    let result = Evaluation::new(/* code = */ "x", None).evaluate();
+    let result = Evaluation::default().evaluate(/* code = */ "x", None);
 
     assert_eq!(result.errors.len(), 1);
 
diff --git a/tvix/eval/tests/nix_oracle.rs b/tvix/eval/tests/nix_oracle.rs
index b777d289e03b..670926d9c8e4 100644
--- a/tvix/eval/tests/nix_oracle.rs
+++ b/tvix/eval/tests/nix_oracle.rs
@@ -51,12 +51,12 @@ fn nix_eval(expr: &str, strictness: Strictness) -> String {
 #[track_caller]
 fn compare_eval(expr: &str, strictness: Strictness) {
     let nix_result = nix_eval(expr, strictness);
-    let mut eval = tvix_eval::Evaluation::new(expr, None);
+    let mut eval = tvix_eval::Evaluation::default();
     eval.strict = matches!(strictness, Strictness::Strict);
     eval.io_handle = Box::new(tvix_eval::StdIO);
 
     let tvix_result = eval
-        .evaluate()
+        .evaluate(expr, None)
         .value
         .expect("tvix evaluation should succeed")
         .to_string();