From f4e17caae88a1180444d8120ba35565e9853199d Mon Sep 17 00:00:00 2001 From: Lyle Mantooth Date: Sat, 3 Dec 2022 00:18:58 -0500 Subject: feat(tvix/eval): Continue removing leakage of BTreeMap. Fixes b/212. Based on feedback in https://cl.tvl.fyi/c/depot/+/7492, all uses of `NixAttrs::from_map` have been removed. Only `from_iter` and `from_kv` remain. Change-Id: I52e25f73018c2aa1843197427516b7a852503e2c Reviewed-on: https://cl.tvl.fyi/c/depot/+/7500 Reviewed-by: tazjin Tested-by: BuildkiteCI Autosubmit: IslandUsurper --- tvix/eval/src/builtins/impure.rs | 48 +++++++++++++++++++--------------------- tvix/eval/src/builtins/mod.rs | 31 ++++++++++++-------------- tvix/eval/src/value/attrs.rs | 8 ------- tvix/eval/src/value/mod.rs | 5 +++-- 4 files changed, 40 insertions(+), 52 deletions(-) diff --git a/tvix/eval/src/builtins/impure.rs b/tvix/eval/src/builtins/impure.rs index dc62e93ada40..ee6cb2afdee4 100644 --- a/tvix/eval/src/builtins/impure.rs +++ b/tvix/eval/src/builtins/impure.rs @@ -40,31 +40,29 @@ mod impure_builtins { error: Rc::new(err), }; - let mut res = BTreeMap::new(); - for entry in path.read_dir().map_err(mk_err)? { - let entry = entry.map_err(mk_err)?; - let file_type = entry - .metadata() - .map_err(|err| ErrorKind::IO { - path: Some(entry.path()), - error: Rc::new(err), - })? - .file_type(); - let val = if file_type.is_dir() { - "directory" - } else if file_type.is_file() { - "regular" - } else if file_type.is_symlink() { - "symlink" - } else { - "unknown" - }; - res.insert( - entry.file_name().to_string_lossy().as_ref().into(), - val.into(), - ); - } - Ok(Value::attrs(NixAttrs::from_map(res))) + let res = path.read_dir().map_err(mk_err)?.into_iter().flat_map( + |entry| -> Result<(String, &str), ErrorKind> { + let entry = entry.map_err(mk_err)?; + let file_type = entry + .metadata() + .map_err(|err| ErrorKind::IO { + path: Some(entry.path()), + error: Rc::new(err), + })? + .file_type(); + let val = if file_type.is_dir() { + "directory" + } else if file_type.is_file() { + "regular" + } else if file_type.is_symlink() { + "symlink" + } else { + "unknown" + }; + Ok((entry.file_name().to_string_lossy().to_string(), val)) + }, + ); + Ok(Value::attrs(NixAttrs::from_iter(res))) } #[builtin("readFile")] diff --git a/tvix/eval/src/builtins/mod.rs b/tvix/eval/src/builtins/mod.rs index 536fc9082b87..f75acd48ad31 100644 --- a/tvix/eval/src/builtins/mod.rs +++ b/tvix/eval/src/builtins/mod.rs @@ -16,7 +16,7 @@ use regex::Regex; use crate::warnings::WarningKind; use crate::{ - errors::ErrorKind, + errors::{ErrorKind, EvalResult}, value::{Builtin, CoercionKind, NixAttrs, NixList, NixString, Value}, vm::VM, }; @@ -329,12 +329,8 @@ mod pure_builtins { } else { return Ok(Value::attrs(NixAttrs::empty())); }; - Ok(Value::attrs(NixAttrs::from_map( - formals - .arguments - .iter() - .map(|(k, v)| (k.clone(), (*v).into())) - .collect(), + Ok(Value::attrs(NixAttrs::from_iter( + formals.arguments.iter().map(|(k, v)| (k.clone(), (*v))), ))) } @@ -543,7 +539,7 @@ mod pure_builtins { // Map entries earlier in the list take precedence over entries later in the list map.entry(name).or_insert(value); } - Ok(Value::attrs(NixAttrs::from_map(map))) + Ok(Value::attrs(NixAttrs::from_iter(map.into_iter()))) } #[builtin("map")] @@ -560,12 +556,15 @@ mod pure_builtins { #[builtin("mapAttrs")] fn builtin_map_attrs(vm: &mut VM, f: Value, attrs: Value) -> Result { let attrs = attrs.to_attrs()?; - let mut res = BTreeMap::new(); - for (key, value) in attrs.as_ref() { - let value = vm.call_with(&f, [key.clone().into(), value.clone()])?; - res.insert(key.clone(), value); - } - Ok(Value::attrs(NixAttrs::from_map(res))) + let res = + attrs + .as_ref() + .into_iter() + .flat_map(|(key, value)| -> EvalResult<(NixString, Value)> { + let value = vm.call_with(&f, [key.clone().into(), value.clone()])?; + Ok((key.to_owned(), value)) + }); + Ok(Value::attrs(NixAttrs::from_iter(res))) } #[builtin("match")] @@ -1081,9 +1080,7 @@ pub fn global_builtins(source: SourceCode) -> GlobalsMapFunc { let mut globals: GlobalsMap = HashMap::new(); - let builtins = Rc::new(NixAttrs::from_map( - map.into_iter().map(|(k, v)| (k.into(), v)).collect(), - )); + let builtins = Rc::new(NixAttrs::from_iter(map.into_iter())); // known global builtins from the builtins set. for global in &[ diff --git a/tvix/eval/src/value/attrs.rs b/tvix/eval/src/value/attrs.rs index 0ef28f068cf2..ecce34fb4af4 100644 --- a/tvix/eval/src/value/attrs.rs +++ b/tvix/eval/src/value/attrs.rs @@ -345,14 +345,6 @@ impl NixAttrs { Ok(attrs) } - /// Construct an attribute set directly from a BTreeMap - /// representation. This is only visible inside of the crate, as - /// it is intended exclusively for use with the construction of - /// global sets for the compiler. - pub(crate) fn from_map(map: BTreeMap) -> Self { - NixAttrs(AttrsRep::Map(map)) - } - /// Construct an optimized "KV"-style attribute set given the value for the /// `"name"` key, and the value for the `"value"` key pub(crate) fn from_kv(name: Value, value: Value) -> Self { diff --git a/tvix/eval/src/value/mod.rs b/tvix/eval/src/value/mod.rs index f0e3b84b9251..2bca9e6d3202 100644 --- a/tvix/eval/src/value/mod.rs +++ b/tvix/eval/src/value/mod.rs @@ -526,10 +526,11 @@ impl TryFrom for Value { name.clone().try_into()?, value.clone().try_into()?, ))), - _ => Ok(Self::attrs(NixAttrs::from_map( + _ => Ok(Self::attrs(NixAttrs::from_iter( obj.into_iter() .map(|(k, v)| Ok((k.into(), v.try_into()?))) - .collect::>()?, + .collect::, ErrorKind>>()? + .into_iter(), ))), } } -- cgit 1.4.1