From 43b9e25025eef302369ff27074bfa5bbfb1c7115 Mon Sep 17 00:00:00 2001
From: Florian Klink <flokli@flokli.de>
Date: Tue, 16 Jan 2024 15:35:44 +0200
Subject: refactor(tvix/eval): move Evaluation::{default,new_pure}() again

Have a Evaluation::new() function that's used to set up the Evaluation
struct initially - which is also used by both new_pure and new_impure
internally.

It's generic over the exact type of IO, making it easier to instantiate
Evaluation with non-tvix-eval EvalIO implementations, that might not be
in a Box.

Change-Id: Ibf728da24aca59639c5b6df58d00ae98c99a63f5
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10640
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
---
 tvix/eval/benches/eval.rs       |  2 +-
 tvix/eval/src/lib.rs            | 30 ++++++++++++++++--------------
 tvix/eval/src/tests/mod.rs      |  7 ++-----
 tvix/eval/src/tests/one_offs.rs |  2 +-
 tvix/eval/tests/nix_oracle.rs   |  2 +-
 5 files changed, 21 insertions(+), 22 deletions(-)

(limited to 'tvix/eval')

diff --git a/tvix/eval/benches/eval.rs b/tvix/eval/benches/eval.rs
index 4e9be8f8f20c..57d4eb71b59f 100644
--- a/tvix/eval/benches/eval.rs
+++ b/tvix/eval/benches/eval.rs
@@ -2,7 +2,7 @@ use criterion::{black_box, criterion_group, criterion_main, Criterion};
 use itertools::Itertools;
 
 fn interpret(code: &str) {
-    tvix_eval::Evaluation::default().evaluate(code, None);
+    tvix_eval::Evaluation::new_pure().evaluate(code, None);
 }
 
 fn eval_literals(c: &mut Criterion) {
diff --git a/tvix/eval/src/lib.rs b/tvix/eval/src/lib.rs
index 72cfefee496d..5d854fcc5f07 100644
--- a/tvix/eval/src/lib.rs
+++ b/tvix/eval/src/lib.rs
@@ -131,21 +131,21 @@ pub struct EvaluationResult {
     pub expr: Option<rnix::ast::Expr>,
 }
 
-/// TODO: this approach of creating the struct, then mutating its values
-/// unnecessarily restricts the type of IO (b/262)
-impl<'co, 'ro> Default for Evaluation<'co, 'ro, Box<dyn EvalIO>> {
-    fn default() -> Self {
-        let source_map = SourceCode::default();
-
+impl<'co, 'ro, IO> Evaluation<'co, 'ro, IO>
+where
+    IO: AsRef<dyn EvalIO> + 'static,
+{
+    /// Initialize an `Evaluation`.
+    pub fn new(io_handle: IO, enable_import: bool) -> Self {
         let mut builtins = builtins::pure_builtins();
         builtins.extend(builtins::placeholders()); // these are temporary
 
         Self {
-            source_map,
+            source_map: SourceCode::default(),
+            enable_import,
+            io_handle,
             builtins,
             src_builtins: vec![],
-            io_handle: Box::new(DummyIO {}) as Box<dyn EvalIO>,
-            enable_import: false,
             strict: false,
             nix_path: None,
             compiler_observer: None,
@@ -158,16 +158,18 @@ impl<'co, 'ro> Evaluation<'co, 'ro, Box<dyn EvalIO>> {
     #[cfg(feature = "impure")]
     /// Initialise an `Evaluation`, with all impure features turned on by default.
     pub fn new_impure() -> Self {
-        let mut eval = Self {
-            enable_import: true,
-            io_handle: Box::new(StdIO),
-            ..Default::default()
-        };
+        let mut eval = Self::new(Box::new(StdIO) as Box<dyn EvalIO>, true);
 
         eval.builtins.extend(builtins::impure_builtins());
 
         eval
     }
+
+    /// Initialize an `Evaluation`, without the import statement available, and
+    /// all IO operations stubbed out.
+    pub fn new_pure() -> Self {
+        Self::new(Box::new(DummyIO) as Box<dyn EvalIO>, false)
+    }
 }
 
 impl<'co, 'ro, IO> Evaluation<'co, 'ro, IO>
diff --git a/tvix/eval/src/tests/mod.rs b/tvix/eval/src/tests/mod.rs
index e3d3445a45ac..d8b593ae9967 100644
--- a/tvix/eval/src/tests/mod.rs
+++ b/tvix/eval/src/tests/mod.rs
@@ -117,11 +117,8 @@ fn eval_test(code_path: PathBuf, expect_success: bool) {
 fn identity(#[files("src/tests/tvix_tests/identity-*.nix")] code_path: PathBuf) {
     let code = std::fs::read_to_string(code_path).expect("should be able to read test code");
 
-    let eval = crate::Evaluation {
-        strict: true,
-        io_handle: Box::new(crate::StdIO) as Box<dyn EvalIO>,
-        ..Default::default()
-    };
+    let mut eval = crate::Evaluation::new(Box::new(crate::StdIO) as Box<dyn EvalIO>, false);
+    eval.strict = true;
 
     let result = eval.evaluate(&code, None);
     assert!(
diff --git a/tvix/eval/src/tests/one_offs.rs b/tvix/eval/src/tests/one_offs.rs
index 6024058a1296..565d1dd48f77 100644
--- a/tvix/eval/src/tests/one_offs.rs
+++ b/tvix/eval/src/tests/one_offs.rs
@@ -25,7 +25,7 @@ fn test_source_builtin() {
 
 #[test]
 fn skip_broken_bytecode() {
-    let result = Evaluation::default().evaluate(/* code = */ "x", None);
+    let result = Evaluation::new_pure().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 670926d9c8e4..6bab75cfd979 100644
--- a/tvix/eval/tests/nix_oracle.rs
+++ b/tvix/eval/tests/nix_oracle.rs
@@ -51,7 +51,7 @@ 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::default();
+    let mut eval = tvix_eval::Evaluation::new_pure();
     eval.strict = matches!(strictness, Strictness::Strict);
     eval.io_handle = Box::new(tvix_eval::StdIO);
 
-- 
cgit 1.4.1