about summary refs log tree commit diff
diff options
context:
space:
mode:
authorAspen Smith <root@gws.fyi>2024-08-11T15·15-0400
committerclbot <clbot@tvl.fyi>2024-10-12T12·27+0000
commitb7a6fc2812f3ed281ba1a0b985a2ae150095f7b1 (patch)
tree8a7703be32a79fc8a6faa03385469efa24fe8be4
parent934e03c0deb88b3a0cff2e407e28f134c29657af (diff)
refactor(tvix/eval): Make `strict` an EvalMode enum r/8796
Refactor the `strict` boolean passed into evaluation at the top-level to
be a (two-variant, so far) EvalMode enum of Lazy and Strict.

This is more explicit than a boolean, and if we ever add more EvalModes
it's a simple extension of the enum.

Change-Id: I3de50e74ec971011664f6cd0999d08b792118410
Reviewed-on: https://cl.tvl.fyi/c/depot/+/12186
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
Autosubmit: aspen <root@gws.fyi>
-rw-r--r--tvix/cli/src/lib.rs7
-rw-r--r--tvix/cli/src/main.rs7
-rw-r--r--tvix/eval/src/lib.rs28
-rw-r--r--tvix/eval/src/tests/nix_tests.rs8
-rw-r--r--tvix/eval/src/vm/mod.rs19
-rw-r--r--tvix/eval/tests/nix_oracle.rs4
-rw-r--r--tvix/glue/src/tests/mod.rs4
-rw-r--r--tvix/serde/src/de.rs4
8 files changed, 50 insertions, 31 deletions
diff --git a/tvix/cli/src/lib.rs b/tvix/cli/src/lib.rs
index beb4c505207c..09ab62280945 100644
--- a/tvix/cli/src/lib.rs
+++ b/tvix/cli/src/lib.rs
@@ -10,7 +10,7 @@ use tvix_build::buildservice;
 use tvix_eval::{
     builtins::impure_builtins,
     observer::{DisassemblingObserver, TracingObserver},
-    ErrorKind, EvalIO, GlobalsMap, SourceCode, Value,
+    ErrorKind, EvalIO, EvalMode, GlobalsMap, SourceCode, Value,
 };
 use tvix_glue::{
     builtins::{add_derivation_builtins, add_fetcher_builtins, add_import_builtins},
@@ -101,9 +101,12 @@ pub fn evaluate(
         tvix_store_io.clone() as Rc<dyn EvalIO>,
     )) as Box<dyn EvalIO>)
     .enable_import()
-    .with_strict(args.strict)
     .env(env);
 
