From 4fba57c2c90f2e7b02da9187e59f8d64deef3fb2 Mon Sep 17 00:00:00 2001 From: Florian Klink Date: Sat, 30 Dec 2023 21:36:48 +0100 Subject: refactor(tvix/eval): remove code and location from struct 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 Autosubmit: flokli --- tvix/eval/src/lib.rs | 89 +++++++++++++++++++++-------------------- tvix/eval/src/tests/mod.rs | 14 ++++--- tvix/eval/src/tests/one_offs.rs | 6 +-- tvix/eval/tests/nix_oracle.rs | 4 +- 4 files changed, 58 insertions(+), 55 deletions(-) (limited to 'tvix/eval') 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, - +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, - /// Set of all builtins that should be available during the /// evaluation. /// @@ -142,27 +131,15 @@ pub struct EvaluationResult { pub expr: Option, } -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) -> 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) -> 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) -> 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) -> 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(); -- cgit 1.4.1