refactor(tvix/eval): simplify `let ... in ...` before recursion r/4560

While full recursion through thunking is not available, there are
actually incorrect behaviours introduced by declaring before
binding (example in the newly introduced test).

This commit simplifies the implementation to avoid this issue, and
also because I intend to explore a bit more how far we can get in non
left-to-right bindings *without* introducing thunks immediately.

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

3 files changed, 14 insertions, 30 deletions

diff --git a/tvix/eval/src/compiler.rs b/tvix/eval/src/compiler.rs
index 970ad7a1b7..1d63acbb0a 100644
--- a/tvix/eval/src/compiler.rs
+++ b/tvix/eval/src/compiler.rs
@@ -721,8 +721,6 @@ impl Compiler {
     // entries vector.
     fn compile_let_in(&mut self, node: rnix::types::LetIn) -> EvalResult<()> {
         self.begin_scope();
-        let mut entries = vec![];
-        let mut from_inherits = vec![];
 
         for inherit in node.inherits() {
             match inherit.from() {
@@ -737,19 +735,18 @@ impl Compiler {
                     continue;
                 }
 
-                Some(_) => {
+                Some(from) => {
                     for ident in inherit.idents() {
+                        // TODO: Optimised multi-select instruction?
+                        self.compile(from.inner().unwrap())?;
+                        self.emit_literal_ident(&ident);
+                        self.chunk.push_op(OpCode::OpAttrsSelect);
                         self.push_local(ident.as_str());
                     }
-                    from_inherits.push(inherit);
                 }
             }
         }
 
-        // Before compiling the values of a let expression, all keys
-        // need to already be added to the known locals. This is
-        // because in Nix these bindings are always recursive (they
-        // can even refer to themselves).
         for entry in node.entries() {
             let key = entry.key().unwrap();
             let mut path = normalise_ident_path(key.path())?;
@@ -758,31 +755,10 @@ impl Compiler {
                 todo!("nested bindings in let expressions :(")
             }
 
-            entries.push(entry.value().unwrap());
+            self.compile(entry.value().unwrap())?;
             self.push_local(path.pop().unwrap());
         }
 
-        // Now we can add instructions to look up each inherited value
-        // ...
-        for inherit in from_inherits {
-            let from = inherit
-                .from()
-                .expect("only inherits with `from` are pushed here");
-
-            for ident in inherit.idents() {
-                // TODO: Optimised multi-select instruction?
-                self.compile(from.inner().unwrap())?;
-                self.emit_literal_ident(&ident);
-                self.chunk.push_op(OpCode::OpAttrsSelect);
-            }
-        }
-
-        // ... and finally each expression, leaving the values on the
-        // stack in the right order.
-        for value in entries {
-            self.compile(value)?;
-        }
-
         // Deal with the body, then clean up the locals afterwards.
         self.compile(node.body().unwrap())?;
         self.end_scope();
diff --git a/tvix/eval/src/tests/tvix_tests/eval-okay-nested-let.exp b/tvix/eval/src/tests/tvix_tests/eval-okay-nested-let.exp
new file mode 100644
index 0000000000..7f8f011eb7
--- /dev/null
+++ b/tvix/eval/src/tests/tvix_tests/eval-okay-nested-let.exp
@@ -0,0 +1 @@
+7
diff --git a/tvix/eval/src/tests/tvix_tests/eval-okay-nested-let.nix b/tvix/eval/src/tests/tvix_tests/eval-okay-nested-let.nix
new file mode 100644
index 0000000000..caaa202119
--- /dev/null
+++ b/tvix/eval/src/tests/tvix_tests/eval-okay-nested-let.nix
@@ -0,0 +1,7 @@
+let
+  a = let
+    b = 1;
+    c = 2;
+  in b + c;
+  b = 4;
+in a + b