fix(tvix/eval): type check function argument with set pattern r/6243

C++ Nix forces and typechecks the passed argument even if it is not
necessary in order to compute the return value of the function. I
discovered this when I thought our formals miscompilation might be that
we are too strict, but doesn't look like it in this case.

Change-Id: Ifb3c92592293052c489d1e3ae8c7c54e4b6b4dc6
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8701
Tested-by: BuildkiteCI
Autosubmit: sterni <sternenseemann@systemli.org>
Reviewed-by: tazjin <tazjin@tvl.su>

5 files changed, 18 insertions, 0 deletions

diff --git a/tvix/eval/src/compiler/mod.rs b/tvix/eval/src/compiler/mod.rs
index 75d9d94475a2..450799d683d1 100644
--- a/tvix/eval/src/compiler/mod.rs
+++ b/tvix/eval/src/compiler/mod.rs
@@ -910,6 +910,7 @@ impl Compiler<'_> {
         // the stack.
         self.scope_mut().mark_initialised(set_idx);
         self.emit_force(pattern);
+        self.push_op(OpCode::OpAssertAttrs, pattern);
 
         let ellipsis = pattern.ellipsis_token().is_some();
         if !ellipsis {
diff --git a/tvix/eval/src/opcode.rs b/tvix/eval/src/opcode.rs
index 130e242668d6..e3eddc3c9ad0 100644
--- a/tvix/eval/src/opcode.rs
+++ b/tvix/eval/src/opcode.rs
@@ -129,6 +129,7 @@ pub enum OpCode {
 
     // Type assertion operators
     OpAssertBool,
+    OpAssertAttrs,
 
     /// Access local identifiers with statically known positions.
     OpGetLocal(StackIdx),
diff --git a/tvix/eval/src/tests/tvix_tests/eval-fail-function-formals-typecheck.nix b/tvix/eval/src/tests/tvix_tests/eval-fail-function-formals-typecheck.nix
new file mode 100644
index 000000000000..0108f958bf89
--- /dev/null
+++ b/tvix/eval/src/tests/tvix_tests/eval-fail-function-formals-typecheck.nix
@@ -0,0 +1,2 @@
+# A function with formal set arguments forces its argument set and verifies its type.
+({ ... }@args: 42) [ ]
diff --git a/tvix/eval/src/value/mod.rs b/tvix/eval/src/value/mod.rs
index 77d40b0e3906..34353df3a72c 100644
--- a/tvix/eval/src/value/mod.rs
+++ b/tvix/eval/src/value/mod.rs
@@ -542,6 +542,7 @@ impl Value {
     gen_is!(is_path, Value::Path(_));
     gen_is!(is_number, Value::Integer(_) | Value::Float(_));
     gen_is!(is_bool, Value::Bool(_));
+    gen_is!(is_attrs, Value::Attrs(_));
 
     // TODO(amjoseph): de-asyncify this (when called directly by the VM)
     /// Compare `self` against other using (fallible) Nix ordering semantics.
diff --git a/tvix/eval/src/vm/mod.rs b/tvix/eval/src/vm/mod.rs
index 4af23a72d73b..9f973eb120e9 100644
--- a/tvix/eval/src/vm/mod.rs
+++ b/tvix/eval/src/vm/mod.rs
@@ -638,6 +638,19 @@ impl<'o> VM<'o> {
                     }
                 }
 
+                OpCode::OpAssertAttrs => {
+                    let val = self.stack_peek(0);
+                    if !val.is_attrs() {
+                        return frame.error(
+                            self,
+                            ErrorKind::TypeError {
+                                expected: "set",
+                                actual: val.type_of(),
+                            },
+                        );
+                    }
+                }
+
                 OpCode::OpAttrs(Count(count)) => self.run_attrset(&frame, count)?,
 
                 OpCode::OpAttrsUpdate => {