From a7e280ec004df8c9f38afa5cc3629b632e2b55ca Mon Sep 17 00:00:00 2001
From: Vincent Ambo <mail@tazj.in>
Date: Fri, 16 Sep 2022 18:47:28 +0300
Subject: refactor(tvix/eval): introduce a type for tracking bindings

This type is used in the list temporarily populated by the *second*
pass over all identifiers in a recursive scope. This first pass only
serves to make all bindings known to the compiler, without populating
their values yet.

Having a type here is going to be useful once we implement `rec`,
which needs to thread through slightly more information.

Change-Id: Ie33e0f096c5fcb6c864c991255466748b6f0d1eb
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6609
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
---
 tvix/eval/src/compiler/mod.rs | 88 +++++++++++++++++++++++++++++++------------
 1 file changed, 64 insertions(+), 24 deletions(-)

(limited to 'tvix')

diff --git a/tvix/eval/src/compiler/mod.rs b/tvix/eval/src/compiler/mod.rs
index bafbad6795c4..25ddb080686c 100644
--- a/tvix/eval/src/compiler/mod.rs
+++ b/tvix/eval/src/compiler/mod.rs
@@ -571,16 +571,45 @@ impl Compiler<'_, '_> {
                 }
             }
         }
-        // Second pass to ensure that all remaining identifiers (that may
-        // resolve recursively) are known.
-        // Track locals as an index, the expression of its values and optionally
-        // the ident of an attribute to select from it.
-        let mut entries: Vec<(LocalIdx, ast::Expr, Option<ast::Ident>)> = vec![];
+
+        // Data structures to track the bindings observed in the
+        // second path, and forward the information needed to compile
+        // their value.
+        enum BindingKind {
+            InheritFrom {
+                namespace: ast::Expr,
+                ident: ast::Ident,
+            },
+
+            Plain {
+                expr: ast::Expr,
+            },
+        }
+
+        struct TrackedBinding {
+            key_slot: Option<LocalIdx>,
+            value_slot: LocalIdx,
+            kind: BindingKind,
+        }
+
+        // Vector to track these observed bindings.
+        let mut bindings: Vec<TrackedBinding> = vec![];
+
+        // Begin second pass to ensure that all remaining identifiers
+        // (that may resolve recursively) are known.
 
         // Begin with the inherit (from)s since they all become a thunk anyway
         for (from, ident) in inherit_froms {
             let idx = self.declare_local(&ident, ident.ident_token().unwrap().text());
-            entries.push((idx, from, Some(ident)))
+
+            bindings.push(TrackedBinding {
+                key_slot: None,
+                value_slot: idx,
+                kind: BindingKind::InheritFrom {
+                    ident,
+                    namespace: from,
+                },
+            });
         }
 
         // Declare all regular bindings
@@ -604,32 +633,43 @@ impl Compiler<'_, '_> {
 
             let idx = self.declare_local(&entry.attrpath().unwrap(), path.pop().unwrap());
 
-            entries.push((idx, entry.value().unwrap(), None));
+            bindings.push(TrackedBinding {
+                key_slot: None,
+                value_slot: idx,
+                kind: BindingKind::Plain {
+                    expr: entry.value().unwrap(),
+                },
+            });
         }
 
         // Third pass to place the values in the correct stack slots.
         let mut indices: Vec<LocalIdx> = vec![];
-        for (idx, value, path) in entries.into_iter() {
-            indices.push(idx);
+        for binding in bindings.into_iter() {
+            indices.push(binding.value_slot);
+
+            match binding.kind {
+                // This entry is an inherit (from) expr. The value is
+                // placed on the stack by selecting an attribute.
+                BindingKind::InheritFrom { namespace, ident } => {
+                    // Create a thunk wrapping value (which may be one as well) to
+                    // avoid forcing the from expr too early.
+                    self.thunk(binding.value_slot, &namespace, move |c, n, s| {
+                        c.compile(s, n.clone());
+                        c.emit_force(n);
+
+                        c.emit_literal_ident(&ident);
+                        c.push_op(OpCode::OpAttrsSelect, &ident);
+                    })
+                }
 
-            // This entry is an inherit (from) expr, we need to select an attr
-            if let Some(ident) = path {
-                // Create a thunk wrapping value (which may be one as well) to
-                // avoid forcing the from expr too early.
-                self.thunk(idx, &value, move |c, n, s| {
-                    c.compile(s, n.clone());
-                    c.emit_force(n);
-
-                    c.emit_literal_ident(&ident);
-                    c.push_op(OpCode::OpAttrsSelect, &ident);
-                })
-            } else {
-                self.compile(idx, value);
+                // Binding is "just" a plain expression that needs to
+                // be compiled.
+                BindingKind::Plain { expr } => self.compile(binding.value_slot, expr),
             }
 
             // Any code after this point will observe the value in the
             // right stack slot, so mark it as initialised.
-            self.scope_mut().mark_initialised(idx);
+            self.scope_mut().mark_initialised(binding.value_slot);
         }
 
         // Fourth pass to emit finaliser instructions if necessary.
@@ -641,7 +681,7 @@ impl Compiler<'_, '_> {
         }
     }
 
-    /// Compile a standard `let ...; in ...` statement.
+    /// Compile a standard `let ...; in ...` expression.
     ///
     /// Unless in a non-standard scope, the encountered values are
     /// simply pushed on the stack and their indices noted in the
-- 
cgit 1.4.1