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, 4 insertions, 42 deletions

diff --git a/tvix/eval/src/compiler/scope.rs b/tvix/eval/src/compiler/scope.rs
index ef4b4d51e118..892727c107c9 100644
--- a/tvix/eval/src/compiler/scope.rs
+++ b/tvix/eval/src/compiler/scope.rs
@@ -2,8 +2,8 @@
 //! compiler.
 //!
 //! Scoping in Nix is fairly complicated, there are features like
-//! mutually recursive bindings, `with`, upvalue capturing, builtin
-//! poisoning and so on that introduce a fair bit of complexity.
+//! mutually recursive bindings, `with`, upvalue capturing, and so
+//! on that introduce a fair bit of complexity.
 //!
 //! Tvix attempts to do as much of the heavy lifting of this at
 //! compile time, and leave the runtime to mostly deal with known
@@ -75,7 +75,7 @@ impl Local {
     }
 }
 
-/// Represents the current position of a local as resolved in a scope.
+/// Represents the current position of an identifier as resolved in a scope.
 pub enum LocalPosition {
     /// Local is not known in this scope.
     Unknown,
@@ -171,14 +171,6 @@ pub struct Scope {
 
     /// Current size of the `with`-stack at runtime.
     with_stack_size: usize,
-
-    /// Users are allowed to override globally defined symbols like
-    /// `true`, `false` or `null` in scopes. We call this "scope
-    /// poisoning", as it requires runtime resolution of those tokens.
-    ///
-    /// To support this efficiently, the depth at which a poisoning
-    /// occured is tracked here.
-    poisoned_tokens: HashMap<&'static str, usize>,
 }
 
 impl Index<LocalIdx> for Scope {
@@ -190,43 +182,17 @@ impl Index<LocalIdx> for Scope {
 }
 
 impl Scope {
-    /// Mark a globally defined token as poisoned.
-    pub fn poison(&mut self, name: &'static str, depth: usize) {
-        match self.poisoned_tokens.entry(name) {
-            hash_map::Entry::Occupied(_) => {
-                /* do nothing, as the token is already poisoned at a
-                 * lower scope depth */
-            }
-            hash_map::Entry::Vacant(entry) => {
-                entry.insert(depth);
-            }
-        }
-    }
-
     /// Inherit scope details from a parent scope (required for
     /// correctly nesting scopes in lambdas and thunks when special
-    /// scope features like poisoning are present).
+    /// scope features like dynamic resolution are present).
     pub fn inherit(&self) -> Self {
         Self {
-            poisoned_tokens: self.poisoned_tokens.clone(),
             scope_depth: self.scope_depth + 1,
             with_stack_size: self.with_stack_size,
             ..Default::default()
         }
     }
 
-    /// Check whether a given token is poisoned.
-    pub fn is_poisoned(&self, name: &str) -> bool {
-        self.poisoned_tokens.contains_key(name)
-    }
-
-    /// "Unpoison" tokens that were poisoned at the current depth.
-    /// Used when scopes are closed.
-    fn unpoison(&mut self) {
-        self.poisoned_tokens
-            .retain(|_, poisoned_at| *poisoned_at != self.scope_depth);
-    }
-
     /// Increase the `with`-stack size of this scope.
     pub fn push_with(&mut self) {
         self.with_stack_size += 1;
@@ -365,10 +331,6 @@ impl Scope {
     pub fn end_scope(&mut self) -> (usize, Vec<codemap::Span>) {
         debug_assert!(self.scope_depth != 0, "can not end top scope");
 
-        // If this scope poisoned any builtins or special identifiers,
-        // they need to be reset.
-        self.unpoison();
-
         let mut pops = 0;
         let mut unused_spans = vec![];