feat(tvix/eval): Continue removing leakage of BTreeMap. r/5379

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 <tazjin@tvl.su>
Tested-by: BuildkiteCI
Autosubmit: IslandUsurper <lyle@menteeth.us>

4 files changed, 40 insertions, 52 deletions

diff --git a/tvix/eval/src/builtins/impure.rs b/tvix/eval/src/builtins/impure.rs
index dc62e93ada..ee6cb2afde 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 536fc9082b..f75acd48ad 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<Value, ErrorKind> {
         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 0ef28f068c..ecce34fb4a 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<NixString, Value>) -> 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 f0e3b84b92..2bca9e6d32 100644
--- a/tvix/eval/src/value/mod.rs
+++ b/tvix/eval/src/value/mod.rs
@@ -526,10 +526,11 @@ impl TryFrom<serde_json::Value> 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::<Result<_, ErrorKind>>()?,
+                            .collect::<Result<Vec<(NixString, Value)>, ErrorKind>>()?
+                            .into_iter(),
                     ))),
                 }
             }