fix(tvix/eval): correctly resolve dynamic upvalues one scope up r/4655

This does not yet correctly resolve them if they are more than one
scope up, however.

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

5 files changed, 29 insertions, 6 deletions

diff --git a/tvix/eval/src/compiler/mod.rs b/tvix/eval/src/compiler/mod.rs
index 6ba470a7af9f..bf5c55db2a0c 100644
--- a/tvix/eval/src/compiler/mod.rs
+++ b/tvix/eval/src/compiler/mod.rs
@@ -992,12 +992,6 @@ impl Compiler {
             LocalPosition::Unknown => { /* continue below */ }
         };
 
-        // Determine whether the upvalue is a dynamic variable in the
-        // enclosing context.
-        if self.contexts[ctx_idx - 1].scope.has_with() {
-            return Some(self.add_upvalue(ctx_idx, Upvalue::Dynamic(SmolStr::new(name))));
-        }
-
         // If the upvalue comes from even further up, we need to
         // recurse to make sure that the upvalues are created at each
         // level.
@@ -1005,6 +999,13 @@ impl Compiler {
             return Some(self.add_upvalue(ctx_idx, Upvalue::Upvalue(idx)));
         }
 
+        // If the resolution of a statically known upvalue failed,
+        // attempt to resolve a dynamic one (i.e. search for enclosing
+        // `with` blocks and make that resolution dynamic).
+        if self.contexts[ctx_idx - 1].scope.has_with() {
+            return Some(self.add_upvalue(ctx_idx, Upvalue::Dynamic(SmolStr::new(name))));
+        }
+
         None
     }
 
diff --git a/tvix/eval/src/tests/tvix_tests/eval-okay-closure-with-shadowing.exp b/tvix/eval/src/tests/tvix_tests/eval-okay-closure-with-shadowing.exp
new file mode 100644
index 000000000000..d00491fd7e5b
--- /dev/null
+++ b/tvix/eval/src/tests/tvix_tests/eval-okay-closure-with-shadowing.exp
@@ -0,0 +1 @@
+1
diff --git a/tvix/eval/src/tests/tvix_tests/eval-okay-closure-with-shadowing.nix b/tvix/eval/src/tests/tvix_tests/eval-okay-closure-with-shadowing.nix
new file mode 100644
index 000000000000..305463775217
--- /dev/null
+++ b/tvix/eval/src/tests/tvix_tests/eval-okay-closure-with-shadowing.nix
@@ -0,0 +1,14 @@
+# If a closure closes over a variable that is statically known *and*
+# available dynamically through `with`, the statically known one must
+# have precedence.
+
+let
+  # introduce statically known `a` (this should be the result)
+  a = 1;
+in
+
+# introduce some closure depth to force both kinds of upvalue
+# resolution, and introduce a dynamically known `a` within the
+# closures
+let f = b: with { a = 2; }; c: a + b + c;
+in f 0 0
diff --git a/tvix/eval/src/tests/tvix_tests/eval-okay-deeply-nested-with.exp b/tvix/eval/src/tests/tvix_tests/eval-okay-deeply-nested-with.exp
new file mode 100644
index 000000000000..3bed31f76e3f
--- /dev/null
+++ b/tvix/eval/src/tests/tvix_tests/eval-okay-deeply-nested-with.exp
@@ -0,0 +1 @@
+[ 1 2 3 4 ]
diff --git a/tvix/eval/src/tests/tvix_tests/eval-okay-deeply-nested-with.nix b/tvix/eval/src/tests/tvix_tests/eval-okay-deeply-nested-with.nix
new file mode 100644
index 000000000000..7f1128b6707b
--- /dev/null
+++ b/tvix/eval/src/tests/tvix_tests/eval-okay-deeply-nested-with.nix
@@ -0,0 +1,6 @@
+with { a = 1; b = 1; };
+with { b = 2; c = 2; };
+with { c = 3; d = 3; };
+with { d = 4; };
+
+[ a b c d ]