From 1ac57b0d1cdade99cd6cdf34211aa23e63db6f97 Mon Sep 17 00:00:00 2001 From: Adam Joseph Date: Mon, 11 Dec 2023 03:57:22 -0800 Subject: feat(tvix/eval): nonrecursive coerce_to_string() After this commit, the only non-builtins uses of generators are: - coerce_to_string() uses generators::request_enter_lambda() - Thunk::force() uses generators::request_enter_lambda() That's it! Once those two are taken care of, GenCo can become an implementation detail of `builtins::BuiltinGen`. No more crazy nonlocal flow control within the interpreter: if you've got a GenCo floating around in your code it's because you're writing a builtin, which isn't part of the core interpreter. The interpreter won't need GenCos to talk to itself anymore. Technically generators::request_path_import() is also used by coerce_to_string(), but that's just because the io_handle happens to be part of the VM. There's no recursion-depth issue there, so the call doesn't need to go through the generator mechanism (request_path_import() doesn't call back to the interpreter!) Change-Id: I83ce5774d49b88fdafdd61160975b4937a435bb0 Reviewed-on: https://cl.tvl.fyi/c/depot/+/10256 Reviewed-by: tazjin Tested-by: BuildkiteCI Autosubmit: Adam Joseph --- tvix/eval/src/builtins/mod.rs | 27 ++++-- tvix/eval/src/value/attrs.rs | 26 ------ tvix/eval/src/value/json.rs | 12 ++- tvix/eval/src/value/mod.rs | 186 +++++++++++++++++++++++------------------ tvix/eval/src/vm/generators.rs | 4 +- tvix/eval/src/vm/mod.rs | 4 +- 6 files changed, 138 insertions(+), 121 deletions(-) (limited to 'tvix/eval') diff --git a/tvix/eval/src/builtins/mod.rs b/tvix/eval/src/builtins/mod.rs index 5f99704e63c3..62684b82453e 100644 --- a/tvix/eval/src/builtins/mod.rs +++ b/tvix/eval/src/builtins/mod.rs @@ -133,7 +133,11 @@ mod pure_builtins { #[builtin("baseNameOf")] async fn builtin_base_name_of(co: GenCo, s: Value) -> Result { - let s = s.coerce_to_string(co, CoercionKind::Weak).await?.to_str()?; + let span = generators::request_span(&co).await; + let s = s + .coerce_to_string(co, CoercionKind::Weak, span) + .await? + .to_str()?; let result: String = s.rsplit_once('/').map(|(_, x)| x).unwrap_or(&s).into(); Ok(result.into()) } @@ -248,7 +252,11 @@ mod pure_builtins { #[builtin("dirOf")] async fn builtin_dir_of(co: GenCo, s: Value) -> Result { let is_path = s.is_path(); - let str = s.coerce_to_string(co, CoercionKind::Weak).await?.to_str()?; + let span = generators::request_span(&co).await; + let str = s + .coerce_to_string(co, CoercionKind::Weak, span) + .await? + .to_str()?; let result = str .rsplit_once('/') .map(|(x, _)| match x { @@ -915,7 +923,8 @@ mod pure_builtins { #[builtin("stringLength")] async fn builtin_string_length(co: GenCo, #[lazy] s: Value) -> Result { // also forces the value - let s = s.coerce_to_string(co, CoercionKind::Weak).await?; + let span = generators::request_span(&co).await; + let s = s.coerce_to_string(co, CoercionKind::Weak, span).await?; Ok(Value::Integer(s.to_str()?.as_str().len() as i64)) } @@ -933,7 +942,11 @@ mod pure_builtins { ) -> Result { let beg = start.as_int()?; let len = len.as_int()?; - let x = s.coerce_to_string(co, CoercionKind::Weak).await?.to_str()?; + let span = generators::request_span(&co).await; + let x = s + .coerce_to_string(co, CoercionKind::Weak, span) + .await? + .to_str()?; if beg < 0 { return Err(ErrorKind::IndexOutOfBounds { index: beg }); @@ -978,7 +991,8 @@ mod pure_builtins { #[builtin("toString")] async fn builtin_to_string(co: GenCo, #[lazy] x: Value) -> Result { // coerce_to_string forces for us - x.coerce_to_string(co, CoercionKind::Strong).await + let span = generators::request_span(&co).await; + x.coerce_to_string(co, CoercionKind::Strong, span).await } #[builtin("toXML")] @@ -1010,7 +1024,8 @@ mod pure_builtins { Err(cek) => Ok(Value::Catchable(cek)), Ok(path) => { let path: Value = crate::value::canon_path(path).into(); - Ok(path.coerce_to_string(co, CoercionKind::Weak).await?) + let span = generators::request_span(&co).await; + Ok(path.coerce_to_string(co, CoercionKind::Weak, span).await?) } } } diff --git a/tvix/eval/src/value/attrs.rs b/tvix/eval/src/value/attrs.rs index 03183810f24b..b80d8b2db84e 100644 --- a/tvix/eval/src/value/attrs.rs +++ b/tvix/eval/src/value/attrs.rs @@ -14,11 +14,9 @@ use serde::Deserialize; use super::string::NixString; use super::thunk::ThunkSet; -use super::CoercionKind; use super::TotalDisplay; use super::Value; use crate::errors::ErrorKind; -use crate::generators::{self, GenCo}; lazy_static! { static ref NAME_S: NixString = "name".into(); @@ -370,30 +368,6 @@ impl NixAttrs { pub(crate) fn from_kv(name: Value, value: Value) -> Self { NixAttrs(AttrsRep::KV { name, value }) } - - /// Attempt to coerce an attribute set with a `__toString` - /// attribute to a string. - pub(crate) async fn try_to_string(&self, co: &GenCo, kind: CoercionKind) -> Option { - if let Some(to_string) = self.select("__toString") { - let callable = generators::request_force(co, to_string.clone()).await; - - // Leave the attribute set on the stack as an argument - // to the function call. - generators::request_stack_push(co, Value::Attrs(Box::new(self.clone()))).await; - - // Call the callable ... - let result = generators::request_call(co, callable).await; - - // Recurse on the result, as attribute set coercion - // actually works recursively, e.g. you can even return - // /another/ set with a __toString attr. - let s = generators::request_string_coerce(co, result, kind).await; - - return s.ok(); - } - - None - } } impl IntoIterator for NixAttrs { diff --git a/tvix/eval/src/value/json.rs b/tvix/eval/src/value/json.rs index bff04487fa50..f7ecf121de5e 100644 --- a/tvix/eval/src/value/json.rs +++ b/tvix/eval/src/value/json.rs @@ -44,8 +44,16 @@ impl Value { // Attribute sets with a callable `__toString` attribute // serialise to the string-coerced version of the result of // calling that. - if let Some(s) = attrs.try_to_string(co, CoercionKind::Weak).await { - return Ok(Ok(Json::String(s.as_str().to_string()))); + if attrs.select("__toString").is_some() { + let span = generators::request_span(co).await; + match Value::Attrs(attrs) + .coerce_to_string_(co, CoercionKind::Weak, span) + .await? + { + Value::Catchable(cek) => return Ok(Err(cek)), + Value::String(s) => return Ok(Ok(Json::String(s.as_str().to_owned()))), + _ => panic!("Value::coerce_to_string_() returned a non-string!"), + } } // Attribute sets with an `outPath` attribute diff --git a/tvix/eval/src/value/mod.rs b/tvix/eval/src/value/mod.rs index 596dddba520b..003567bfd440 100644 --- a/tvix/eval/src/value/mod.rs +++ b/tvix/eval/src/value/mod.rs @@ -289,102 +289,122 @@ impl Value { } } - // TODO(amjoseph): de-asyncify this (when called directly by the VM) + pub async fn coerce_to_string( + self, + co: GenCo, + kind: CoercionKind, + span: LightSpan, + ) -> Result { + self.coerce_to_string_(&co, kind, span).await + } + /// Coerce a `Value` to a string. See `CoercionKind` for a rundown of what /// input types are accepted under what circumstances. - pub async fn coerce_to_string(self, co: GenCo, kind: CoercionKind) -> Result { - let value = generators::request_force(&co, self).await; - - match (value, kind) { - // coercions that are always done - tuple @ (Value::String(_), _) => Ok(tuple.0), - - // TODO(sterni): Think about proper encoding handling here. This needs - // general consideration anyways, since one current discrepancy between - // C++ Nix and Tvix is that the former's strings are arbitrary byte - // sequences without NUL bytes, whereas Tvix only allows valid - // Unicode. See also b/189. - (Value::Path(p), _) => { - // TODO(tazjin): there are cases where coerce_to_string does not import - let imported = generators::request_path_import(&co, *p).await; - Ok(imported.to_string_lossy().into_owned().into()) - } - - // Attribute sets can be converted to strings if they either have an - // `__toString` attribute which holds a function that receives the - // set itself or an `outPath` attribute which should be a string. - // `__toString` is preferred. - (Value::Attrs(attrs), kind) => { - if let Some(s) = attrs.try_to_string(&co, kind).await { - return Ok(Value::String(s)); + pub async fn coerce_to_string_( + self, + co: &GenCo, + kind: CoercionKind, + span: LightSpan, + ) -> Result { + let mut result = String::new(); + let mut vals = vec![self]; + loop { + let value = if let Some(v) = vals.pop() { + v.force(co, span.clone()).await? + } else { + return Ok(Value::String(result.into())); + }; + let coerced = match (value, kind) { + // coercions that are always done + (Value::String(s), _) => Ok(s.as_str().to_owned()), + + // TODO(sterni): Think about proper encoding handling here. This needs + // general consideration anyways, since one current discrepancy between + // C++ Nix and Tvix is that the former's strings are arbitrary byte + // sequences without NUL bytes, whereas Tvix only allows valid + // Unicode. See also b/189. + (Value::Path(p), _) => { + // TODO(tazjin): there are cases where coerce_to_string does not import + let imported = generators::request_path_import(co, *p).await; + Ok(imported.to_string_lossy().into_owned()) } - if let Some(out_path) = attrs.select("outPath") { - return match generators::request_string_coerce(&co, out_path.clone(), kind) - .await - { - Ok(s) => Ok(Value::String(s)), - Err(c) => Ok(Value::Catchable(c)), - }; + // Attribute sets can be converted to strings if they either have an + // `__toString` attribute which holds a function that receives the + // set itself or an `outPath` attribute which should be a string. + // `__toString` is preferred. + (Value::Attrs(attrs), kind) => { + if let Some(to_string) = attrs.select("__toString") { + let callable = to_string.clone().force(co, span.clone()).await?; + + // Leave the attribute set on the stack as an argument + // to the function call. + generators::request_stack_push(co, Value::Attrs(attrs.clone())).await; + + // Call the callable ... + let result = generators::request_call(co, callable).await; + + // Recurse on the result, as attribute set coercion + // actually works recursively, e.g. you can even return + // /another/ set with a __toString attr. + vals.push(result); + continue; + } else if let Some(out_path) = attrs.select("outPath") { + vals.push(out_path.clone()); + continue; + } else { + return Err(ErrorKind::NotCoercibleToString { from: "set", kind }); + } } - Err(ErrorKind::NotCoercibleToString { from: "set", kind }) - } - - // strong coercions - (Value::Null, CoercionKind::Strong) | (Value::Bool(false), CoercionKind::Strong) => { - Ok("".into()) - } - (Value::Bool(true), CoercionKind::Strong) => Ok("1".into()), - - (Value::Integer(i), CoercionKind::Strong) => Ok(format!("{i}").into()), - (Value::Float(f), CoercionKind::Strong) => { - // contrary to normal Display, coercing a float to a string will - // result in unconditional 6 decimal places - Ok(format!("{:.6}", f).into()) - } - - // Lists are coerced by coercing their elements and interspersing spaces - (Value::List(list), CoercionKind::Strong) => { - let mut out = String::new(); + // strong coercions + (Value::Null, CoercionKind::Strong) + | (Value::Bool(false), CoercionKind::Strong) => Ok("".to_owned()), + (Value::Bool(true), CoercionKind::Strong) => Ok("1".to_owned()), - for (idx, elem) in list.into_iter().enumerate() { - if idx > 0 { - out.push(' '); - } + (Value::Integer(i), CoercionKind::Strong) => Ok(format!("{i}")), + (Value::Float(f), CoercionKind::Strong) => { + // contrary to normal Display, coercing a float to a string will + // result in unconditional 6 decimal places + Ok(format!("{:.6}", f)) + } - match generators::request_string_coerce(&co, elem, kind).await { - Ok(s) => out.push_str(s.as_str()), - Err(c) => return Ok(Value::Catchable(c)), + // Lists are coerced by coercing their elements and interspersing spaces + (Value::List(list), CoercionKind::Strong) => { + for elem in list.into_iter().rev() { + vals.push(elem); } + continue; } - Ok(Value::String(out.into())) - } + (Value::Thunk(_), _) => panic!("Tvix bug: force returned unforced thunk"), + + val @ (Value::Closure(_), _) + | val @ (Value::Builtin(_), _) + | val @ (Value::Null, _) + | val @ (Value::Bool(_), _) + | val @ (Value::Integer(_), _) + | val @ (Value::Float(_), _) + | val @ (Value::List(_), _) => Err(ErrorKind::NotCoercibleToString { + from: val.0.type_of(), + kind, + }), - (Value::Thunk(_), _) => panic!("Tvix bug: force returned unforced thunk"), - - val @ (Value::Closure(_), _) - | val @ (Value::Builtin(_), _) - | val @ (Value::Null, _) - | val @ (Value::Bool(_), _) - | val @ (Value::Integer(_), _) - | val @ (Value::Float(_), _) - | val @ (Value::List(_), _) => Err(ErrorKind::NotCoercibleToString { - from: val.0.type_of(), - kind, - }), - - (c @ Value::Catchable(_), _) => Ok(c), - - (Value::AttrNotFound, _) - | (Value::Blueprint(_), _) - | (Value::DeferredUpvalue(_), _) - | (Value::UnresolvedPath(_), _) - | (Value::Json(_), _) - | (Value::FinaliseRequest(_), _) => { - panic!("tvix bug: .coerce_to_string() called on internal value") + (c @ Value::Catchable(_), _) => return Ok(c), + + (Value::AttrNotFound, _) + | (Value::Blueprint(_), _) + | (Value::DeferredUpvalue(_), _) + | (Value::UnresolvedPath(_), _) + | (Value::Json(_), _) + | (Value::FinaliseRequest(_), _) => { + panic!("tvix bug: .coerce_to_string() called on internal value") + } + }; + if !result.is_empty() { + result.push(' '); } + result.push_str(&coerced?); } } diff --git a/tvix/eval/src/vm/generators.rs b/tvix/eval/src/vm/generators.rs index 7b92c1f0af35..6563a82790a6 100644 --- a/tvix/eval/src/vm/generators.rs +++ b/tvix/eval/src/vm/generators.rs @@ -344,8 +344,8 @@ impl<'o> VM<'o> { VMRequest::StringCoerce(val, kind) => { self.reenqueue_generator(name, span.clone(), generator); - self.enqueue_generator("coerce_to_string", span, |co| { - val.coerce_to_string(co, kind) + self.enqueue_generator("coerce_to_string", span.clone(), |co| { + val.coerce_to_string(co, kind, span) }); return Ok(false); } diff --git a/tvix/eval/src/vm/mod.rs b/tvix/eval/src/vm/mod.rs index 5ab095b22056..6128e3a601a0 100644 --- a/tvix/eval/src/vm/mod.rs +++ b/tvix/eval/src/vm/mod.rs @@ -744,8 +744,8 @@ impl<'o> VM<'o> { let gen_span = frame.current_light_span(); self.push_call_frame(span, frame); - self.enqueue_generator("coerce_to_string", gen_span, |co| { - value.coerce_to_string(co, CoercionKind::Weak) + self.enqueue_generator("coerce_to_string", gen_span.clone(), |co| { + value.coerce_to_string(co, CoercionKind::Weak, gen_span) }); return Ok(false); -- cgit 1.4.1