From 1015f2f8e7c37c5c4b4ebca799579c5f6c0d5100 Mon Sep 17 00:00:00 2001
From: Vincent Ambo <mail@tazj.in>
Date: Fri, 23 Sep 2022 00:51:19 +0300
Subject: fix(tvix/eval): manually count entries in recursive scopes

The previous version had a bug where we assumed that the number of
entries in an attribute set AST node would be equivalent to the number
of entries in the runtime attribute set, but due to inherit nodes
containing a variable number of entries, this did not work out.

Fixes b/199

Change-Id: I6f7f7729f3512b297cf29a2e046302ca28477854
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6749
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
---
 tvix/eval/src/compiler/attrs.rs                              |  4 ++--
 tvix/eval/src/compiler/mod.rs                                | 12 ++++++++++--
 .../src/tests/tvix_tests/eval-okay-empty-rec-inherit.exp     |  1 +
 .../src/tests/tvix_tests/eval-okay-empty-rec-inherit.nix     |  1 +
 4 files changed, 14 insertions(+), 4 deletions(-)
 create mode 100644 tvix/eval/src/tests/tvix_tests/eval-okay-empty-rec-inherit.exp
 create mode 100644 tvix/eval/src/tests/tvix_tests/eval-okay-empty-rec-inherit.nix

(limited to 'tvix')

diff --git a/tvix/eval/src/compiler/attrs.rs b/tvix/eval/src/compiler/attrs.rs
index b4d183c51df8..704bf211ffa7 100644
--- a/tvix/eval/src/compiler/attrs.rs
+++ b/tvix/eval/src/compiler/attrs.rs
@@ -241,8 +241,8 @@ impl Compiler<'_> {
         self.scope_mut().begin_scope();
 
         if node.rec_token().is_some() {
-            self.compile_recursive_scope(slot, true, &node);
-            self.push_op(OpCode::OpAttrs(Count(node.entries().count())), &node);
+            let count = self.compile_recursive_scope(slot, true, &node);
+            self.push_op(OpCode::OpAttrs(Count(count)), &node);
         } else {
             let mut count = self.compile_inherit_attrs(slot, node.inherits());
 
diff --git a/tvix/eval/src/compiler/mod.rs b/tvix/eval/src/compiler/mod.rs
index 55b8d960a9a6..21f131e4543a 100644
--- a/tvix/eval/src/compiler/mod.rs
+++ b/tvix/eval/src/compiler/mod.rs
@@ -552,10 +552,11 @@ impl Compiler<'_> {
         self.patch_jump(else_idx); // patch jump *over* else body
     }
 
-    fn compile_recursive_scope<N>(&mut self, slot: LocalIdx, rec_attrs: bool, node: &N)
+    fn compile_recursive_scope<N>(&mut self, slot: LocalIdx, rec_attrs: bool, node: &N) -> usize
     where
         N: ToSpan + ast::HasEntry,
     {
+        let mut count = 0;
         self.scope_mut().begin_scope();
 
         // First pass to find all plain inherits (if they are not useless).
@@ -588,6 +589,8 @@ impl Compiler<'_> {
                             }
                         };
 
+                        count += 1;
+
                         // If the identifier resolves statically in a
                         // `let`, it has precedence over dynamic
                         // bindings, and the inherit is useless.
@@ -625,6 +628,7 @@ impl Compiler<'_> {
                             }
                         };
 
+                        count += 1;
                         inherit_froms.push((from.expr().unwrap(), name, self.span_for(&attr)));
                     }
                 }
@@ -632,7 +636,7 @@ impl Compiler<'_> {
         }
 
         // Data structures to track the bindings observed in the
-        // second path, and forward the information needed to compile
+        // second pass, and forward the information needed to compile
         // their value.
         enum BindingKind {
             InheritFrom {
@@ -689,6 +693,8 @@ impl Compiler<'_> {
 
         // Declare all regular bindings
         for entry in node.attrpath_values() {
+            count += 1;
+
             let mut path = match self.normalise_ident_path(entry.attrpath().unwrap().attrs()) {
                 Ok(p) => p,
                 Err(err) => {
@@ -773,6 +779,8 @@ impl Compiler<'_> {
                 self.push_op(OpCode::OpFinalise(stack_idx), node);
             }
         }
+
+        count
     }
 
     /// Compile a standard `let ...; in ...` expression.
diff --git a/tvix/eval/src/tests/tvix_tests/eval-okay-empty-rec-inherit.exp b/tvix/eval/src/tests/tvix_tests/eval-okay-empty-rec-inherit.exp
new file mode 100644
index 000000000000..ffcd4415b08f
--- /dev/null
+++ b/tvix/eval/src/tests/tvix_tests/eval-okay-empty-rec-inherit.exp
@@ -0,0 +1 @@
+{ }
diff --git a/tvix/eval/src/tests/tvix_tests/eval-okay-empty-rec-inherit.nix b/tvix/eval/src/tests/tvix_tests/eval-okay-empty-rec-inherit.nix
new file mode 100644
index 000000000000..a1181431deca
--- /dev/null
+++ b/tvix/eval/src/tests/tvix_tests/eval-okay-empty-rec-inherit.nix
@@ -0,0 +1 @@
+rec { inherit; }
-- 
cgit 1.4.1