From cd280e07964db1a789c44748a1a7e5dc4e5d534e Mon Sep 17 00:00:00 2001
From: sterni <sternenseemann@systemli.org>
Date: Mon, 19 Sep 2022 12:03:50 +0200
Subject: fix(tvix/eval): make sure to force before selecting in catAttrs

Previously, this would almost always crash because list items are
thunked more often nowadays and selecting from a thunk would fail. Also
we no longer pop from args, accessing it by index should avoid an
unnecessary clone here.

Change-Id: I4410c4c2e28cc255a2c7cf2a5322db3d2c556a0e
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6693
Reviewed-by: grfn <grfn@gws.fyi>
Autosubmit: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
---
 tvix/eval/src/builtins/mod.rs                                 | 11 ++++++-----
 .../eval/src/tests/tvix_tests/eval-okay-builtins-catAttrs.exp |  1 +
 .../eval/src/tests/tvix_tests/eval-okay-builtins-catAttrs.nix | 10 ++++++++++
 3 files changed, 17 insertions(+), 5 deletions(-)
 create mode 100644 tvix/eval/src/tests/tvix_tests/eval-okay-builtins-catAttrs.exp
 create mode 100644 tvix/eval/src/tests/tvix_tests/eval-okay-builtins-catAttrs.nix

diff --git a/tvix/eval/src/builtins/mod.rs b/tvix/eval/src/builtins/mod.rs
index 7e55e8bdbf84..18a50f255a50 100644
--- a/tvix/eval/src/builtins/mod.rs
+++ b/tvix/eval/src/builtins/mod.rs
@@ -87,13 +87,14 @@ fn pure_builtins() -> Vec<Builtin> {
         Builtin::new("bitXor", &[true, true], |args, _| {
             Ok(Value::Integer(args[0].as_int()? ^ args[1].as_int()?))
         }),
-        Builtin::new("catAttrs", &[true, true], |mut args, _| {
-            let list = args.pop().unwrap().to_list()?;
-            let key = args.pop().unwrap().to_str()?;
+        Builtin::new("catAttrs", &[true, true], |args, vm| {
+            let key = args[0].to_str()?;
+            let list = args[1].to_list()?;
             let mut output = vec![];
 
-            for set in list.into_iter() {
-                if let Some(value) = set.to_attrs()?.select(key.as_str()) {
+            for item in list.into_iter() {
+                let set = item.force(vm)?.to_attrs()?;
+                if let Some(value) = set.select(key.as_str()) {
                     output.push(value.clone());
                 }
             }
diff --git a/tvix/eval/src/tests/tvix_tests/eval-okay-builtins-catAttrs.exp b/tvix/eval/src/tests/tvix_tests/eval-okay-builtins-catAttrs.exp
new file mode 100644
index 000000000000..f8c0b2de5fba
--- /dev/null
+++ b/tvix/eval/src/tests/tvix_tests/eval-okay-builtins-catAttrs.exp
@@ -0,0 +1 @@
+[ 21 "+" 21 "=" 42 ]
diff --git a/tvix/eval/src/tests/tvix_tests/eval-okay-builtins-catAttrs.nix b/tvix/eval/src/tests/tvix_tests/eval-okay-builtins-catAttrs.nix
new file mode 100644
index 000000000000..edac76d4461a
--- /dev/null
+++ b/tvix/eval/src/tests/tvix_tests/eval-okay-builtins-catAttrs.nix
@@ -0,0 +1,10 @@
+builtins.catAttrs "foo" [
+  { foo = 21; }
+  { bar = 23; foo = "+"; }
+  { }
+  { bar = 12; }
+  { foo = 21 + 0; }
+  { foo = "="; }
+  ({ bar = 13; } // { baz = 89; })
+  { foo = 42; bar = 33; }
+]
-- 
cgit 1.4.1