fix(tvix/eval): consider `let ... inherit ...` in dynamic scopes r/4598

In conditions where no dynamic identifiers exist in a scope,
inheriting is usually a no-op - *unless* the identifier is not
statically known and the scope has a non-empty `with`-stack.

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

3 files changed, 40 insertions, 3 deletions

diff --git a/tvix/eval/src/compiler.rs b/tvix/eval/src/compiler.rs
index a2ab6273c1..6eca088ec2 100644
--- a/tvix/eval/src/compiler.rs
+++ b/tvix/eval/src/compiler.rs
@@ -667,13 +667,34 @@ impl Compiler {
         for inherit in node.inherits() {
             match inherit.from() {
                 // Within a `let` binding, inheriting from the outer
-                // scope is practically a no-op.
-                None => {
+                // scope is a no-op *if* the identifier can be
+                // statically resolved.
+                None if self.scope().with_stack.is_empty() => {
                     self.emit_warning(inherit.syntax().clone(), WarningKind::UselessInherit);
-
                     continue;
                 }
 
+                None => {
+                    for ident in inherit.idents() {
+                        // If the identifier resolves statically, it
+                        // has precedence over dynamic bindings, and
+                        // the inherit is useless.
+                        if self
+                            .resolve_local(ident.ident_token().unwrap().text())
+                            .is_some()
+                        {
+                            self.emit_warning(ident.syntax().clone(), WarningKind::UselessInherit);
+                            continue;
+                        }
+
+                        self.compile_ident(ident.clone());
+                        self.declare_local(
+                            ident.syntax().clone(),
+                            ident.ident_token().unwrap().text(),
+                        );
+                    }
+                }
+
                 Some(from) => {
                     for ident in inherit.idents() {
                         self.compile(from.expr().unwrap());
diff --git a/tvix/eval/src/tests/tvix_tests/eval-okay-useless-inherit-with.exp b/tvix/eval/src/tests/tvix_tests/eval-okay-useless-inherit-with.exp
new file mode 100644
index 0000000000..d00491fd7e
--- /dev/null
+++ b/tvix/eval/src/tests/tvix_tests/eval-okay-useless-inherit-with.exp
@@ -0,0 +1 @@
+1
diff --git a/tvix/eval/src/tests/tvix_tests/eval-okay-useless-inherit-with.nix b/tvix/eval/src/tests/tvix_tests/eval-okay-useless-inherit-with.nix
new file mode 100644
index 0000000000..d335e53630
--- /dev/null
+++ b/tvix/eval/src/tests/tvix_tests/eval-okay-useless-inherit-with.nix
@@ -0,0 +1,15 @@
+# Normally using an `inherit` without a source attribute set within a
+# `let` is a no-op, *unless* there is a with in-scope that might
+# provide the value.
+
+# Provide a dynamic `x` identifier in the scope.
+with ({ x = 1;});
+
+# inherit this `x` as a static identifier
+let inherit x;
+
+# Provide another dynamic `x` identifier
+in with ({ x = 3; });
+
+# Inherited static identifier should have precedence
+x