From 08b4d65fbd0ab1f5809aa2f6eb5da819d61299c4 Mon Sep 17 00:00:00 2001 From: Vincent Ambo Date: Wed, 10 Aug 2022 17:30:02 +0300 Subject: feat(tvix/value): implement nested attribute set literals With this change, nested attribute sets can now be created from literals. This required some logic for dealing with cases where at a deeper nesting point a literal attribute set was constructed from an optimised representation. For example, this is valid Nix code: ```nix { a = {}; # creates optimised empty representation a.b = 1; # wants to add a `b = 1` to it b = { name = "foo"; value = "bar"; }; # creates optimised K/V repr b.foo = 42; # wants to add an additional `foo = 42` } ``` In these cases, the attribute set must be coerced to a map representation first which is achieved by the new internal NixAttr::map_mut helper. Change-Id: Ia61d3d9d14c4e0f5e207c00f6a2f4daa3265afb2 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6109 Reviewed-by: eta Tested-by: BuildkiteCI --- tvix/eval/src/value/attrs.rs | 58 +++++++++++++++++++++++++++++++++----------- 1 file changed, 44 insertions(+), 14 deletions(-) (limited to 'tvix') diff --git a/tvix/eval/src/value/attrs.rs b/tvix/eval/src/value/attrs.rs index c9c5e1780a3c..209f97778c60 100644 --- a/tvix/eval/src/value/attrs.rs +++ b/tvix/eval/src/value/attrs.rs @@ -14,7 +14,7 @@ use crate::errors::{Error, EvalResult}; use super::string::NixString; use super::Value; -#[derive(Debug)] +#[derive(Clone, Debug)] pub enum NixAttrs { Empty, Map(BTreeMap), @@ -55,6 +55,33 @@ impl PartialEq for NixAttrs { } impl NixAttrs { + /// Retrieve reference to a mutable map inside of an attrs, + /// optionally changing the representation if required. + fn map_mut(&mut self) -> &mut BTreeMap { + match self { + NixAttrs::Map(m) => m, + + NixAttrs::Empty => { + *self = NixAttrs::Map(BTreeMap::new()); + self.map_mut() + } + + NixAttrs::KV { name, value } => { + *self = NixAttrs::Map(BTreeMap::from([ + ( + NixString("name".into()), + std::mem::replace(name, Value::Blackhole), + ), + ( + NixString("value".into()), + std::mem::replace(value, Value::Blackhole), + ), + ])); + self.map_mut() + } + } + } + /// Implement construction logic of an attribute set, to encapsulate /// logic about attribute set optimisations inside of this module. pub fn construct(count: usize, mut stack_slice: Vec) -> EvalResult { @@ -78,7 +105,7 @@ impl NixAttrs { } // TODO(tazjin): extend_reserve(count) (rust#72631) - let mut attrs: BTreeMap = BTreeMap::new(); + let mut attrs = NixAttrs::Map(BTreeMap::new()); for _ in 0..count { let value = stack_slice.pop().unwrap(); @@ -107,7 +134,7 @@ impl NixAttrs { } } - Ok(NixAttrs::Map(attrs)) + Ok(attrs) } } @@ -151,12 +178,9 @@ fn attempt_optimise_kv(slice: &mut [Value]) -> Option { } // Set an attribute on an in-construction attribute set, while -// checking against duplicate key.s -fn set_attr( - attrs: &mut BTreeMap, - key: NixString, - value: Value, -) -> EvalResult<()> { +// checking against duplicate keys. +fn set_attr(attrs: &mut NixAttrs, key: NixString, value: Value) -> EvalResult<()> { + let attrs = attrs.map_mut(); let entry = attrs.entry(key); match entry { @@ -180,7 +204,7 @@ fn set_attr( // There is some optimisation potential for this simple implementation // if it becomes a problem. fn set_nested_attr( - attrs: &mut BTreeMap, + attrs: &mut NixAttrs, key: NixString, mut path: Vec, value: Value, @@ -191,6 +215,7 @@ fn set_nested_attr( return set_attr(attrs, key, value); } + let attrs = attrs.map_mut(); let entry = attrs.entry(key); // If there is not we go one step further down, in which case we @@ -202,21 +227,26 @@ fn set_nested_attr( match entry { // Vacant entry -> new attribute set is needed. std::collections::btree_map::Entry::Vacant(entry) => { - let mut map = BTreeMap::new(); + let mut map = NixAttrs::Map(BTreeMap::new()); // TODO(tazjin): technically recursing further is not // required, we can create the whole hierarchy here, but // it's noisy. set_nested_attr(&mut map, path.pop().expect("next key exists"), path, value)?; - entry.insert(Value::Attrs(Rc::new(NixAttrs::Map(map)))); + entry.insert(Value::Attrs(Rc::new(map))); } // Occupied entry: Either error out if there is something // other than attrs, or insert the next value. std::collections::btree_map::Entry::Occupied(mut entry) => match entry.get_mut() { - Value::Attrs(_attrs) => { - todo!("implement mutable attrsets") + Value::Attrs(attrs) => { + set_nested_attr( + Rc::make_mut(attrs), + path.pop().expect("next key exists"), + path, + value, + )?; } _ => { -- cgit 1.4.1