From a5e22c532b074cca80d15046e6aa109d9ca79a80 Mon Sep 17 00:00:00 2001 From: Vincent Ambo Date: Sat, 27 Aug 2022 23:58:46 +0300 Subject: fix(tvix/eval): correctly resolve dynamic upvalues one scope up 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 --- tvix/eval/src/compiler/mod.rs | 13 +++++++------ .../tests/tvix_tests/eval-okay-closure-with-shadowing.exp | 1 + .../tests/tvix_tests/eval-okay-closure-with-shadowing.nix | 14 ++++++++++++++ .../src/tests/tvix_tests/eval-okay-deeply-nested-with.exp | 1 + .../src/tests/tvix_tests/eval-okay-deeply-nested-with.nix | 6 ++++++ 5 files changed, 29 insertions(+), 6 deletions(-) create mode 100644 tvix/eval/src/tests/tvix_tests/eval-okay-closure-with-shadowing.exp create mode 100644 tvix/eval/src/tests/tvix_tests/eval-okay-closure-with-shadowing.nix create mode 100644 tvix/eval/src/tests/tvix_tests/eval-okay-deeply-nested-with.exp create mode 100644 tvix/eval/src/tests/tvix_tests/eval-okay-deeply-nested-with.nix (limited to 'tvix') diff --git a/tvix/eval/src/compiler/mod.rs b/tvix/eval/src/compiler/mod.rs index 6ba470a7af..bf5c55db2a 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 0000000000..d00491fd7e --- /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 0000000000..3054637752 --- /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 0000000000..3bed31f76e --- /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 0000000000..7f1128b670 --- /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 ] -- cgit 1.4.1