From 10c6cb7251480ca12e67b3d237740e6dcb93f87e Mon Sep 17 00:00:00 2001 From: sterni Date: Fri, 2 Jun 2023 22:38:00 +0200 Subject: fix(tvix/eval): type check function argument with set pattern 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 Reviewed-by: tazjin --- tvix/eval/src/compiler/mod.rs | 1 + tvix/eval/src/opcode.rs | 1 + .../tvix_tests/eval-fail-function-formals-typecheck.nix | 2 ++ tvix/eval/src/value/mod.rs | 1 + tvix/eval/src/vm/mod.rs | 13 +++++++++++++ 5 files changed, 18 insertions(+) create mode 100644 tvix/eval/src/tests/tvix_tests/eval-fail-function-formals-typecheck.nix 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 => { -- cgit 1.4.1