refactor(tvix/eval): Make strict an EvalMode enum

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>
This commit is contained in:
Aspen Smith 2024-08-11 11:15:47 -04:00 committed by clbot
parent 934e03c0de
commit b7a6fc2812
8 changed files with 50 additions and 31 deletions

View file

@ -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);

View file

@ -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();

View file

@ -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 {

View file

@ -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);

View file

@ -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 {

View file

@ -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>)

View file

@ -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));

View file

@ -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() {