+    if args.strict {
+        eval_builder = eval_builder.mode(EvalMode::Strict);
+    }
+
     match globals {
         Some(globals) => {
             eval_builder = eval_builder.with_globals(globals);
diff --git a/tvix/cli/src/main.rs b/tvix/cli/src/main.rs
index 0bac75e0f85e..338486195e3d 100644
--- a/tvix/cli/src/main.rs
+++ b/tvix/cli/src/main.rs
@@ -6,6 +6,7 @@ use tvix_cli::args::Args;
 use tvix_cli::repl::Repl;
 use tvix_cli::{init_io_handle, interpret, AllowIncomplete};
 use tvix_eval::observer::DisassemblingObserver;
+use tvix_eval::EvalMode;
 use tvix_glue::tvix_store_io::TvixStoreIO;
 
 #[global_allocator]
@@ -14,7 +15,11 @@ static GLOBAL: MiMalloc = MiMalloc;
 /// Interpret the given code snippet, but only run the Tvix compiler
 /// on it and return errors and warnings.
 fn lint(code: &str, path: Option<PathBuf>, args: &Args) -> bool {
-    let mut eval_builder = tvix_eval::Evaluation::builder_impure().with_strict(args.strict);
+    let mut eval_builder = tvix_eval::Evaluation::builder_impure();
+
+    if args.strict {
+        eval_builder = eval_builder.mode(EvalMode::Strict);
+    }
 
     let source_map = eval_builder.source_map().clone();
 
diff --git a/tvix/eval/src/lib.rs b/tvix/eval/src/lib.rs
index 6897e1dd494f..a4ef3f01e40a 100644
--- a/tvix/eval/src/lib.rs
+++ b/tvix/eval/src/lib.rs
@@ -53,7 +53,7 @@ pub use crate::io::{DummyIO, EvalIO, FileType};
 pub use crate::pretty_ast::pretty_print_expr;
 pub use crate::source::SourceCode;
 pub use crate::value::{NixContext, NixContextElement};
-pub use crate::vm::generators;
+pub use crate::vm::{generators, EvalMode};
 pub use crate::warnings::{EvalWarning, WarningKind};
 pub use builtin_macros;
 use smol_str::SmolStr;
@@ -89,7 +89,7 @@ pub struct EvaluationBuilder<'co, 'ro, 'env, IO> {
     env: Option<&'env FxHashMap<SmolStr, Value>>,
     io_handle: IO,
     enable_import: bool,
-    strict: bool,
+    mode: EvalMode,
     nix_path: Option<String>,
     compiler_observer: Option<&'co mut dyn CompilerObserver>,
     runtime_observer: Option<&'ro mut dyn RuntimeObserver>,
@@ -134,7 +134,7 @@ where
             globals,
             env: self.env,
             io_handle: self.io_handle,
-            strict: self.strict,
+            mode: self.mode,
             nix_path: self.nix_path,
             compiler_observer: self.compiler_observer,
             runtime_observer: self.runtime_observer,
@@ -158,7 +158,7 @@ impl<'co, 'ro, 'env, IO> EvaluationBuilder<'co, 'ro, 'env, IO> {
                 src_builtins: vec![],
             }),
             env: None,
-            strict: false,
+            mode: Default::default(),
             nix_path: None,
             compiler_observer: None,
             runtime_observer: None,
@@ -172,7 +172,7 @@ impl<'co, 'ro, 'env, IO> EvaluationBuilder<'co, 'ro, 'env, IO> {
             globals: self.globals,
             env: self.env,
             enable_import: self.enable_import,
-            strict: self.strict,
+            mode: self.mode,
             nix_path: self.nix_path,
             compiler_observer: self.compiler_observer,
             runtime_observer: self.runtime_observer,
@@ -252,12 +252,8 @@ impl<'co, 'ro, 'env, IO> EvaluationBuilder<'co, 'ro, 'env, IO> {
         }
     }
 
-    pub fn with_strict(self, strict: bool) -> Self {
-        Self { strict, ..self }
-    }
-
-    pub fn strict(self) -> Self {
-        self.with_strict(true)
+    pub fn mode(self, mode: EvalMode) -> Self {
+        Self { mode, ..self }
     }
 
     pub fn nix_path(self, nix_path: Option<String>) -> Self {
@@ -360,10 +356,10 @@ pub struct Evaluation<'co, 'ro, 'env, IO> {
     /// Defaults to [`DummyIO`] if not set explicitly.
     io_handle: IO,
 
-    /// Determines whether the returned value should be strictly
-    /// evaluated, that is whether its list and attribute set elements
-    /// should be forced recursively.
-    strict: bool,
+    /// Specification for how to handle top-level values returned by evaluation
+    ///
+    /// See the documentation for [`EvalMode`] for more information.
+    mode: EvalMode,
 
     /// (optional) Nix search path, e.g. the value of `NIX_PATH` used
     /// for resolving items on the search path (such as `<nixpkgs>`).
@@ -534,7 +530,7 @@ where
             source.clone(),
             self.globals,
             lambda,
-            self.strict,
+            self.mode,
         );
 
         match vm_result {
diff --git a/tvix/eval/src/tests/nix_tests.rs b/tvix/eval/src/tests/nix_tests.rs
index cdaed193f206..b17fe98bc9e4 100644
--- a/tvix/eval/src/tests/nix_tests.rs
+++ b/tvix/eval/src/tests/nix_tests.rs
@@ -37,6 +37,8 @@ mod mock_builtins {
 
 #[cfg(feature = "impure")]
 fn eval_test(code_path: PathBuf, expect_success: bool) {
+    use crate::vm::EvalMode;
+
     std::env::set_var("TEST_VAR", "foo"); // for eval-okay-getenv.nix
 
     eprintln!("path: {}", code_path.display());
@@ -49,7 +51,7 @@ fn eval_test(code_path: PathBuf, expect_success: bool) {
     let code = std::fs::read_to_string(&code_path).expect("should be able to read test code");
 
     let eval = crate::Evaluation::builder_impure()
-        .strict()
+        .mode(EvalMode::Strict)
         .add_builtins(mock_builtins::builtins())
         .build();
 
@@ -125,13 +127,13 @@ fn eval_test(code_path: PathBuf, expect_success: bool) {
 #[cfg(feature = "impure")]
 #[rstest]
 fn identity(#[files("src/tests/tvix_tests/identity-*.nix")] code_path: PathBuf) {
-    use crate::EvalIO;
+    use crate::{vm::EvalMode, EvalIO};
 
     let code = std::fs::read_to_string(code_path).expect("should be able to read test code");
 
     let eval = crate::Evaluation::builder(Box::new(crate::StdIO) as Box<dyn EvalIO>)
         .disable_import()
-        .strict()
+        .mode(EvalMode::Strict)
         .build();
 
     let result = eval.evaluate(&code, None);
diff --git a/tvix/eval/src/vm/mod.rs b/tvix/eval/src/vm/mod.rs
index 49e9fc5864be..0630ed4174e8 100644
--- a/tvix/eval/src/vm/mod.rs
+++ b/tvix/eval/src/vm/mod.rs
@@ -1378,6 +1378,18 @@ async fn final_deep_force(co: GenCo) -> Result<Value, ErrorKind> {
     Ok(generators::request_deep_force(&co, value).await)
 }
 
+/// Specification for how to handle top-level values returned by evaluation
+#[derive(Debug, Clone, Copy, Default)]
+pub enum EvalMode {
+    /// The default. Values are returned from evaluations as-is, without any extra forcing or
+    /// special handling.
+    #[default]
+    Lazy,
+
+    /// Strictly and deeply evaluate top-level values returned by evaluation.
+    Strict,
+}
+
 pub fn run_lambda<IO>(
     nix_search_path: NixSearchPath,
     io_handle: IO,
@@ -1385,7 +1397,7 @@ pub fn run_lambda<IO>(
     source: SourceCode,
     globals: Rc<GlobalsMap>,
     lambda: Rc<Lambda>,
-    strict: bool,
+    mode: EvalMode,
 ) -> EvalResult<RuntimeResult>
 where
     IO: AsRef<dyn EvalIO> + 'static,
@@ -1409,8 +1421,9 @@ where
 
     // When evaluating strictly, synthesise a frame that will instruct
     // the VM to deep-force the final value before returning it.
-    if strict {
-        vm.enqueue_generator("final_deep_force", root_span, final_deep_force);
+    match mode {
+        EvalMode::Lazy => {}
+        EvalMode::Strict => vm.enqueue_generator("final_deep_force", root_span, final_deep_force),
     }
 
     vm.frames.push(Frame::CallFrame {
diff --git a/tvix/eval/tests/nix_oracle.rs b/tvix/eval/tests/nix_oracle.rs
index cc2a7b519e8f..137dff6b00f6 100644
--- a/tvix/eval/tests/nix_oracle.rs
+++ b/tvix/eval/tests/nix_oracle.rs
@@ -58,12 +58,12 @@ fn nix_eval(expr: &str, strictness: Strictness) -> String {
 #[track_caller]
 #[cfg(feature = "impure")]
 fn compare_eval(expr: &str, strictness: Strictness) {
-    use tvix_eval::EvalIO;
+    use tvix_eval::{EvalIO, EvalMode};
 
     let nix_result = nix_eval(expr, strictness);
     let mut eval_builder = tvix_eval::Evaluation::builder_pure();
     if matches!(strictness, Strictness::Strict) {
-        eval_builder = eval_builder.strict();
+        eval_builder = eval_builder.mode(EvalMode::Strict);
     }
     let eval = eval_builder
         .io_handle(Box::new(tvix_eval::StdIO) as Box<dyn EvalIO>)
diff --git a/tvix/glue/src/tests/mod.rs b/tvix/glue/src/tests/mod.rs
index f68ea5425899..dbde42064a77 100644
--- a/tvix/glue/src/tests/mod.rs
+++ b/tvix/glue/src/tests/mod.rs
@@ -4,7 +4,7 @@ use clap::Parser;
 use pretty_assertions::assert_eq;
 use std::path::PathBuf;
 use tvix_build::buildservice::DummyBuildService;
-use tvix_eval::{EvalIO, Value};
+use tvix_eval::{EvalIO, EvalMode, Value};
 use tvix_store::utils::{construct_services, ServiceUrlsMemory};
 
 use rstest::rstest;
@@ -54,7 +54,7 @@ fn eval_test(code_path: PathBuf, expect_success: bool) {
         tvix_store_io.clone() as Rc<dyn EvalIO>,
     )) as Box<dyn EvalIO>)
     .enable_import()
-    .strict();
+    .mode(EvalMode::Strict);
 
     eval_builder = add_derivation_builtins(eval_builder, Rc::clone(&tvix_store_io));
     eval_builder = add_fetcher_builtins(eval_builder, Rc::clone(&tvix_store_io));
diff --git a/tvix/serde/src/de.rs b/tvix/serde/src/de.rs
index 88cb46d618ce..0728311c6a13 100644
--- a/tvix/serde/src/de.rs
+++ b/tvix/serde/src/de.rs
@@ -3,7 +3,7 @@
 use bstr::ByteSlice;
 use serde::de::value::{MapDeserializer, SeqDeserializer};
 use serde::de::{self, EnumAccess, VariantAccess};
-use tvix_eval::{EvalIO, EvaluationBuilder, Value};
+use tvix_eval::{EvalIO, EvalMode, EvaluationBuilder, Value};
 
 use crate::error::Error;
 
@@ -48,7 +48,7 @@ where
     ) -> EvaluationBuilder<'co, 'ro, 'env, Box<dyn EvalIO>>,
 {
     // First step is to evaluate the Nix code ...
-    let eval = config(EvaluationBuilder::new_pure().strict()).build();
+    let eval = config(EvaluationBuilder::new_pure().mode(EvalMode::Strict)).build();
     let result = eval.evaluate(src, None);
 
     if !result.errors.is_empty() {