fix(tvix/eval): implement cppnix JSON-serialisation semantics r/5979

This drops the usage of serde::Serialize, as the trait can not be used
to implement the correct semantics (function colouring!).

Instead, a manual JSON serialisation function is written which
correctly handles toString, outPath and other similar weirdnesses.

Unexpectedly, the eval-okay-tojson test from the C++ Nix test suite
now passes, too.

This fixes an issue where serialising data structures containing
derivations to JSON would fail.

Change-Id: I5c39e3d8356ee93a07eda481410f88610f6dd9f8
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8209
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Tested-by: BuildkiteCI

12 files changed, 138 insertions, 70 deletions

diff --git a/tvix/eval/src/builtins/mod.rs b/tvix/eval/src/builtins/mod.rs
index 535bf8b2a305..e24285f5c23a 100644
--- a/tvix/eval/src/builtins/mod.rs
+++ b/tvix/eval/src/builtins/mod.rs
@@ -359,36 +359,8 @@ mod pure_builtins {
 
     #[builtin("toJSON")]
     async fn builtin_to_json(co: GenCo, val: Value) -> Result<Value, ErrorKind> {
-        let mut val = val; // shadow mutably, not supported by macro
-        loop {
-            if let Value::Attrs(attrs) = &val {
-                // 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(Value::String(serde_json::to_string(&s)?.into()));
-                }
-
-                // Attribute sets with an `outPath` attribute
-                // serialise to a JSON serialisation of that inner
-                // value (regardless of what it is!).
-                if let Some(out_path) = attrs.select("outPath") {
-                    val = out_path.clone();
-                    continue;
-                }
-
-                // Attribute set should be serialised normally (by
-                // traversing it and serialising keys/values).
-                break;
-            }
-
-            break;
-        }
-
-        // All thunks need to be evaluated before serialising, as the
-        // data structure is fully traversed by the Serializer.
-        let val = generators::request_deep_force(&co, val, SharedThunkSet::default()).await;
-        let json_str = serde_json::to_string(&val)?;
+        let json_value = val.to_json(&co).await?;
+        let json_str = serde_json::to_string(&json_value)?;
         Ok(json_str.into())
     }
 
diff --git a/tvix/eval/src/builtins/to_xml.rs b/tvix/eval/src/builtins/to_xml.rs
index 750bb27aa226..375785b7a63e 100644
--- a/tvix/eval/src/builtins/to_xml.rs
+++ b/tvix/eval/src/builtins/to_xml.rs
@@ -126,7 +126,8 @@ fn value_variant_to_xml<W: Write>(w: &mut EventWriter<W>, value: &Value) -> Resu
         Value::AttrNotFound
         | Value::Blueprint(_)
         | Value::DeferredUpvalue(_)
