From 39dba6aa1624b78690ade7f1b657283b48b56b39 Mon Sep 17 00:00:00 2001 From: Lyle Mantooth Date: Mon, 28 Nov 2022 09:48:29 -0500 Subject: feat(tvix/eval): impl FromIterator for NixAttrs Allows for the removal of some BTreeMap usage when constructing NixAttrs by allowing any iterator over 2-tuples to build a NixAttrs. Some instances of BTreeMap didn't have anything to do with making NixAttrs, and some were just the best tool for the job, so they are left using the old `from_map` interface. Change-Id: I668ea600b0d93eae700a6b1861ac84502c968d78 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7492 Tested-by: BuildkiteCI Reviewed-by: tazjin --- tvix/eval/src/builtins/mod.rs | 98 ++++++++++++++++++++----------------------- tvix/eval/src/value/attrs.rs | 18 ++++++++ 2 files changed, 64 insertions(+), 52 deletions(-) (limited to 'tvix/eval') diff --git a/tvix/eval/src/builtins/mod.rs b/tvix/eval/src/builtins/mod.rs index 21509cb10d8f..536fc9082b87 100644 --- a/tvix/eval/src/builtins/mod.rs +++ b/tvix/eval/src/builtins/mod.rs @@ -420,10 +420,9 @@ mod pure_builtins { let key = vm.call_with(&f, [val.clone()])?.force(vm)?.to_str()?; res.entry(key).or_insert_with(|| vec![]).push(val); } - Ok(Value::attrs(NixAttrs::from_map( + Ok(Value::attrs(NixAttrs::from_iter( res.into_iter() - .map(|(k, v)| (k, Value::List(NixList::from(v)))) - .collect(), + .map(|(k, v)| (k, Value::List(NixList::from(v)))), ))) } @@ -445,15 +444,16 @@ mod pure_builtins { #[builtin("intersectAttrs")] fn builtin_intersect_attrs(_: &mut VM, x: Value, y: Value) -> Result { - let mut res = BTreeMap::new(); let attrs1 = x.to_attrs()?; let attrs2 = y.to_attrs()?; - for (k, v) in attrs2.iter() { + let res = attrs2.iter().filter_map(|(k, v)| { if attrs1.contains(k) { - res.insert(k.clone(), v.clone()); + Some((k.clone(), v.clone())) + } else { + None } - } - Ok(Value::attrs(NixAttrs::from_map(res))) + }); + Ok(Value::attrs(NixAttrs::from_iter(res))) } // For `is*` predicates we force manually, as Value::force also unwraps any Thunks @@ -608,12 +608,10 @@ mod pure_builtins { let version = dash_and_version .split_first() .map(|x| core::str::from_utf8(x.1)) - .unwrap_or(Ok(""))? - .into(); - Ok(Value::attrs(NixAttrs::from_map(BTreeMap::from([ - (NixString::NAME, core::str::from_utf8(name)?.into()), - ("version".into(), version), - ])))) + .unwrap_or(Ok(""))?; + Ok(Value::attrs(NixAttrs::from_iter( + [("name", core::str::from_utf8(name)?), ("version", version)].into_iter(), + ))) } #[builtin("partition")] fn builtin_partition(vm: &mut VM, pred: Value, list: Value) -> Result { @@ -621,7 +619,7 @@ mod pure_builtins { let mut wrong: Vec = vec![]; let list: NixList = list.to_list()?; - for elem in list.into_iter() { + for elem in list { let result = vm.call_with(&pred, [elem.clone()])?; if result.force(vm)?.as_bool()? { @@ -631,11 +629,9 @@ mod pure_builtins { }; } - let mut res: BTreeMap = BTreeMap::new(); - res.insert("right".into(), Value::List(right.into())); - res.insert("wrong".into(), Value::List(wrong.into())); + let res = [("right", right), ("wrong", wrong)]; - Ok(Value::attrs(NixAttrs::from_map(res))) + Ok(Value::attrs(NixAttrs::from_iter(res.into_iter()))) } #[builtin("removeAttrs")] @@ -646,13 +642,14 @@ mod pure_builtins { .into_iter() .map(|v| v.to_str()) .collect::, _>>()?; - let mut res = BTreeMap::new(); - for (k, v) in attrs.iter() { + let res = attrs.iter().filter_map(|(k, v)| { if !keys.contains(k) { - res.insert(k.clone(), v.clone()); + Some((k.clone(), v.clone())) + } else { + None } - } - Ok(Value::attrs(NixAttrs::from_map(res))) + }); + Ok(Value::attrs(NixAttrs::from_iter(res))) } #[builtin("replaceStrings")] @@ -922,19 +919,12 @@ mod pure_builtins { #[builtin("tryEval")] fn builtin_try_eval(vm: &mut VM, #[lazy] e: Value) -> Result { - let mut res = BTreeMap::new(); - match e.force(vm) { - Ok(value) => { - res.insert("value".into(), (*value).clone()); - res.insert("success".into(), true.into()); - } - Err(e) if e.is_catchable() => { - res.insert("value".into(), false.into()); - res.insert("success".into(), false.into()); - } + let res = match e.force(vm) { + Ok(value) => [("value", (*value).clone()), ("success", true.into())], + Err(e) if e.is_catchable() => [("value", false.into()), ("success", false.into())], Err(e) => return Err(e), - } - Ok(Value::attrs(NixAttrs::from_map(res))) + }; + Ok(Value::attrs(NixAttrs::from_iter(res.into_iter()))) } #[builtin("typeOf")] @@ -1005,11 +995,12 @@ fn placeholders() -> Vec { vm.emit_warning(WarningKind::NotImplemented("builtins.unsafeGetAttrsPos")); let _attrset = args.pop().unwrap().to_attrs(); let _name = args.pop().unwrap().to_str(); - let mut res: BTreeMap = BTreeMap::new(); - res.insert("line".into(), 42.into()); - res.insert("col".into(), 42.into()); - res.insert("file".into(), Value::Path("/deep/thought".into())); - Ok(Value::attrs(NixAttrs::from_map(res))) + let res = [ + ("line", 42.into()), + ("col", 42.into()), + ("file", Value::Path("/deep/thought".into())), + ]; + Ok(Value::attrs(NixAttrs::from_iter(res.into_iter()))) }, ), Builtin::new( @@ -1027,17 +1018,20 @@ fn placeholders() -> Vec { // // Crucially this means we do not yet *validate* the values either. let attrs = unwrap_or_clone_rc(args[0].to_attrs()?); - let attrs = attrs.update(NixAttrs::from_map(BTreeMap::from([ - ( - "outPath".into(), - "/nix/store/00000000000000000000000000000000-mock".into(), - ), - ( - "drvPath".into(), - "/nix/store/00000000000000000000000000000000-mock.drv".into(), - ), - ("type".into(), "derivation".into()), - ]))); + let attrs = attrs.update(NixAttrs::from_iter( + [ + ( + "outPath", + "/nix/store/00000000000000000000000000000000-mock", + ), + ( + "drvPath", + "/nix/store/00000000000000000000000000000000-mock.drv", + ), + ("type", "derivation"), + ] + .into_iter(), + )); Ok(Value::Attrs(Rc::new(attrs))) }, diff --git a/tvix/eval/src/value/attrs.rs b/tvix/eval/src/value/attrs.rs index fced32a865a6..0ef28f068cf2 100644 --- a/tvix/eval/src/value/attrs.rs +++ b/tvix/eval/src/value/attrs.rs @@ -7,6 +7,7 @@ //! some peculiarities that are encapsulated within this module. use std::collections::btree_map; use std::collections::BTreeMap; +use std::iter::FromIterator; use crate::errors::ErrorKind; use crate::vm::VM; @@ -88,6 +89,23 @@ impl AttrsRep { #[derive(Clone, Debug, Default)] pub struct NixAttrs(AttrsRep); +impl FromIterator<(K, V)> for NixAttrs +where + NixString: From, + Value: From, +{ + fn from_iter(iter: T) -> NixAttrs + where + T: IntoIterator, + { + NixAttrs(AttrsRep::Map( + iter.into_iter() + .map(|(k, v)| (k.into(), v.into())) + .collect(), + )) + } +} + impl TotalDisplay for NixAttrs { fn total_fmt(&self, f: &mut std::fmt::Formatter<'_>, set: &mut ThunkSet) -> std::fmt::Result { f.write_str("{ ")?; -- cgit 1.4.1