refactor(tvix/eval): generalize EvalIO container

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>
This commit is contained in:
Florian Klink 2024-01-16 14:19:16 +02:00 committed by flokli
parent 44d24852c3
commit e0a867cabf
8 changed files with 74 additions and 48 deletions

View file

@ -68,7 +68,7 @@ pub use crate::io::StdIO;
/// ///
/// Public fields are intended to be set by the caller. Setting all /// Public fields are intended to be set by the caller. Setting all
/// fields is optional. /// fields is optional.
pub struct Evaluation<'co, 'ro> { pub struct Evaluation<'co, 'ro, IO> {
/// Source code map used for error reporting. /// Source code map used for error reporting.
source_map: SourceCode, source_map: SourceCode,
@ -87,7 +87,7 @@ pub struct Evaluation<'co, 'ro> {
/// impure builtins. /// impure builtins.
/// ///
/// Defaults to [`DummyIO`] if not set explicitly. /// 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 /// Determines whether the `import` builtin should be made
/// available. Note that this depends on the `io_handle` being /// available. Note that this depends on the `io_handle` being
@ -131,7 +131,9 @@ pub struct EvaluationResult {
pub expr: Option<rnix::ast::Expr>, 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 { fn default() -> Self {
let source_map = SourceCode::default(); let source_map = SourceCode::default();
@ -142,7 +144,7 @@ impl<'co, 'ro> Default for Evaluation<'co, 'ro> {
source_map, source_map,
builtins, builtins,
src_builtins: vec![], src_builtins: vec![],
io_handle: Box::new(DummyIO {}), io_handle: Box::new(DummyIO {}) as Box<dyn EvalIO>,
enable_import: false, enable_import: false,
strict: false, strict: false,
nix_path: None, 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")] #[cfg(feature = "impure")]
/// Initialise an `Evaluation`, with all impure features turned on by default. /// Initialise an `Evaluation`, with all impure features turned on by default.
pub fn new_impure() -> Self { pub fn new_impure() -> Self {
@ -166,7 +168,12 @@ impl<'co, 'ro> Evaluation<'co, 'ro> {
eval 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 /// Clone the reference to the contained source code map. This is used after
/// an evaluation for pretty error printing. /// an evaluation for pretty error printing.
pub fn source_map(&self) -> SourceCode { 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); let compiler_observer = self.compiler_observer.take().unwrap_or(&mut noop_observer);
// Insert a storeDir builtin *iff* a store directory is present. // 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())); self.builtins.push(("storeDir", store_dir.into()));
} }

View file

@ -68,11 +68,10 @@ impl NixSearchPathEntry {
/// ///
/// For prefixed path, an entry matches if the prefix does. /// For prefixed path, an entry matches if the prefix does.
// TODO(tazjin): verify these rules in the C++ impl, seems fishy. // TODO(tazjin): verify these rules in the C++ impl, seems fishy.
fn resolve( fn resolve<IO>(&self, io: IO, lookup_path: &Path) -> Result<Option<PathBuf>, ErrorKind>
&self, where
io: &mut dyn EvalIO, IO: AsRef<dyn EvalIO>,
lookup_path: &Path, {
) -> Result<Option<PathBuf>, ErrorKind> {
let path = match self { let path = match self {
NixSearchPathEntry::Path(parent) => canonicalise(parent.join(lookup_path))?, 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()), path: Some(path.clone()),
error: e.into(), error: e.into(),
})? { })? {
@ -124,17 +123,18 @@ pub struct NixSearchPath {
impl NixSearchPath { impl NixSearchPath {
/// Attempt to resolve the given `path` within this [`NixSearchPath`] using the /// Attempt to resolve the given `path` within this [`NixSearchPath`] using the
/// path resolution rules for `<...>`-style paths /// path resolution rules for `<...>`-style paths
pub fn resolve<P>( pub fn resolve<P, IO>(
&self, &self,
io: &mut dyn EvalIO, io: IO,
path: P, path: P,
) -> Result<Result<PathBuf, CatchableErrorKind>, ErrorKind> ) -> Result<Result<PathBuf, CatchableErrorKind>, ErrorKind>
where where
P: AsRef<Path>, P: AsRef<Path>,
IO: AsRef<dyn EvalIO>,
{ {
let path = path.as_ref(); let path = path.as_ref();
for entry in &self.entries { 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)); return Ok(Ok(p));
} }
} }
@ -204,8 +204,8 @@ mod tests {
#[test] #[test]
fn simple_dir() { fn simple_dir() {
let nix_search_path = NixSearchPath::from_str("./.").unwrap(); let nix_search_path = NixSearchPath::from_str("./.").unwrap();
let mut io = StdIO {}; let io = Box::new(StdIO {}) as Box<dyn EvalIO>;
let res = nix_search_path.resolve(&mut io, "src").unwrap(); let res = nix_search_path.resolve(&io, "src").unwrap();
assert_eq!( assert_eq!(
res.unwrap().to_path_buf(), res.unwrap().to_path_buf(),
current_dir().unwrap().join("src").clean() current_dir().unwrap().join("src").clean()
@ -215,8 +215,8 @@ mod tests {
#[test] #[test]
fn failed_resolution() { fn failed_resolution() {
let nix_search_path = NixSearchPath::from_str("./.").unwrap(); let nix_search_path = NixSearchPath::from_str("./.").unwrap();
let mut io = StdIO {}; let io = Box::new(StdIO {}) as Box<dyn EvalIO>;
let err = nix_search_path.resolve(&mut io, "nope").unwrap(); let err = nix_search_path.resolve(&io, "nope").unwrap();
assert!( assert!(
matches!(err, Err(CatchableErrorKind::NixPathResolution(..))), matches!(err, Err(CatchableErrorKind::NixPathResolution(..))),
"err = {err:?}" "err = {err:?}"
@ -226,16 +226,16 @@ mod tests {
#[test] #[test]
fn second_in_path() { fn second_in_path() {
let nix_search_path = NixSearchPath::from_str("./.:/").unwrap(); let nix_search_path = NixSearchPath::from_str("./.:/").unwrap();
let mut io = StdIO {}; let io = Box::new(StdIO {}) as Box<dyn EvalIO>;
let res = nix_search_path.resolve(&mut io, "etc").unwrap(); let res = nix_search_path.resolve(&io, "etc").unwrap();
assert_eq!(res.unwrap().to_path_buf(), Path::new("/etc")); assert_eq!(res.unwrap().to_path_buf(), Path::new("/etc"));
} }
#[test] #[test]
fn prefix() { fn prefix() {
let nix_search_path = NixSearchPath::from_str("/:tvix=.").unwrap(); let nix_search_path = NixSearchPath::from_str("/:tvix=.").unwrap();
let mut io = StdIO {}; let io = Box::new(StdIO {}) as Box<dyn EvalIO>;
let res = nix_search_path.resolve(&mut io, "tvix/src").unwrap(); let res = nix_search_path.resolve(&io, "tvix/src").unwrap();
assert_eq!( assert_eq!(
res.unwrap().to_path_buf(), res.unwrap().to_path_buf(),
current_dir().unwrap().join("src").clean() current_dir().unwrap().join("src").clean()
@ -245,8 +245,8 @@ mod tests {
#[test] #[test]
fn matching_prefix() { fn matching_prefix() {
let nix_search_path = NixSearchPath::from_str("/:tvix=.").unwrap(); let nix_search_path = NixSearchPath::from_str("/:tvix=.").unwrap();
let mut io = StdIO {}; let io = Box::new(StdIO {}) as Box<dyn EvalIO>;
let res = nix_search_path.resolve(&mut io, "tvix").unwrap(); let res = nix_search_path.resolve(&io, "tvix").unwrap();
assert_eq!(res.unwrap().to_path_buf(), current_dir().unwrap().clean()); assert_eq!(res.unwrap().to_path_buf(), current_dir().unwrap().clean());
} }
} }

View file

@ -1,4 +1,4 @@
use crate::value::Value; use crate::{value::Value, EvalIO};
use builtin_macros::builtins; use builtin_macros::builtins;
use pretty_assertions::assert_eq; use pretty_assertions::assert_eq;
use rstest::rstest; use rstest::rstest;
@ -119,7 +119,7 @@ fn identity(#[files("src/tests/tvix_tests/identity-*.nix")] code_path: PathBuf)
let eval = crate::Evaluation { let eval = crate::Evaluation {
strict: true, strict: true,
io_handle: Box::new(crate::StdIO), io_handle: Box::new(crate::StdIO) as Box<dyn EvalIO>,
..Default::default() ..Default::default()
}; };

View file

@ -224,7 +224,10 @@ pub fn pin_generator(
Box::pin(f) 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 /// Helper function to re-enqueue the current generator while it
/// is awaiting a value. /// is awaiting a value.
fn reenqueue_generator(&mut self, name: &'static str, span: LightSpan, generator: Generator) { fn reenqueue_generator(&mut self, name: &'static str, span: LightSpan, generator: Generator) {
@ -411,6 +414,7 @@ impl<'o> VM<'o> {
VMRequest::PathImport(path) => { VMRequest::PathImport(path) => {
let imported = self let imported = self
.io_handle .io_handle
.as_ref()
.import_path(&path) .import_path(&path)
.map_err(|e| ErrorKind::IO { .map_err(|e| ErrorKind::IO {
path: Some(path), path: Some(path),
@ -424,6 +428,7 @@ impl<'o> VM<'o> {
VMRequest::ReadToString(path) => { VMRequest::ReadToString(path) => {
let content = self let content = self
.io_handle .io_handle
.as_ref()
.read_to_string(&path) .read_to_string(&path)
.map_err(|e| ErrorKind::IO { .map_err(|e| ErrorKind::IO {
path: Some(path), path: Some(path),
@ -437,6 +442,7 @@ impl<'o> VM<'o> {
VMRequest::PathExists(path) => { VMRequest::PathExists(path) => {
let exists = self let exists = self
.io_handle .io_handle
.as_ref()
.path_exists(&path) .path_exists(&path)
.map_err(|e| ErrorKind::IO { .map_err(|e| ErrorKind::IO {
path: Some(path), path: Some(path),
@ -451,6 +457,7 @@ impl<'o> VM<'o> {
VMRequest::ReadDir(path) => { VMRequest::ReadDir(path) => {
let dir = self let dir = self
.io_handle .io_handle
.as_ref()
.read_dir(&path) .read_dir(&path)
.map_err(|e| ErrorKind::IO { .map_err(|e| ErrorKind::IO {
path: Some(path), path: Some(path),

View file

@ -48,7 +48,7 @@ trait GetSpan {
fn get_span(self) -> Span; fn get_span(self) -> Span;
} }
impl<'o> GetSpan for &VM<'o> { impl<'o, IO> GetSpan for &VM<'o, IO> {
fn get_span(self) -> Span { fn get_span(self) -> Span {
self.reasonable_span.span() self.reasonable_span.span()
} }
@ -75,12 +75,12 @@ impl GetSpan for Span {
/// Internal helper trait for ergonomically converting from a `Result<T, /// Internal helper trait for ergonomically converting from a `Result<T,
/// ErrorKind>` to a `Result<T, Error>` using the current span of a call frame, /// 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. /// and chaining the VM's frame stack around it for printing a cause chain.
trait WithSpan<T, S: GetSpan> { trait WithSpan<T, S: GetSpan, IO> {
fn with_span(self, top_span: S, vm: &VM) -> Result<T, Error>; fn with_span(self, top_span: S, vm: &VM<IO>) -> Result<T, Error>;
} }
impl<T, S: GetSpan> WithSpan<T, S> for Result<T, ErrorKind> { impl<T, S: GetSpan, IO> WithSpan<T, S, IO> for Result<T, ErrorKind> {
fn with_span(self, top_span: S, vm: &VM) -> Result<T, Error> { fn with_span(self, top_span: S, vm: &VM<IO>) -> Result<T, Error> {
match self { match self {
Ok(something) => Ok(something), Ok(something) => Ok(something),
Err(kind) => { Err(kind) => {
@ -149,7 +149,7 @@ impl CallFrame {
/// Construct an error result from the given ErrorKind and the source span /// Construct an error result from the given ErrorKind and the source span
/// of the current instruction. /// 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) 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 /// VM's frame stack, representing the execution contexts the VM is working
/// through. Elements are usually pushed when functions are called, or /// through. Elements are usually pushed when functions are called, or
/// thunks are being forced. /// thunks are being forced.
@ -280,7 +280,7 @@ struct VM<'o> {
/// Implementation of I/O operations used for impure builtins and /// Implementation of I/O operations used for impure builtins and
/// features like `import`. /// features like `import`.
io_handle: Box<dyn EvalIO>, io_handle: IO,
/// Runtime observer which can print traces of runtime operations. /// Runtime observer which can print traces of runtime operations.
observer: &'o mut dyn RuntimeObserver, observer: &'o mut dyn RuntimeObserver,
@ -324,10 +324,13 @@ struct VM<'o> {
try_eval_frames: Vec<usize>, try_eval_frames: Vec<usize>,
} }
impl<'o> VM<'o> { impl<'o, IO> VM<'o, IO>
where
IO: AsRef<dyn EvalIO> + 'static,
{
pub fn new( pub fn new(
nix_search_path: NixSearchPath, nix_search_path: NixSearchPath,
io_handle: Box<dyn EvalIO>, io_handle: IO,
observer: &'o mut dyn RuntimeObserver, observer: &'o mut dyn RuntimeObserver,
globals: Rc<GlobalsMap>, globals: Rc<GlobalsMap>,
reasonable_span: LightSpan, reasonable_span: LightSpan,
@ -858,7 +861,7 @@ impl<'o> VM<'o> {
Value::UnresolvedPath(path) => { Value::UnresolvedPath(path) => {
let resolved = self let resolved = self
.nix_search_path .nix_search_path
.resolve(&mut *self.io_handle, *path) .resolve(&self.io_handle, *path)
.with_span(&frame, self)?; .with_span(&frame, self)?;
self.stack.push(resolved.into()); self.stack.push(resolved.into());
} }
@ -914,7 +917,10 @@ impl<'o> VM<'o> {
} }
/// Implementation of helper functions for the runtime logic above. /// 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 { pub(crate) fn stack_pop(&mut self) -> Value {
self.stack.pop().expect("runtime stack empty") 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) Ok(generators::request_deep_force(&co, value).await)
} }
pub fn run_lambda( pub fn run_lambda<IO>(
nix_search_path: NixSearchPath, nix_search_path: NixSearchPath,
io_handle: Box<dyn EvalIO>, io_handle: IO,
observer: &mut dyn RuntimeObserver, observer: &mut dyn RuntimeObserver,
globals: Rc<GlobalsMap>, globals: Rc<GlobalsMap>,
lambda: Rc<Lambda>, lambda: Rc<Lambda>,
strict: bool, strict: bool,
) -> EvalResult<RuntimeResult> { ) -> EvalResult<RuntimeResult>
where
IO: AsRef<dyn EvalIO> + 'static,
{
// Retain the top-level span of the expression in this lambda, as // Retain the top-level span of the expression in this lambda, as
// synthetic "calls" in deep_force will otherwise not have a span // synthetic "calls" in deep_force will otherwise not have a span
// to fall back to. // to fall back to.

View file

@ -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 /// As they need to interact with `known_paths`, we also need to pass in
/// `known_paths`. /// `known_paths`.
pub fn add_derivation_builtins( pub fn add_derivation_builtins<IO>(
eval: &mut tvix_eval::Evaluation, eval: &mut tvix_eval::Evaluation<IO>,
known_paths: Rc<RefCell<KnownPaths>>, known_paths: Rc<RefCell<KnownPaths>>,
) { ) {
eval.builtins eval.builtins

View file

@ -11,7 +11,10 @@ mod tests;
/// Tell the Evaluator to resolve `<nix>` to the path `/__corepkgs__`, /// Tell the Evaluator to resolve `<nix>` to the path `/__corepkgs__`,
/// which has special handling in [tvix_io::TvixIO]. /// which has special handling in [tvix_io::TvixIO].
/// This is used in nixpkgs to import `fetchurl.nix` from `<nix>`. /// 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 eval.nix_path = nix_search_path
.as_ref() .as_ref()
.map(|p| format!("nix=/__corepkgs__:{}", p)) .map(|p| format!("nix=/__corepkgs__:{}", p))

View file

@ -3,7 +3,7 @@
use serde::de::value::{MapDeserializer, SeqDeserializer}; use serde::de::value::{MapDeserializer, SeqDeserializer};
use serde::de::{self, EnumAccess, VariantAccess}; use serde::de::{self, EnumAccess, VariantAccess};
pub use tvix_eval::Evaluation; pub use tvix_eval::Evaluation;
use tvix_eval::Value; use tvix_eval::{EvalIO, Value};
use crate::error::Error; 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> pub fn from_str_with_config<'code, T, F>(src: &'code str, config: F) -> Result<T, Error>
where where
T: serde::Deserialize<'code>, T: serde::Deserialize<'code>,
F: FnOnce(&mut Evaluation), F: FnOnce(&mut Evaluation<Box<dyn EvalIO>>),
{ {
// First step is to evaluate the Nix code ... // First step is to evaluate the Nix code ...
let mut eval = Evaluation::default(); let mut eval = Evaluation::default();