-        | Value::UnresolvedPath(_) => {
+        | Value::UnresolvedPath(_)
+        | Value::Json(_) => {
             return Err(ErrorKind::TvixBug {
                 msg: "internal value variant encountered in builtins.toXML",
                 metadata: Some(Rc::new(value.clone())),
diff --git a/tvix/eval/src/errors.rs b/tvix/eval/src/errors.rs
index ec697fa84288..2f00ecab8543 100644
--- a/tvix/eval/src/errors.rs
+++ b/tvix/eval/src/errors.rs
@@ -134,6 +134,9 @@ pub enum ErrorKind {
     /// Errors converting JSON to a value
     FromJsonError(String),
 
+    /// Nix value that can not be serialised to JSON.
+    NotSerialisableToJson(&'static str),
+
     /// Errors converting TOML to a value
     FromTomlError(String),
 
@@ -442,6 +445,10 @@ to a missing value in the attribute set(s) included via `with`."#,
                 write!(f, "Error converting JSON to a Nix value: {msg}")
             }
 
+            ErrorKind::NotSerialisableToJson(_type) => {
+                write!(f, "a {} cannot be converted to JSON", _type)
+            }
+
             ErrorKind::FromTomlError(msg) => {
                 write!(f, "Error converting TOML to a Nix value: {msg}")
             }
@@ -761,6 +768,7 @@ impl Error {
             | ErrorKind::ImportCompilerError { .. }
             | ErrorKind::IO { .. }
             | ErrorKind::FromJsonError(_)
+            | ErrorKind::NotSerialisableToJson(_)
             | ErrorKind::FromTomlError(_)
             | ErrorKind::Xml(_)
             | ErrorKind::TvixError(_)
@@ -809,6 +817,7 @@ impl Error {
             ErrorKind::DivisionByZero => "E033",
             ErrorKind::Xml(_) => "E034",
             ErrorKind::FromTomlError(_) => "E035",
+            ErrorKind::NotSerialisableToJson(_) => "E036",
 
             // Special error code for errors from other Tvix
             // components. We may want to introduce a code namespacing
diff --git a/tvix/eval/src/tests/nix_tests/notyetpassing/eval-okay-tojson.exp b/tvix/eval/src/tests/nix_tests/eval-okay-tojson.exp
index e92aae3235f2..e92aae3235f2 100644
--- a/tvix/eval/src/tests/nix_tests/notyetpassing/eval-okay-tojson.exp
+++ b/tvix/eval/src/tests/nix_tests/eval-okay-tojson.exp
diff --git a/tvix/eval/src/tests/nix_tests/notyetpassing/eval-okay-tojson.nix b/tvix/eval/src/tests/nix_tests/eval-okay-tojson.nix
index ce67943bead5..ce67943bead5 100644
--- a/tvix/eval/src/tests/nix_tests/notyetpassing/eval-okay-tojson.nix
+++ b/tvix/eval/src/tests/nix_tests/eval-okay-tojson.nix
diff --git a/tvix/eval/src/value/attrs.rs b/tvix/eval/src/value/attrs.rs
index e4584e3aa2f7..bacfd22217bd 100644
--- a/tvix/eval/src/value/attrs.rs
+++ b/tvix/eval/src/value/attrs.rs
@@ -10,8 +10,7 @@ use std::iter::FromIterator;
 use imbl::{ordmap, OrdMap};
 use lazy_static::lazy_static;
 use serde::de::{Deserializer, Error, Visitor};
-use serde::ser::SerializeMap;
-use serde::{Deserialize, Serialize};
+use serde::Deserialize;
 
 use super::string::NixString;
 use super::thunk::ThunkSet;
@@ -151,24 +150,6 @@ impl TotalDisplay for NixAttrs {
     }
 }
 
-impl Serialize for NixAttrs {
-    fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
-    where
-        S: serde::Serializer,
-    {
-        match &self.0 {
-            AttrsRep::Empty => serializer.serialize_map(Some(0))?.end(),
-            AttrsRep::KV { name, value } => {
-                let mut map = serializer.serialize_map(Some(2))?;
-                map.serialize_entry("name", name)?;
-                map.serialize_entry("value", value)?;
-                map.end()
-            }
-            AttrsRep::Im(map) => map.serialize(serializer),
-        }
-    }
-}
-
 impl<'de> Deserialize<'de> for NixAttrs {
     fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
     where
diff --git a/tvix/eval/src/value/json.rs b/tvix/eval/src/value/json.rs
new file mode 100644
index 000000000000..33e16ebffcd0
--- /dev/null
+++ b/tvix/eval/src/value/json.rs
@@ -0,0 +1,84 @@
+/// Implementation of Value serialisation *to* JSON.
+///
+/// This can not be implemented through standard serde-derive methods,
+/// as there is internal Nix logic that must happen within the
+/// serialisation methods.
+use super::{CoercionKind, Value};
+use crate::generators::{self, GenCo};
+use crate::ErrorKind;
+
+use serde_json::value::to_value;
+use serde_json::Value as Json;
+use serde_json::{Map, Number}; // name clash with *our* `Value`
+
+impl Value {
+    pub(crate) async fn to_json(self, co: &GenCo) -> Result<Json, ErrorKind> {
+        let self_forced = generators::request_force(co, self).await;
+
+        let value = match self_forced {
+            Value::Null => Json::Null,
+            Value::Bool(b) => Json::Bool(b),
+            Value::Integer(i) => Json::Number(Number::from(i)),
+            Value::Float(f) => to_value(f)?,
+            Value::String(s) => Json::String(s.as_str().into()),
+
+            Value::Path(p) => {
+                let imported = generators::request_path_import(co, *p).await;
+                Json::String(imported.to_string_lossy().to_string())
+            }
+
+            Value::List(l) => {
+                let mut out = vec![];
+
+                for val in l.into_iter() {
+                    out.push(generators::request_to_json(co, val).await);
+                }
+
+                Json::Array(out)
+            }
+
+            Value::Attrs(attrs) => {
+                // 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(Json::String(s.as_str().to_string()));
+                }
+
+                // Attribute sets with an `outPath` attribute
+                // 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 mut out = Map::with_capacity(attrs.len());
+                for (name, value) in attrs.into_iter_sorted() {
+                    out.insert(
+                        name.as_str().to_string(),
+                        generators::request_to_json(co, value).await,
+                    );
+                }
+
+                Json::Object(out)
+            }
+
+            val @ Value::Closure(_)
+            | val @ Value::Thunk(_)
+            | val @ Value::Builtin(_)
+            | val @ Value::AttrNotFound
+            | val @ Value::Blueprint(_)
+            | val @ Value::DeferredUpvalue(_)
+            | val @ Value::UnresolvedPath(_)
+            | val @ Value::Json(_) => return Err(ErrorKind::NotSerialisableToJson(val.type_of())),
+        };
+
+        Ok(value)
+    }
+
+    /// Generator version of the above, which wraps responses in
+    /// Value::Json.
+    pub(crate) async fn to_json_generator(self, co: GenCo) -> Result<Value, ErrorKind> {
+        Ok(Value::Json(self.to_json(&co).await?))
+    }
+}
diff --git a/tvix/eval/src/value/list.rs b/tvix/eval/src/value/list.rs
index 0bdc61c91d68..cfaefff82195 100644
--- a/tvix/eval/src/value/list.rs
+++ b/tvix/eval/src/value/list.rs
@@ -4,7 +4,7 @@ use std::rc::Rc;
 
 use imbl::{vector, Vector};
 
-use serde::{Deserialize, Serialize};
+use serde::Deserialize;
 
 use crate::generators;
 use crate::generators::GenCo;
@@ -16,7 +16,7 @@ use super::TotalDisplay;
 use super::Value;
 
 #[repr(transparent)]
-#[derive(Clone, Debug, Deserialize, Serialize)]
+#[derive(Clone, Debug, Deserialize)]
 pub struct NixList(Rc<Vector<Value>>);
 
 impl TotalDisplay for NixList {
diff --git a/tvix/eval/src/value/mod.rs b/tvix/eval/src/value/mod.rs
index 4e93b36d55d3..41b1e5ac8a16 100644
--- a/tvix/eval/src/value/mod.rs
+++ b/tvix/eval/src/value/mod.rs
@@ -9,13 +9,14 @@ use std::pin::Pin;
 use std::rc::Rc;
 
 use lexical_core::format::CXX_LITERAL;
-use serde::{Deserialize, Serialize};
+use serde::Deserialize;
 
 #[cfg(feature = "arbitrary")]
 mod arbitrary;
 mod attrs;
 mod builtin;
 mod function;
+mod json;
 mod list;
 mod path;
 mod string;
@@ -39,7 +40,7 @@ pub use self::thunk::{SharedThunkSet, ThunkSet};
 use lazy_static::lazy_static;
 
 #[warn(variant_size_differences)]
-#[derive(Clone, Debug, Serialize, Deserialize)]
+#[derive(Clone, Debug, Deserialize)]
 #[serde(untagged)]
 pub enum Value {
     Null,
@@ -76,6 +77,8 @@ pub enum Value {
     DeferredUpvalue(StackIdx),
     #[serde(skip)]
     UnresolvedPath(Box<PathBuf>),
+    #[serde(skip)]
+    Json(serde_json::Value),
 }
 
 lazy_static! {
@@ -231,7 +234,8 @@ impl Value {
             Value::AttrNotFound
             | Value::Blueprint(_)
             | Value::DeferredUpvalue(_)
-            | Value::UnresolvedPath(_) => panic!(
+            | Value::UnresolvedPath(_)
+            | Value::Json(_) => panic!(
                 "Tvix bug: internal value left on stack: {}",
                 value.type_of()
             ),
@@ -322,7 +326,8 @@ impl Value {
             (Value::AttrNotFound, _)
             | (Value::Blueprint(_), _)
             | (Value::DeferredUpvalue(_), _)
-            | (Value::UnresolvedPath(_), _) => {
+            | (Value::UnresolvedPath(_), _)
+            | (Value::Json(_), _) => {
                 panic!("tvix bug: .coerce_to_string() called on internal value")
             }
         }
@@ -512,6 +517,7 @@ impl Value {
             Value::Blueprint(_) => "internal[blueprint]",
             Value::DeferredUpvalue(_) => "internal[deferred_upvalue]",
             Value::UnresolvedPath(_) => "internal[unresolved_path]",
+            Value::Json(_) => "internal[json]",
         }
     }
 
@@ -643,7 +649,8 @@ impl Value {
             Value::AttrNotFound
             | Value::Blueprint(_)
             | Value::DeferredUpvalue(_)
-            | Value::UnresolvedPath(_) => "an internal Tvix evaluator value".into(),
+            | Value::UnresolvedPath(_)
+            | Value::Json(_) => "an internal Tvix evaluator value".into(),
         }
     }
 }
@@ -756,6 +763,7 @@ 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]"),
 
             // Delegate thunk display to the type, as it must handle
             // the case of already evaluated or cyclic thunks.
diff --git a/tvix/eval/src/value/thunk.rs b/tvix/eval/src/value/thunk.rs
index 42e8ec869ac5..e990a5cad5cc 100644
--- a/tvix/eval/src/value/thunk.rs
+++ b/tvix/eval/src/value/thunk.rs
@@ -25,8 +25,6 @@ use std::{
     rc::Rc,
 };
 
-use serde::Serialize;
-
 use crate::{
     errors::ErrorKind,
     spans::LightSpan,
@@ -269,15 +267,6 @@ impl TotalDisplay for Thunk {
     }
 }
 
-impl Serialize for Thunk {
-    fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
-    where
-        S: serde::Serializer,
-    {
-        self.value().serialize(serializer)
-    }
-}
-
 /// A wrapper type for tracking which thunks have already been seen in a
 /// context. This is necessary for cycle detection.
 ///
diff --git a/tvix/eval/src/vm/generators.rs b/tvix/eval/src/vm/generators.rs
index 7a1ba0153d24..9b055eb1c826 100644
--- a/tvix/eval/src/vm/generators.rs
+++ b/tvix/eval/src/vm/generators.rs
@@ -114,6 +114,10 @@ pub enum GeneratorRequest {
     /// Request evaluation of `builtins.tryEval` from the VM. See
     /// [`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),
 }
 
 /// Human-readable representation of a generator message, used by observers.
@@ -160,6 +164,7 @@ impl Display for GeneratorRequest {
             GeneratorRequest::ReadDir(p) => write!(f, "read_dir({})", p.to_string_lossy()),
             GeneratorRequest::Span => write!(f, "span"),
             GeneratorRequest::TryForce(v) => write!(f, "try_force({})", v.type_of()),
+            GeneratorRequest::ToJson(v) => write!(f, "to_json({})", v.type_of()),
         }
     }
 }
@@ -440,6 +445,14 @@ impl<'o> VM<'o> {
                             self.enqueue_generator("force", span, |co| value.force(co));
                             return Ok(false);
                         }
+
+                        GeneratorRequest::ToJson(value) => {
+                            self.reenqueue_generator(name, span.clone(), generator);
+                            self.enqueue_generator("to_json", span, |co| {
+                                value.to_json_generator(co)
+                            });
+                            return Ok(false);
+                        }
                     }
                 }
 
@@ -708,6 +721,16 @@ pub(crate) async fn request_span(co: &GenCo) -> LightSpan {
     }
 }
 
+pub(crate) async fn request_to_json(co: &GenCo, value: Value) -> serde_json::Value {
+    match co.yield_(GeneratorRequest::ToJson(value)).await {
+        GeneratorResponse::Value(Value::Json(json)) => json,
+        msg => panic!(
+            "Tvix bug: VM responded with incorrect generator message: {}",
+            msg
+        ),
+    }
+}
+
 /// Call the given value as if it was an attribute set containing a functor. The
 /// arguments must already be prepared on the stack when a generator frame from
 /// this function is invoked.
diff --git a/tvix/serde/src/de.rs b/tvix/serde/src/de.rs
index e6bcf41cf231..0285c386ca2e 100644
--- a/tvix/serde/src/de.rs
+++ b/tvix/serde/src/de.rs
@@ -91,7 +91,8 @@ impl<'de> de::Deserializer<'de> for NixDeserializer {
             | Value::AttrNotFound
             | Value::Blueprint(_)
             | Value::DeferredUpvalue(_)
-            | Value::UnresolvedPath(_) => Err(Error::Unserializable {
+            | Value::UnresolvedPath(_)
+            | Value::Json(_) => Err(Error::Unserializable {
                 value_type: self.value.type_of(),
             }),
         }