refactor(tvix/eval): administer antidote for poison r/5721

The codebase contains a lot of complexity and odd roundabout
handling for shadowing globals.  I'm pretty sure none of this is
necessary, and all of it disappears if you simply make the globals
part of the ordinary identifier resolution chain, with their own
scope up above the root scope.  Then the ordinary shadowing routines
do the right thing, and no special cases or new terminology are
required.

This commit does that.

Note by tazjin: This commit was originally abandoned when Adam decided
not to take away reviewer bandwidth for this at the time (eval was
still in a much earlier stage). As we've recently done some
significant refactoring of globals initialisation this came up again,
and it seems we can easily cover the use-cases of the poison tracking
in other ways now, so I've rebased, updated and resurrected the CL.

Co-Authored-By: Vincent Ambo <tazjin@tvl.su>
Signed-off-by: Adam Joseph <adam@westernsemico.com>
Change-Id: Ib3309a47a7b31fa5bf10466bade0d876b76ae462
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7089
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>

1 files changed, 17 insertions, 9 deletions

diff --git a/tvix/eval/src/compiler/bindings.rs b/tvix/eval/src/compiler/bindings.rs
index 682cf134ce1a..59ca222b7219 100644
--- a/tvix/eval/src/compiler/bindings.rs
+++ b/tvix/eval/src/compiler/bindings.rs
@@ -685,6 +685,14 @@ impl Compiler<'_> {
         self.push_op(OpCode::OpAttrsSelect, node);
     }
 
+    /// Is the given identifier defined *by the user* in any current scope?
+    pub(super) fn is_user_defined(&mut self, ident: &str) -> bool {
+        matches!(
+            self.scope_mut().resolve_local(ident),
+            LocalPosition::Known(_) | LocalPosition::Recursive(_)
+        )
+    }
+
     /// Resolve and compile access to an identifier in the scope.
     fn compile_identifier_access<N: ToSpan + Clone>(
         &mut self,
@@ -692,15 +700,6 @@ impl Compiler<'_> {
         ident: &str,
         node: &N,
     ) {
-        // If the identifier is a global, and it is not poisoned, emit the
-        // global directly.
-        if let Some(global) = self.globals.get(ident) {
-            if !self.scope().is_poisoned(ident) {
-                global.clone()(self, self.span_for(node));
-                return;
-            }
-        }
-
         match self.scope_mut().resolve_local(ident) {
             LocalPosition::Unknown => {
                 // Are we possibly dealing with an upvalue?
@@ -709,6 +708,15 @@ impl Compiler<'_> {
                     return;
                 }
 
+                // Globals are the "upmost upvalues": they behave
+                // exactly like a `let ... in` prepended to the
+                // program's text, and the global scope is nothing
+                // more than the parent scope of the root scope.
+                if let Some(global) = self.globals.get(ident) {
+                    self.emit_constant(global.clone(), &self.span_for(node));
+                    return;
+                }
+
                 // If there is a non-empty `with`-stack (or a parent context
                 // with one), emit a runtime dynamic resolution instruction.
                 //