refactor(tvix/eval): split `compile_inherits` into two r/4978

Splits the large `compile_inherits` function which
previously *compiled* plain inherits and *declared* namespaced
inherits into `compile_plain_inherits` and
`declare_namespaced_inherits`.

This is supposed to make more sense than before, but is still not
consistently used (notably, non-recursive attribute sets still
duplicate most of this logic).

Another salami slice.

Change-Id: Id97fac1cbd5ee97b24d047e7728655e6b7734153
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6773
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI

1 files changed, 29 insertions, 12 deletions

diff --git a/tvix/eval/src/compiler/bindings.rs b/tvix/eval/src/compiler/bindings.rs
index 659dce299a1a..4f384484a973 100644
--- a/tvix/eval/src/compiler/bindings.rs
+++ b/tvix/eval/src/compiler/bindings.rs
@@ -53,19 +53,23 @@ impl BindingsKind {
 
 /// AST-traversing functions related to bindings.
 impl Compiler<'_> {
-    fn compile_inherits<N>(
+    /// Compile all inherits of a node with entries that do *not* have a
+    /// namespace to inherit from, and return the remaining ones that do.
+    fn compile_plain_inherits<N>(
         &mut self,
         slot: LocalIdx,
-        count: &mut usize,
-        bindings: &mut Vec<TrackedBinding>,
         kind: BindingsKind,
+        count: &mut usize,
         node: &N,
-    ) where
+    ) -> Vec<(ast::Expr, String, Span)>
+    where
         N: ToSpan + ast::HasEntry,
     {
-        // First pass over all inherits resolves only those without
-        // namespaces. Since they always resolve to a higher scope, we
-        // can just compile and declare them immediately.
+        // Pass over all inherits, resolving only those without namespaces.
+        // Since they always resolve in a higher scope, we can just compile and
+        // declare them immediately.
+        //
+        // Inherits with namespaces are returned to the caller.
         let mut inherit_froms: Vec<(ast::Expr, String, Span)> = vec![];
 
         for inherit in node.inherits() {
@@ -145,9 +149,21 @@ impl Compiler<'_> {
             }
         }
 
-        // Second pass over the inherits that have a namespace, to declare them
-        // in the tracked bindings. Compiling the values into their slots is the
-        // job of the caller.
+        inherit_froms
+    }
+
+    /// Declare all namespaced inherits, that is inherits which are inheriting
+    /// values from an attribute set.
+    ///
+    /// This only ensures that the locals stack is aware of the inherits, it
+    /// does not yet emit bytecode that places them on the stack. This is up to
+    /// the owner of the `bindings` vector, which this function will populate.
+    fn declare_namespaced_inherits(
+        &mut self,
+        kind: BindingsKind,
+        inherit_froms: Vec<(ast::Expr, String, Span)>,
+        bindings: &mut Vec<TrackedBinding>,
+    ) {
         for (from, name, span) in inherit_froms {
             let key_slot = if kind.is_attrs() {
                 // In an attribute set, the keys themselves are placed
@@ -425,10 +441,11 @@ impl Compiler<'_> {
         let mut count = 0;
         self.scope_mut().begin_scope();
 
-        // Vector to track these observed bindings.
+        // Vector to track all observed bindings.
         let mut bindings: Vec<TrackedBinding> = vec![];
 
-        self.compile_inherits(slot, &mut count, &mut bindings, kind, node);
+        let inherit_froms = self.compile_plain_inherits(slot, kind, &mut count, node);
+        self.declare_namespaced_inherits(kind, inherit_froms, &mut bindings);
 
         // Declare all regular bindings
         for entry in node.attrpath_values() {