diff options
| -rw-r--r-- | tvix/docs/src/TODO.md | 9 | ||||
| -rw-r--r-- | tvix/eval/src/builtins/mod.rs | 15 | ||||
| -rw-r--r-- | tvix/eval/src/builtins/to_xml.rs | 1 | ||||
| -rw-r--r-- | tvix/eval/src/value/json.rs | 68 | ||||
| -rw-r--r-- | tvix/eval/src/value/mod.rs | 7 | ||||
| -rw-r--r-- | tvix/eval/src/vm/generators.rs | 27 | ||||
| -rw-r--r-- | tvix/glue/src/builtins/derivation.rs | 8 | ||||
| -rw-r--r-- | tvix/serde/src/de.rs | 1 |
8 files changed, 27 insertions, 109 deletions
diff --git a/tvix/docs/src/TODO.md b/tvix/docs/src/TODO.md index ff10b26cbf1d..4db77b04d442 100644 --- a/tvix/docs/src/TODO.md +++ b/tvix/docs/src/TODO.md @@ -10,15 +10,6 @@ Feel free to add new ideas. Before picking something, ask in `#tvix-dev` to make sure noone is working on this, or has some specific design in mind already. ## Cleanups -### Evaluator - - There's not really a good reason why the `tvix_eval::Value::Json` enum kind - exists. - `builtins.toJSON` should simply produce a string with context, and everything - else should be a hidden implementation detail and should not be leaked to - `Value`. - This is a hack, as we wanted to use `serde_json` as is, but should be cleaned - up. - ### Nix language test suite - Think about how to merge, but "categorize" `tvix_tests` in `glue` and `eval`. We currently only have this split as they need a different feature set / diff --git a/tvix/eval/src/builtins/mod.rs b/tvix/eval/src/builtins/mod.rs index f130bbc5b15f..47c0f85b3c1b 100644 --- a/tvix/eval/src/builtins/mod.rs +++ b/tvix/eval/src/builtins/mod.rs @@ -442,17 +442,20 @@ mod pure_builtins { #[builtin("fromJSON")] async fn builtin_from_json(co: GenCo, json: Value) -> Result<Value, ErrorKind> { let json_str = json.to_str()?; - serde_json::from_slice(&json_str).map_err(|err| err.into()) } #[builtin("toJSON")] async fn builtin_to_json(co: GenCo, val: Value) -> Result<Value, ErrorKind> { - match val.into_contextful_json(&co).await? { - Err(cek) => Ok(Value::from(cek)), - Ok((json_value, ctx)) => { - let json_str = serde_json::to_string(&json_value)?; - Ok(NixString::new_context_from(ctx, json_str).into()) + match val.into_contextful_json(&co).await { + Err(ErrorKind::CatchableError(catchable)) => Ok(Value::Catchable(Box::new(catchable))), + Err(err) => Err(err), + Ok((json, context)) => { + let json_str = serde_json::to_string(&json) + .map_err(|err| ErrorKind::JsonError(err.to_string()))?; + Ok(Value::String(NixString::new_context_from( + context, json_str, + ))) } } } diff --git a/tvix/eval/src/builtins/to_xml.rs b/tvix/eval/src/builtins/to_xml.rs index 093e127fe25e..785992e457ac 100644 --- a/tvix/eval/src/builtins/to_xml.rs +++ b/tvix/eval/src/builtins/to_xml.rs @@ -122,7 +122,6 @@ fn value_variant_to_xml<W: Write>(w: &mut XmlEmitter<W>, value: &Value) -> Resul | Value::Blueprint(_) | Value::DeferredUpvalue(_) | Value::UnresolvedPath(_) - | Value::Json(..) | Value::FinaliseRequest(_) => { return Err(ErrorKind::TvixBug { msg: "internal value variant encountered in builtins.toXML", diff --git a/tvix/eval/src/value/json.rs b/tvix/eval/src/value/json.rs index 24a6bcaf6f21..1ef3f9911def 100644 --- a/tvix/eval/src/value/json.rs +++ b/tvix/eval/src/value/json.rs @@ -4,7 +4,7 @@ /// as there is internal Nix logic that must happen within the /// serialisation methods. use super::{CoercionKind, Value}; -use crate::errors::{CatchableErrorKind, ErrorKind}; +use crate::errors::ErrorKind; use crate::generators::{self, GenCo}; use crate::NixContext; @@ -17,10 +17,7 @@ impl Value { /// Transforms the structure into a JSON /// and accumulate all encountered context in the second's element /// of the return type. - pub async fn into_contextful_json( - self, - co: &GenCo, - ) -> Result<Result<(Json, NixContext), CatchableErrorKind>, ErrorKind> { + pub async fn into_contextful_json(self, co: &GenCo) -> Result<(Json, NixContext), ErrorKind> { let self_forced = generators::request_force(co, self).await; let mut context = NixContext::new(); @@ -46,13 +43,9 @@ impl Value { let mut out = vec![]; for val in l.into_iter() { - match generators::request_to_json(co, val).await { - Ok((v, ctx)) => { - context.extend(ctx.into_iter()); - out.push(v) - } - Err(cek) => return Ok(Err(cek)), - } + let (json_item, ctx) = Box::pin(val.into_contextful_json(co)).await?; + context.extend(ctx.into_iter()); + out.push(json_item); } Json::Array(out) @@ -75,14 +68,13 @@ impl Value { ) .await? { - Value::Catchable(cek) => return Ok(Err(*cek)), + Value::Catchable(cek) => return Err(ErrorKind::CatchableError(*cek)), Value::String(s) => { // We need a fresh context here because `__toString` will discard // everything. let mut fresh = NixContext::new(); fresh.mimic(&s); - - return Ok(Ok((Json::String(s.to_str()?.to_owned()), fresh))); + return Ok((Json::String(s.to_str()?.to_owned()), fresh)); } _ => panic!("Value::coerce_to_string_() returned a non-string!"), } @@ -92,27 +84,23 @@ impl Value { // serialise to a JSON serialisation of that inner // value (regardless of what it is!). if let Some(out_path) = attrs.select("outPath") { - return Ok(generators::request_to_json(co, out_path.clone()).await); + let (json_out_path, ctx) = + Box::pin(out_path.clone().into_contextful_json(co)).await?; + context.extend(ctx.into_iter()); + return Ok((json_out_path, context)); } let mut out = Map::with_capacity(attrs.len()); for (name, value) in attrs.into_iter_sorted() { - out.insert( - name.to_str()?.to_owned(), - match generators::request_to_json(co, value).await { - Ok((v, ctx)) => { - context.extend(ctx.into_iter()); - v - } - Err(cek) => return Ok(Err(cek)), - }, - ); + let (json_value, ctx) = Box::pin(value.into_contextful_json(co)).await?; + context.extend(ctx.into_iter()); + out.insert(name.to_str()?.to_owned(), json_value); } Json::Object(out) } - Value::Catchable(c) => return Ok(Err(*c)), + Value::Catchable(c) => return Err(ErrorKind::CatchableError(*c)), val @ Value::Closure(_) | val @ Value::Thunk(_) @@ -121,34 +109,10 @@ impl Value { | val @ Value::Blueprint(_) | val @ Value::DeferredUpvalue(_) | val @ Value::UnresolvedPath(_) - | val @ Value::Json(..) | val @ Value::FinaliseRequest(_) => { return Err(ErrorKind::NotSerialisableToJson(val.type_of())) } }; - - Ok(Ok((value, context))) - } - - /// Generator version of the above, which wraps responses in - /// [`Value::Json`]. - pub(crate) async fn into_contextful_json_generator( - self, - co: GenCo, - ) -> Result<Value, ErrorKind> { - match self.into_contextful_json(&co).await? { - Err(cek) => Ok(Value::from(cek)), - Ok((json, ctx)) => Ok(Value::Json(Box::new((json, ctx)))), - } - } - - /// Transforms the structure into a JSON - /// All the accumulated context is ignored, use [`into_contextful_json`] - /// to obtain the resulting context of the JSON object. - pub async fn into_json( - self, - co: &GenCo, - ) -> Result<Result<Json, CatchableErrorKind>, ErrorKind> { - Ok(self.into_contextful_json(co).await?.map(|(json, _)| json)) + Ok((value, context)) } } diff --git a/tvix/eval/src/value/mod.rs b/tvix/eval/src/value/mod.rs index 19ff17feab51..1775c6f71adb 100644 --- a/tvix/eval/src/value/mod.rs +++ b/tvix/eval/src/value/mod.rs @@ -76,8 +76,6 @@ pub enum Value { DeferredUpvalue(StackIdx), #[serde(skip)] UnresolvedPath(Box<PathBuf>), - #[serde(skip)] - Json(Box<(serde_json::Value, NixContext)>), #[serde(skip)] FinaliseRequest(bool), @@ -299,7 +297,6 @@ impl Value { | Value::Blueprint(_) | Value::DeferredUpvalue(_) | Value::UnresolvedPath(_) - | Value::Json(..) | Value::FinaliseRequest(_) => panic!( "Tvix bug: internal value left on stack: {}", value.type_of() @@ -449,7 +446,6 @@ impl Value { | (Value::Blueprint(_), _) | (Value::DeferredUpvalue(_), _) | (Value::UnresolvedPath(_), _) - | (Value::Json(..), _) | (Value::FinaliseRequest(_), _) => { panic!("tvix bug: .coerce_to_string() called on internal value") } @@ -686,7 +682,6 @@ impl Value { Value::Blueprint(_) => "internal[blueprint]", Value::DeferredUpvalue(_) => "internal[deferred_upvalue]", Value::UnresolvedPath(_) => "internal[unresolved_path]", - Value::Json(..) => "internal[json]", Value::FinaliseRequest(_) => "internal[finaliser_sentinel]", Value::Catchable(_) => "internal[catchable]", } @@ -882,7 +877,6 @@ impl Value { | Value::Blueprint(_) | Value::DeferredUpvalue(_) | Value::UnresolvedPath(_) - | Value::Json(..) | Value::FinaliseRequest(_) => "an internal Tvix evaluator value".into(), } } @@ -1002,7 +996,6 @@ impl TotalDisplay for Value { Value::Blueprint(_) => f.write_str("internal[blueprint]"), Value::DeferredUpvalue(_) => f.write_str("internal[deferred_upvalue]"), Value::UnresolvedPath(_) => f.write_str("internal[unresolved_path]"), - Value::Json(..) => f.write_str("internal[json]"), Value::FinaliseRequest(_) => f.write_str("internal[finaliser_sentinel]"), // Delegate thunk display to the type, as it must handle diff --git a/tvix/eval/src/vm/generators.rs b/tvix/eval/src/vm/generators.rs index ae9a8dd6ab51..b47f20bae289 100644 --- a/tvix/eval/src/vm/generators.rs +++ b/tvix/eval/src/vm/generators.rs @@ -118,10 +118,6 @@ pub enum VMRequest { /// [`VM::catch_result`] for an explanation of how this works. TryForce(Value), - /// Request serialisation of a value to JSON, according to the - /// slightly odd Nix evaluation rules. - ToJson(Value), - /// Request the VM for the file type of the given path. ReadFileType(PathBuf), } @@ -180,7 +176,6 @@ impl Display for VMRequest { VMRequest::ReadDir(p) => write!(f, "read_dir({})", p.to_string_lossy()), VMRequest::Span => write!(f, "span"), VMRequest::TryForce(v) => write!(f, "try_force({})", v.type_of()), - VMRequest::ToJson(v) => write!(f, "to_json({})", v.type_of()), VMRequest::ReadFileType(p) => write!(f, "read_file_type({})", p.to_string_lossy()), } } @@ -497,14 +492,6 @@ where return Ok(false); } - VMRequest::ToJson(value) => { - self.reenqueue_generator(name, span, generator); - self.enqueue_generator("to_json", span, |co| { - value.into_contextful_json_generator(co) - }); - return Ok(false); - } - VMRequest::ReadFileType(path) => { let file_type = self .io_handle @@ -798,20 +785,6 @@ pub(crate) async fn request_span(co: &GenCo) -> Span { } } -pub(crate) async fn request_to_json( - co: &GenCo, - value: Value, -) -> Result<(serde_json::Value, NixContext), CatchableErrorKind> { - match co.yield_(VMRequest::ToJson(value)).await { - VMResponse::Value(Value::Json(json_with_ctx)) => Ok(*json_with_ctx), - VMResponse::Value(Value::Catchable(cek)) => Err(*cek), - msg => panic!( - "Tvix bug: VM responded with incorrect generator message: {}", - msg - ), - } -} - #[cfg_attr(not(feature = "impure"), allow(unused))] pub(crate) async fn request_read_file_type(co: &GenCo, path: PathBuf) -> FileType { match co.yield_(VMRequest::ReadFileType(path)).await { diff --git a/tvix/glue/src/builtins/derivation.rs b/tvix/glue/src/builtins/derivation.rs index 11b70a934a7b..3048fd8390c2 100644 --- a/tvix/glue/src/builtins/derivation.rs +++ b/tvix/glue/src/builtins/derivation.rs @@ -300,7 +300,7 @@ pub(crate) mod derivation_builtins { // Remove the original default `out` output. drv.outputs.clear(); - let mut output_names = vec![]; + let mut output_names = Vec::with_capacity(outputs.len()); for output in outputs { let output_name = generators::request_force(&co, output) @@ -379,11 +379,7 @@ pub(crate) mod derivation_builtins { return Ok(val); } - let (val_json, context) = match val.into_contextful_json(&co).await? { - Ok(v) => v, - Err(cek) => return Ok(Value::from(cek)), - }; - + let (val_json, context) = val.into_contextful_json(&co).await?; input_context.extend(context.into_iter()); // No need to check for dups, we only iterate over every attribute name once diff --git a/tvix/serde/src/de.rs b/tvix/serde/src/de.rs index 0728311c6a13..ac2e7ef5730a 100644 --- a/tvix/serde/src/de.rs +++ b/tvix/serde/src/de.rs @@ -105,7 +105,6 @@ impl<'de> de::Deserializer<'de> for NixDeserializer { | Value::Blueprint(_) | Value::DeferredUpvalue(_) | Value::UnresolvedPath(_) - | Value::Json(..) | Value::Catchable(_) | Value::FinaliseRequest(_) => Err(Error::Unserializable { value_type: self.value.type_of(), |