diff options
author | Florian Klink <flokli@flokli.de> | 2024-01-16T12·19+0200 |
---|---|---|
committer | flokli <flokli@flokli.de> | 2024-01-18T09·09+0000 |
commit | e0a867cabff021348cc283b25467cfd40b8eb15a (patch) | |
tree | 9b4f1fd63460ba8385b38259481a7bb32363801b /tvix | |
parent | 44d24852c3c62320cb2a4c9b9627e744c518f207 (diff) |
refactor(tvix/eval): generalize EvalIO container r/7407
Don't restrict to a Box<dyn EvalIO>. There's still one or two places where we do restrict, this will be solved by b/262. Change-Id: Ic8d927d6ea81fa12d90b1e4352f35ffaafbd1adf Reviewed-on: https://cl.tvl.fyi/c/depot/+/10639 Tested-by: BuildkiteCI Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Diffstat (limited to 'tvix')
-rw-r--r-- | tvix/eval/src/lib.rs | 19 | ||||
-rw-r--r-- | tvix/eval/src/nix_search_path.rs | 38 | ||||
-rw-r--r-- | tvix/eval/src/tests/mod.rs | 4 | ||||
-rw-r--r-- | tvix/eval/src/vm/generators.rs | 9 | ||||
-rw-r--r-- | tvix/eval/src/vm/mod.rs | 39 | ||||
-rw-r--r-- | tvix/glue/src/builtins/mod.rs | 4 | ||||
-rw-r--r-- | tvix/glue/src/lib.rs | 5 | ||||
-rw-r--r-- | tvix/serde/src/de.rs | 4 |
8 files changed, 74 insertions, 48 deletions
diff --git a/tvix/eval/src/lib.rs b/tvix/eval/src/lib.rs index f852b1a3243d..72cfefee496d 100644 --- a/tvix/eval/src/lib.rs +++ b/tvix/eval/src/lib.rs @@ -68,7 +68,7 @@ pub use crate::io::StdIO; /// /// Public fields are intended to be set by the caller. Setting all /// fields is optional. -pub struct Evaluation<'co, 'ro> { +pub struct Evaluation<'co, 'ro, IO> { /// Source code map used for error reporting. source_map: SourceCode, @@ -87,7 +87,7 @@ pub struct Evaluation<'co, 'ro> { /// impure builtins. /// /// Defaults to [`DummyIO`] if not set explicitly. - pub io_handle: Box<dyn EvalIO>, + pub io_handle: IO, /// Determines whether the `import` builtin should be made /// available. Note that this depends on the `io_handle` being @@ -131,7 +131,9 @@ pub struct EvaluationResult { pub expr: Option<rnix::ast::Expr>, } -impl<'co, 'ro> Default for Evaluation<'co, 'ro> { +/// 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(); @@ -142,7 +144,7 @@ impl<'co, 'ro> Default for Evaluation<'co, 'ro> { source_map, builtins, src_builtins: vec![], - io_handle: Box::new(DummyIO {}), + io_handle: Box::new(DummyIO {}) as Box<dyn EvalIO>, enable_import: false, strict: false, nix_path: None, @@ -152,7 +154,7 @@ impl<'co, 'ro> Default for Evaluation<'co, 'ro> { } } -impl<'co, 'ro> Evaluation<'co, 'ro> { +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 { @@ -166,7 +168,12 @@ impl<'co, 'ro> Evaluation<'co, 'ro> { eval } +} +impl<'co, 'ro, IO> Evaluation<'co, 'ro, IO> +where + IO: AsRef<dyn EvalIO> + 'static, +{ /// Clone the reference to the contained source code map. This is used after /// an evaluation for pretty error printing. pub fn source_map(&self) -> SourceCode { @@ -233,7 +240,7 @@ impl<'co, 'ro> Evaluation<'co, 'ro> { let compiler_observer = self.compiler_observer.take().unwrap_or(&mut noop_observer); // Insert a storeDir builtin *iff* a store directory is present. - if let Some(store_dir) = self.io_handle.store_dir() { + if let Some(store_dir) = self.io_handle.as_ref().store_dir() { self.builtins.push(("storeDir", store_dir.into())); } diff --git a/tvix/eval/src/nix_search_path.rs b/tvix/eval/src/nix_search_path.rs index f86cffd6d615..2f6d444475ff 100644 --- a/tvix/eval/src/nix_search_path.rs +++ b/tvix/eval/src/nix_search_path.rs @@ -68,11 +68,10 @@ impl NixSearchPathEntry { /// /// For prefixed path, an entry matches if the prefix does. // TODO(tazjin): verify these rules in the C++ impl, seems fishy. - fn resolve( - &self, - io: &mut dyn EvalIO, - lookup_path: &Path, - ) -> Result<Option<PathBuf>, ErrorKind> { + fn resolve<IO>(&self, io: IO, lookup_path: &Path) -> Result<Option<PathBuf>, ErrorKind> + where + IO: AsRef<dyn EvalIO>, + { let path = match self { NixSearchPathEntry::Path(parent) => canonicalise(parent.join(lookup_path))?, @@ -85,7 +84,7 @@ impl NixSearchPathEntry { } }; - if io.path_exists(&path).map_err(|e| ErrorKind::IO { + if io.as_ref().path_exists(&path).map_err(|e| ErrorKind::IO { path: Some(path.clone()), error: e.into(), })? { @@ -124,17 +123,18 @@ pub struct NixSearchPath { impl NixSearchPath { /// Attempt to resolve the given `path` within this [`NixSearchPath`] using the /// path resolution rules for `<...>`-style paths - pub fn resolve<P>( + pub fn resolve<P, IO>( &self, - io: &mut dyn EvalIO, + io: IO, path: P, ) -> Result<Result<PathBuf, CatchableErrorKind>, ErrorKind> where P: AsRef<Path>, + IO: AsRef<dyn EvalIO>, { let path = path.as_ref(); for entry in &self.entries { - if let Some(p) = entry.resolve(io, path)? { + if let Some(p) = entry.resolve(&io, path)? { return Ok(Ok(p)); } } @@ -204,8 +204,8 @@ mod tests { #[test] fn simple_dir() { let nix_search_path = NixSearchPath::from_str("./.").unwrap(); - let mut io = StdIO {}; - let res = nix_search_path.resolve(&mut io, "src").unwrap(); + let io = Box::new(StdIO {}) as Box<dyn EvalIO>; + let res = nix_search_path.resolve(&io, "src").unwrap(); assert_eq!( res.unwrap().to_path_buf(), current_dir().unwrap().join("src").clean() @@ -215,8 +215,8 @@ mod tests { #[test] fn failed_resolution() { let nix_search_path = NixSearchPath::from_str("./.").unwrap(); - let mut io = StdIO {}; - let err = nix_search_path.resolve(&mut io, "nope").unwrap(); + let io = Box::new(StdIO {}) as Box<dyn EvalIO>; + let err = nix_search_path.resolve(&io, "nope").unwrap(); assert!( matches!(err, Err(CatchableErrorKind::NixPathResolution(..))), "err = {err:?}" @@ -226,16 +226,16 @@ mod tests { #[test] fn second_in_path() { let nix_search_path = NixSearchPath::from_str("./.:/").unwrap(); - let mut io = StdIO {}; - let res = nix_search_path.resolve(&mut io, "etc").unwrap(); + let io = Box::new(StdIO {}) as Box<dyn EvalIO>; + let res = nix_search_path.resolve(&io, "etc").unwrap(); assert_eq!(res.unwrap().to_path_buf(), Path::new("/etc")); } #[test] fn prefix() { let nix_search_path = NixSearchPath::from_str("/:tvix=.").unwrap(); - let mut io = StdIO {}; - let res = nix_search_path.resolve(&mut io, "tvix/src").unwrap(); + let io = Box::new(StdIO {}) as Box<dyn EvalIO>; + let res = nix_search_path.resolve(&io, "tvix/src").unwrap(); assert_eq!( res.unwrap().to_path_buf(), current_dir().unwrap().join("src").clean() @@ -245,8 +245,8 @@ mod tests { #[test] fn matching_prefix() { let nix_search_path = NixSearchPath::from_str("/:tvix=.").unwrap(); - let mut io = StdIO {}; - let res = nix_search_path.resolve(&mut io, "tvix").unwrap(); + let io = Box::new(StdIO {}) as Box<dyn EvalIO>; + let res = nix_search_path.resolve(&io, "tvix").unwrap(); assert_eq!(res.unwrap().to_path_buf(), current_dir().unwrap().clean()); } } diff --git a/tvix/eval/src/tests/mod.rs b/tvix/eval/src/tests/mod.rs index 7509379fdab6..e3d3445a45ac 100644 --- a/tvix/eval/src/tests/mod.rs +++ b/tvix/eval/src/tests/mod.rs @@ -1,4 +1,4 @@ -use crate::value::Value; +use crate::{value::Value, EvalIO}; use builtin_macros::builtins; use pretty_assertions::assert_eq; use rstest::rstest; @@ -119,7 +119,7 @@ fn identity(#[files("src/tests/tvix_tests/identity-*.nix")] code_path: PathBuf) let eval = crate::Evaluation { strict: true, - io_handle: Box::new(crate::StdIO), + io_handle: Box::new(crate::StdIO) as Box<dyn EvalIO>, ..Default::default() }; diff --git a/tvix/eval/src/vm/generators.rs b/tvix/eval/src/vm/generators.rs index f2ce73a0c742..3d2fd9d266c7 100644 --- a/tvix/eval/src/vm/generators.rs +++ b/tvix/eval/src/vm/generators.rs @@ -224,7 +224,10 @@ pub fn pin_generator( Box::pin(f) } -impl<'o> VM<'o> { +impl<'o, IO> VM<'o, IO> +where + IO: AsRef<dyn EvalIO> + 'static, +{ /// Helper function to re-enqueue the current generator while it /// is awaiting a value. fn reenqueue_generator(&mut self, name: &'static str, span: LightSpan, generator: Generator) { @@ -411,6 +414,7 @@ impl<'o> VM<'o> { VMRequest::PathImport(path) => { let imported = self .io_handle + .as_ref() .import_path(&path) .map_err(|e| ErrorKind::IO { path: Some(path), @@ -424,6 +428,7 @@ impl<'o> VM<'o> { VMRequest::ReadToString(path) => { let content = self .io_handle + .as_ref() .read_to_string(&path) .map_err(|e| ErrorKind::IO { path: Some(path), @@ -437,6 +442,7 @@ impl<'o> VM<'o> { VMRequest::PathExists(path) => { let exists = self .io_handle + .as_ref() .path_exists(&path) .map_err(|e| ErrorKind::IO { path: Some(path), @@ -451,6 +457,7 @@ impl<'o> VM<'o> { VMRequest::ReadDir(path) => { let dir = self .io_handle + .as_ref() .read_dir(&path) .map_err(|e| ErrorKind::IO { path: Some(path), diff --git a/tvix/eval/src/vm/mod.rs b/tvix/eval/src/vm/mod.rs index 398a02cc78c8..5238fdc0662f 100644 --- a/tvix/eval/src/vm/mod.rs +++ b/tvix/eval/src/vm/mod.rs @@ -48,7 +48,7 @@ trait GetSpan { fn get_span(self) -> Span; } -impl<'o> GetSpan for &VM<'o> { +impl<'o, IO> GetSpan for &VM<'o, IO> { fn get_span(self) -> Span { self.reasonable_span.span() } @@ -75,12 +75,12 @@ impl GetSpan for Span { /// Internal helper trait for ergonomically converting from a `Result<T, /// ErrorKind>` to a `Result<T, Error>` using the current span of a call frame, /// and chaining the VM's frame stack around it for printing a cause chain. -trait WithSpan<T, S: GetSpan> { - fn with_span(self, top_span: S, vm: &VM) -> Result<T, Error>; +trait WithSpan<T, S: GetSpan, IO> { + fn with_span(self, top_span: S, vm: &VM<IO>) -> Result<T, Error>; } -impl<T, S: GetSpan> WithSpan<T, S> for Result<T, ErrorKind> { - fn with_span(self, top_span: S, vm: &VM) -> Result<T, Error> { +impl<T, S: GetSpan, IO> WithSpan<T, S, IO> for Result<T, ErrorKind> { + fn with_span(self, top_span: S, vm: &VM<IO>) -> Result<T, Error> { match self { Ok(something) => Ok(something), Err(kind) => { @@ -149,7 +149,7 @@ impl CallFrame { /// Construct an error result from the given ErrorKind and the source span /// of the current instruction. - pub fn error<T>(&self, vm: &VM, kind: ErrorKind) -> Result<T, Error> { + pub fn error<T, IO>(&self, vm: &VM<IO>, kind: ErrorKind) -> Result<T, Error> { Err(kind).with_span(self, vm) } @@ -249,7 +249,7 @@ impl ImportCache { } } -struct VM<'o> { +struct VM<'o, IO> { /// VM's frame stack, representing the execution contexts the VM is working /// through. Elements are usually pushed when functions are called, or /// thunks are being forced. @@ -280,7 +280,7 @@ struct VM<'o> { /// Implementation of I/O operations used for impure builtins and /// features like `import`. - io_handle: Box<dyn EvalIO>, + io_handle: IO, /// Runtime observer which can print traces of runtime operations. observer: &'o mut dyn RuntimeObserver, @@ -324,10 +324,13 @@ struct VM<'o> { try_eval_frames: Vec<usize>, } -impl<'o> VM<'o> { +impl<'o, IO> VM<'o, IO> +where + IO: AsRef<dyn EvalIO> + 'static, +{ pub fn new( nix_search_path: NixSearchPath, - io_handle: Box<dyn EvalIO>, + io_handle: IO, observer: &'o mut dyn RuntimeObserver, globals: Rc<GlobalsMap>, reasonable_span: LightSpan, @@ -858,7 +861,7 @@ impl<'o> VM<'o> { Value::UnresolvedPath(path) => { let resolved = self .nix_search_path - .resolve(&mut *self.io_handle, *path) + .resolve(&self.io_handle, *path) .with_span(&frame, self)?; self.stack.push(resolved.into()); } @@ -914,7 +917,10 @@ impl<'o> VM<'o> { } /// Implementation of helper functions for the runtime logic above. -impl<'o> VM<'o> { +impl<'o, IO> VM<'o, IO> +where + IO: AsRef<dyn EvalIO> + 'static, +{ pub(crate) fn stack_pop(&mut self) -> Value { self.stack.pop().expect("runtime stack empty") } @@ -1301,14 +1307,17 @@ async fn final_deep_force(co: GenCo) -> Result<Value, ErrorKind> { Ok(generators::request_deep_force(&co, value).await) } -pub fn run_lambda( +pub fn run_lambda<IO>( nix_search_path: NixSearchPath, - io_handle: Box<dyn EvalIO>, + io_handle: IO, observer: &mut dyn RuntimeObserver, globals: Rc<GlobalsMap>, lambda: Rc<Lambda>, strict: bool, -) -> EvalResult<RuntimeResult> { +) -> EvalResult<RuntimeResult> +where + IO: AsRef<dyn EvalIO> + 'static, +{ // Retain the top-level span of the expression in this lambda, as // synthetic "calls" in deep_force will otherwise not have a span // to fall back to. diff --git a/tvix/glue/src/builtins/mod.rs b/tvix/glue/src/builtins/mod.rs index 3656bb9d1616..22166055d067 100644 --- a/tvix/glue/src/builtins/mod.rs +++ b/tvix/glue/src/builtins/mod.rs @@ -14,8 +14,8 @@ pub use derivation_error::Error as DerivationError; /// /// As they need to interact with `known_paths`, we also need to pass in /// `known_paths`. -pub fn add_derivation_builtins( - eval: &mut tvix_eval::Evaluation, +pub fn add_derivation_builtins<IO>( + eval: &mut tvix_eval::Evaluation<IO>, known_paths: Rc<RefCell<KnownPaths>>, ) { eval.builtins diff --git a/tvix/glue/src/lib.rs b/tvix/glue/src/lib.rs index 31bcbede37d4..b2f586ce52d6 100644 --- a/tvix/glue/src/lib.rs +++ b/tvix/glue/src/lib.rs @@ -11,7 +11,10 @@ mod tests; /// Tell the Evaluator to resolve `<nix>` to the path `/__corepkgs__`, /// which has special handling in [tvix_io::TvixIO]. /// This is used in nixpkgs to import `fetchurl.nix` from `<nix>`. -pub fn configure_nix_path(eval: &mut tvix_eval::Evaluation, nix_search_path: &Option<String>) { +pub fn configure_nix_path<IO>( + eval: &mut tvix_eval::Evaluation<IO>, + nix_search_path: &Option<String>, +) { eval.nix_path = nix_search_path .as_ref() .map(|p| format!("nix=/__corepkgs__:{}", p)) diff --git a/tvix/serde/src/de.rs b/tvix/serde/src/de.rs index 500c9c98409d..4f16736af477 100644 --- a/tvix/serde/src/de.rs +++ b/tvix/serde/src/de.rs @@ -3,7 +3,7 @@ use serde::de::value::{MapDeserializer, SeqDeserializer}; use serde::de::{self, EnumAccess, VariantAccess}; pub use tvix_eval::Evaluation; -use tvix_eval::Value; +use tvix_eval::{EvalIO, Value}; use crate::error::Error; @@ -43,7 +43,7 @@ where pub fn from_str_with_config<'code, T, F>(src: &'code str, config: F) -> Result<T, Error> where T: serde::Deserialize<'code>, - F: FnOnce(&mut Evaluation), + F: FnOnce(&mut Evaluation<Box<dyn EvalIO>>), { // First step is to evaluate the Nix code ... let mut eval = Evaluation::default(); |