From 805219a2fad0edac10d046fc5ad5820edb4482ee Mon Sep 17 00:00:00 2001 From: Ryan Lahfa Date: Sat, 24 Dec 2022 18:18:26 +0100 Subject: [PATCH] feat(tvix/eval): implement serde::Deserialize for Value Co-Authored-By: Vincent Ambo Change-Id: Ib6f7d1f4f4faac36b44f5f75cccc57bf912cf606 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7626 Reviewed-by: tazjin Tested-by: BuildkiteCI --- tvix/Cargo.lock | 1 + tvix/Cargo.nix | 16 ++++- tvix/eval/Cargo.toml | 4 +- tvix/eval/src/builtins/mod.rs | 4 +- .../tests/tvix_tests/eval-okay-fromjson.exp | 2 +- .../tests/tvix_tests/eval-okay-fromjson.nix | 1 + tvix/eval/src/value/attrs.rs | 37 +++++++++++- tvix/eval/src/value/list.rs | 4 +- tvix/eval/src/value/mod.rs | 59 ++++++------------- tvix/eval/src/value/string.rs | 36 +++++++++++ 10 files changed, 114 insertions(+), 50 deletions(-) diff --git a/tvix/Cargo.lock b/tvix/Cargo.lock index 604e16bdc..197bee75e 100644 --- a/tvix/Cargo.lock +++ b/tvix/Cargo.lock @@ -840,6 +840,7 @@ dependencies = [ "imbl-sized-chunks", "rand_core 0.6.4", "rand_xoshiro", + "serde", "version_check", ] diff --git a/tvix/Cargo.nix b/tvix/Cargo.nix index 977def1c7..9f13a1607 100644 --- a/tvix/Cargo.nix +++ b/tvix/Cargo.nix @@ -2453,6 +2453,11 @@ rec { name = "rand_xoshiro"; packageId = "rand_xoshiro"; } + { + name = "serde"; + packageId = "serde"; + optional = true; + } ]; buildDependencies = [ { @@ -2460,6 +2465,12 @@ rec { packageId = "version_check"; } ]; + devDependencies = [ + { + name = "serde"; + packageId = "serde"; + } + ]; features = { "arbitrary" = [ "dep:arbitrary" ]; "proptest" = [ "dep:proptest" ]; @@ -2468,6 +2479,7 @@ rec { "refpool" = [ "dep:refpool" ]; "serde" = [ "dep:serde" ]; }; + resolvedDefaultFeatures = [ "serde" ]; }; "imbl-sized-chunks" = rec { crateName = "imbl-sized-chunks"; @@ -4700,7 +4712,7 @@ rec { "derive" = [ "serde_derive" ]; "serde_derive" = [ "dep:serde_derive" ]; }; - resolvedDefaultFeatures = [ "alloc" "default" "derive" "serde_derive" "std" ]; + resolvedDefaultFeatures = [ "alloc" "default" "derive" "rc" "serde_derive" "std" ]; }; "serde_derive" = rec { crateName = "serde_derive"; @@ -6570,6 +6582,7 @@ rec { { name = "imbl"; packageId = "imbl"; + features = [ "serde" ]; } { name = "path-clean"; @@ -6597,6 +6610,7 @@ rec { { name = "serde"; packageId = "serde"; + features = [ "rc" "derive" ]; } { name = "serde_json"; diff --git a/tvix/eval/Cargo.toml b/tvix/eval/Cargo.toml index b866ec2c7..75c45c96b 100644 --- a/tvix/eval/Cargo.toml +++ b/tvix/eval/Cargo.toml @@ -14,13 +14,13 @@ builtin-macros = { path = "./builtin-macros", package = "tvix-eval-builtin-macro codemap = "0.1.3" codemap-diagnostic = "0.1.1" dirs = "4.0.0" -imbl = "2.0" +imbl = { version = "2.0", features = [ "serde" ] } path-clean = "0.1" proptest = { version = "1.0.0", default_features = false, features = ["std", "alloc", "break-dead-code", "tempfile"], optional = true } regex = "1.6.0" rnix = "0.11.0" rowan = "*" # pinned by rnix -serde = "1.0" +serde = { version = "1.0", features = [ "rc", "derive" ] } serde_json = "1.0" smol_str = "0.1" tabwriter = "1.2" diff --git a/tvix/eval/src/builtins/mod.rs b/tvix/eval/src/builtins/mod.rs index 770dfe5ba..9c9d171a2 100644 --- a/tvix/eval/src/builtins/mod.rs +++ b/tvix/eval/src/builtins/mod.rs @@ -339,8 +339,8 @@ mod pure_builtins { #[builtin("fromJSON")] fn builtin_from_json(_: &mut VM, json: Value) -> Result { let json_str = json.to_str()?; - let json: serde_json::Value = serde_json::from_str(&json_str)?; - json.try_into() + + serde_json::from_str(&json_str).map_err(|err| err.into()) } #[builtin("genericClosure")] diff --git a/tvix/eval/src/tests/tvix_tests/eval-okay-fromjson.exp b/tvix/eval/src/tests/tvix_tests/eval-okay-fromjson.exp index 4f75c0923..c855950a3 100644 --- a/tvix/eval/src/tests/tvix_tests/eval-okay-fromjson.exp +++ b/tvix/eval/src/tests/tvix_tests/eval-okay-fromjson.exp @@ -1 +1 @@ -[ { Image = { Animated = false; Height = 600; IDs = [ 116 943 234 38793 true false null -100 ]; Latitude = 37.7668; Longitude = -122.3959; Thumbnail = { Height = 125; Url = "http://www.example.com/image/481989943"; Width = 100; }; Title = "View from 15th Floor"; Width = 800; }; } { name = "a"; value = "b"; } ] +[ { Image = { Animated = false; Height = 600; IDs = [ 116 943 234 38793 true false null -100 ]; Latitude = 37.7668; Longitude = -122.3959; Thumbnail = { Height = 125; Url = "http://www.example.com/image/481989943"; Width = 100; }; Title = "View from 15th Floor"; Width = 800; }; } { name = "a"; value = "b"; } [ 1 2 3 4 ] ] diff --git a/tvix/eval/src/tests/tvix_tests/eval-okay-fromjson.nix b/tvix/eval/src/tests/tvix_tests/eval-okay-fromjson.nix index ccb83fd0b..e4f621312 100644 --- a/tvix/eval/src/tests/tvix_tests/eval-okay-fromjson.nix +++ b/tvix/eval/src/tests/tvix_tests/eval-okay-fromjson.nix @@ -20,4 +20,5 @@ } '') (builtins.fromJSON ''{"name": "a", "value": "b"}'') + (builtins.fromJSON "[ 1, 2, 3, 4 ]") ] diff --git a/tvix/eval/src/value/attrs.rs b/tvix/eval/src/value/attrs.rs index a41e7ce58..c6b274f0b 100644 --- a/tvix/eval/src/value/attrs.rs +++ b/tvix/eval/src/value/attrs.rs @@ -8,6 +8,8 @@ use std::iter::FromIterator; use imbl::{ordmap, OrdMap}; +use serde::de::{Deserializer, Error, Visitor}; +use serde::Deserialize; use crate::errors::ErrorKind; use crate::vm::VM; @@ -20,7 +22,7 @@ use super::Value; #[cfg(test)] mod tests; -#[derive(Clone, Debug)] +#[derive(Clone, Debug, Deserialize)] enum AttrsRep { Empty, @@ -138,6 +140,39 @@ impl TotalDisplay for NixAttrs { } } +impl<'de> Deserialize<'de> for NixAttrs { + fn deserialize(deserializer: D) -> Result + where + D: Deserializer<'de>, + { + struct MapVisitor; + + impl<'de> Visitor<'de> for MapVisitor { + type Value = NixAttrs; + + fn expecting(&self, formatter: &mut std::fmt::Formatter) -> std::fmt::Result { + formatter.write_str("a valid Nix attribute set") + } + + fn visit_map(self, mut map: A) -> Result + where + A: serde::de::MapAccess<'de>, + { + let mut stack_array = Vec::with_capacity(map.size_hint().unwrap_or(0) * 2); + + while let Some((key, value)) = map.next_entry()? { + stack_array.push(key); + stack_array.push(value); + } + + NixAttrs::construct(stack_array.len() / 2, stack_array).map_err(A::Error::custom) + } + } + + deserializer.deserialize_map(MapVisitor) + } +} + #[cfg(feature = "arbitrary")] mod arbitrary { use super::*; diff --git a/tvix/eval/src/value/list.rs b/tvix/eval/src/value/list.rs index fa1f266c8..744130d2a 100644 --- a/tvix/eval/src/value/list.rs +++ b/tvix/eval/src/value/list.rs @@ -3,6 +3,8 @@ use std::ops::Index; use imbl::{vector, Vector}; +use serde::Deserialize; + use crate::errors::ErrorKind; use crate::vm::VM; @@ -11,7 +13,7 @@ use super::TotalDisplay; use super::Value; #[repr(transparent)] -#[derive(Clone, Debug)] +#[derive(Clone, Debug, Deserialize)] pub struct NixList(Vector); impl TotalDisplay for NixList { diff --git a/tvix/eval/src/value/mod.rs b/tvix/eval/src/value/mod.rs index 49ab62fd1..89e8fdd09 100644 --- a/tvix/eval/src/value/mod.rs +++ b/tvix/eval/src/value/mod.rs @@ -6,6 +6,8 @@ use std::path::PathBuf; use std::rc::Rc; use std::{cell::Ref, fmt::Display}; +use serde::Deserialize; + #[cfg(feature = "arbitrary")] mod arbitrary; mod attrs; @@ -31,30 +33,41 @@ pub use thunk::Thunk; use self::thunk::ThunkSet; #[warn(variant_size_differences)] -#[derive(Clone, Debug)] +#[derive(Clone, Debug, Deserialize)] +#[serde(untagged)] pub enum Value { Null, Bool(bool), Integer(i64), Float(f64), String(NixString), + + #[serde(skip)] Path(PathBuf), Attrs(Box), List(NixList), + + #[serde(skip)] Closure(Rc), // must use Rc here in order to get proper pointer equality + #[serde(skip)] Builtin(Builtin), // Internal values that, while they technically exist at runtime, // are never returned to or created directly by users. + #[serde(skip)] Thunk(Thunk), // See [`compiler::compile_select_or()`] for explanation + #[serde(skip)] AttrNotFound, // this can only occur in Chunk::Constants and nowhere else + #[serde(skip)] Blueprint(Rc), + #[serde(skip)] DeferredUpvalue(StackIdx), + #[serde(skip)] UnresolvedPath(PathBuf), } @@ -542,47 +555,9 @@ impl From for Value { } } -impl TryFrom for Value { - type Error = ErrorKind; - - fn try_from(value: serde_json::Value) -> Result { - // TODO(grfn): Replace with a real serde::Deserialize impl (for perf) - match value { - serde_json::Value::Null => Ok(Self::Null), - serde_json::Value::Bool(b) => Ok(Self::Bool(b)), - serde_json::Value::Number(n) => { - if let Some(i) = n.as_i64() { - Ok(Self::Integer(i)) - } else if let Some(f) = n.as_f64() { - Ok(Self::Float(f)) - } else { - Err(ErrorKind::FromJsonError(format!( - "JSON number not representable as Nix value: {n}" - ))) - } - } - serde_json::Value::String(s) => Ok(s.into()), - serde_json::Value::Array(a) => Ok(Value::List( - a.into_iter() - .map(Value::try_from) - .collect::, _>>()? - .into(), - )), - serde_json::Value::Object(obj) => { - match (obj.len(), obj.get("name"), obj.get("value")) { - (2, Some(name), Some(value)) => Ok(Self::attrs(NixAttrs::from_kv( - name.clone().try_into()?, - value.clone().try_into()?, - ))), - _ => Ok(Self::attrs(NixAttrs::from_iter( - obj.into_iter() - .map(|(k, v)| Ok((k.into(), v.try_into()?))) - .collect::, ErrorKind>>()? - .into_iter(), - ))), - } - } - } +impl From> for Value { + fn from(val: Vec) -> Self { + Self::List(NixList::from_vec(val)) } } diff --git a/tvix/eval/src/value/string.rs b/tvix/eval/src/value/string.rs index 9ebdf687d..93cbc98da 100644 --- a/tvix/eval/src/value/string.rs +++ b/tvix/eval/src/value/string.rs @@ -8,6 +8,9 @@ use std::ops::Deref; use std::path::Path; use std::{borrow::Cow, fmt::Display, str::Chars}; +use serde::de::{Deserializer, Visitor}; +use serde::Deserialize; + #[derive(Clone, Debug)] enum StringRepr { Smol(SmolStr), @@ -68,6 +71,39 @@ impl Hash for NixString { } } +impl<'de> Deserialize<'de> for NixString { + fn deserialize(deserializer: D) -> Result + where + D: Deserializer<'de>, + { + struct StringVisitor; + + impl<'de> Visitor<'de> for StringVisitor { + type Value = NixString; + + fn expecting(&self, formatter: &mut std::fmt::Formatter) -> std::fmt::Result { + formatter.write_str("a valid Nix string") + } + + fn visit_string(self, v: String) -> Result + where + E: serde::de::Error, + { + Ok(v.into()) + } + + fn visit_str(self, v: &str) -> Result + where + E: serde::de::Error, + { + Ok(v.into()) + } + } + + deserializer.deserialize_string(StringVisitor) + } +} + #[cfg(feature = "arbitrary")] mod arbitrary { use super::*;