lowering: rename a root-level let that shadows an enclosing-scope name read earlier in the function

A root-level let in a nested function that reuses a name the function had
already read from an enclosing scope (a parameter of the outer function, a
module-level binding, a builtin) made that name local to the whole Python
def, so the earlier read raised UnboundLocalError. The capture census now
tells a reference to a binding outside the frame from one to the frame's
own root (a parameter or an earlier root-level let), and a root-level let
renames to _name exactly when some read of the name in the frame is outside.
Rebinding a parameter or an earlier let in sequence stays as Python does it,
and top-level lets are globals and never rename.

Author: simontreanor

DESIGN.md

@@ -210,9 +210,14 @@ sibling arms of one match or arms of two matches in sequence, so the common `cas
 Error why: …` keeps its names however many matches in a function spell it. The one exception is a
 reference inside a nested closure, which always counts, since a closure can outlive its arm: a
 lambda made in one arm that reads `why` forces a later arm capturing `why` to rename. A `let` at
-the root of a function body is never renamed, since rebinding a name in sequence is exactly what
-Python does too; a nested `let x = x + 1` reads the outer `x` in its value and binds the fresh
-name, as `_x = x + 1`. A renamed nested function is defined under its fresh name and recurses under
+the root of a function body has its own rule, because Python makes a name local to the whole `def`
+the moment anything in the `def` assigns it: such a `let` is renamed only when some read of the
+name in that function means a binding *outside* it (an enclosing function's, a module-level one, a
+builtin) and sits before the `let` or in its own value or in a closure made before it, where Python
+would raise `UnboundLocalError` or read the wrong slot. Rebinding a parameter or an earlier `let` of
+the same function in sequence is exactly what Python does too, so that is emitted as itself, and a
+top-level `let` is a global and never renames. A nested `let x = x + 1` reads the outer `x` in its
+value and binds the fresh name, as `_x = x + 1`. A renamed nested function is defined under its fresh name and recurses under
 it, and a renamed `let mut` is assigned under it, including from a closure's `nonlocal`. Mechanics
 in `INTERNALS.md`.
 

INTERNALS.md

@@ -234,7 +234,7 @@ the runtime alongside `Some`/`None_` unless it binds that name itself.
 ### Arm-scoped captures and nested-block `let`s — implements DESIGN §5
 
 A match arm's capture, and a `let` in a nested block, both become function-wide Python locals, so one
-that another binding of the same name is live across is renamed (issues #92, #96 and #97). The
+that another binding of the same name is live across is renamed (issues #92, #96, #97 and #99). The
 rule, in `src/lowering/captures.rs`, is liveness: binder `B` of name `n` (an arm capture or a
 nested-block `let`) is freshened when some reference to `n` outside `B` resolves, under Pyfun
 scoping, to a binding that is live across `B`: the frame root (a parameter, a root-level `let`, a
@@ -247,8 +247,10 @@ three arms under a root-level `why` read at the end all rename, and a lambda mad
 reads `why` forces a later arm capturing `why` to rename.
 
 The census is one walk of the frame's body (`Census`) that resolves every `Var` reference and `<-`
-target to what it reads at that point, recorded as an `Occurrence { binder, in_closure }`: `binder`
-is `None` for the frame root, or the identity of a registered arm/nested-block binder. Identity is
+target to what it reads at that point, recorded as an `Occurrence { target, in_closure }`: the
+`target` is `Outside` (a binding outside the frame: an enclosing function's, a module-level one, a
+builtin), `Root` (a parameter or a root-level `let` of this frame), or `Binder` (a registered
+arm/nested-block binder); `must_rename` counts the first two alike. Identity is
 the address of the AST node (`binder_key`, the `MatchArm` or the `LetBinding`), which is what the
 lowering holds when it decides that binder, so no spans are involved and a desugared or cloned tree
 can never be mistaken for the original. Each registered binder also records its `ancestors`, the
@@ -298,7 +300,14 @@ are not a block, so any block inside a CE is nested; the module frame treats a b
 binding as root, since a collision with a module-level binding is already isolated by
 `lower_module`'s frame wrap). In a block that is not the root (`Frame::block_is_nested`, reported by
 `enter_block`), `lower_block_let` decides each bound name by `Frame::must_rename(n, key(let))`. A
-root-level `let` never renames: rebinding in sequence is what Python does too. Ordering: a value `let` lowers its value first (references there mean the outer binding), then
+root-level `let` asks `Frame::must_rename_root(n)` instead (issue #99): rename iff any occurrence of
+`n` in the frame is `Outside`. Python makes `n` local to the whole `def` as soon as anything in it
+assigns `n`, so a read of an outer `n` before the `let` (or in its value, or in a closure made
+before it) would raise `UnboundLocalError`; a read after the `let` resolves to `Root(let)` and a
+read of a parameter or an earlier root-level `let` resolves to `Root(that)`, neither of which is
+`Outside`, so rebinding in sequence stays as Python does it. Parameters are bound as `Root` when
+the frame is built (`Frame::of_body` takes them), and the module frame never renames a root-level
+`let` (`Frame::module`), since those are globals. Ordering: a value `let` lowers its value first (references there mean the outer binding), then
 installs `n → _n` in `renames` for the rest of the block, where it dies with the block's
 `restore_local_scope`; a parameterised `let` installs it before its body so a recursive call resolves
 to the renamed def, and the def is emitted under the fresh name. The emitted target is passed down

ROADMAP.md

@@ -231,7 +231,10 @@ this entry.
     nested-block `let`s"). Downstream review found the first rule too eager (#97, closed the same
     day): it renamed a name reused across sequential matches, where nothing is live across the
     rename, so the rule was tightened to liveness and `case Error why:` in three matches of one
-    function keeps its plain name.
+    function keeps its plain name. The same review turned up the last member of the family (#99,
+    closed the same day): a root-level `let x` in a function that had already read an enclosing
+    `x` made `x` local to the whole `def` and the earlier read raised `UnboundLocalError`; such a
+    `let` now renames exactly when a read in the function means a binding outside it.
 
 11. ~~**A destructuring folder went quadratic**~~ **CLOSED 2026-08-30** (#85 in #90). The in-place
     fold pass rejected `fun m (p, l) -> Map.add p l m` because the element parameter had no single

src/lowering/captures.rs

@@ -32,19 +32,30 @@
 
 use std::collections::{HashMap, HashSet};
 
-use crate::parser::ast::{BlockStmt, CeItem, Expr, ExprKind, InterpPart, Item, LetBinding};
+use crate::parser::ast::{BlockStmt, CeItem, Expr, ExprKind, InterpPart, Item, LetBinding, Param};
 
 /// The identity of an arm or nested-block binder: the address of its AST node.
 pub(super) fn binder_key<T>(node: &T) -> usize {
     node as *const T as usize
 }
 
-/// One occurrence of a name: the binder it resolves to (`None` for the frame
-/// root or anything outside the frame) and whether it sits inside a nested
-/// closure of the frame.
+/// What an occurrence of a name reads, under Pyfun scoping.
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+enum Target {
+    /// A binding outside the frame: an enclosing function's, a module-level
+    /// binding, a builtin or prelude name.
+    Outside,
+    /// A root binding of the frame: a parameter or a root-level `let`.
+    Root(usize),
+    /// A registered arm or nested-block binder of the frame.
+    Binder(usize),
+}
+
+/// One occurrence of a name: what it resolves to and whether it sits inside a
+/// nested closure of the frame.
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
 struct Occurrence {
-    binder: Option<usize>,
+    target: Target,
     in_closure: bool,
 }
 
@@ -64,14 +75,21 @@ pub(super) struct Frame {
     /// function whose body is a block). Its `let`s are root-level and never
     /// renamed; any other block is nested.
     pub root_is_body: bool,
+    /// Whether this is the module frame, whose root-level `let`s are globals and
+    /// never rename.
+    pub module: bool,
 }
 
 impl Frame {
-    /// The frame for a function whose body is `body` (parameters are root
-    /// bindings of the frame).
-    pub fn of_body(body: &Expr) -> Frame {
+    /// The frame for a function whose body is `body`; `params` are its
+    /// parameters, root bindings of the frame.
+    pub fn of_body(params: &[Param], body: &Expr) -> Frame {
         let root_is_body = matches!(body.kind, ExprKind::Block { .. });
         let mut census = Census::new(root_is_body);
+        for p in params {
+            let names = super::pattern_bindings(&p.pattern);
+            census.bind_root(binder_key(p), &names);
+        }
         census.expr(body);
         census.finish(root_is_body)
     }
@@ -93,7 +111,9 @@ impl Frame {
         // as root here.
         let mut census = Census::new(true);
         census.items(items);
-        census.finish(true)
+        let mut frame = census.finish(true);
+        frame.module = true;
+        frame
     }
 
     /// This frame's census with `body`'s occurrences added on top, `body` being
@@ -108,6 +128,7 @@ impl Frame {
         census.expr(body);
         let mut frame = census.finish(false);
         frame.fresh = self.fresh.clone();
+        frame.module = self.module;
         frame
     }
 
@@ -132,17 +153,37 @@ impl Frame {
             return false;
         };
         let ancestors = self.ancestors.get(&key);
-        occurrences.iter().any(|o| match o.binder {
-            Some(b) if b == key => false,
-            None => true,
-            Some(b) => o.in_closure || ancestors.is_none_or(|a| a.contains(&b)),
+        occurrences.iter().any(|o| match o.target {
+            Target::Binder(b) if b == key => false,
+            Target::Outside | Target::Root(_) => true,
+            Target::Binder(b) => o.in_closure || ancestors.is_none_or(|a| a.contains(&b)),
         })
     }
+
+    /// The root-level rule (issue #99): a root-level `let` of `name` must be
+    /// renamed when any occurrence of `name` in the frame reads a binding
+    /// *outside* it. Python makes a name local to the whole `def` as soon as
+    /// anything in the `def` assigns it, so such a read (before the `let`, in
+    /// its own value, or in a closure made before it) would fail or read the
+    /// wrong slot. A reference after the `let` resolves to the `let` itself and
+    /// never counts; one that reads a parameter or an earlier root-level `let`
+    /// is a same-frame rebinding, which is what Python does too. The module
+    /// frame's root-level `let`s are globals and never rename.
+    pub fn must_rename_root(&self, name: &str) -> bool {
+        if self.module {
+            return false;
+        }
+        self.occurrences
+            .get(name)
+            .is_some_and(|os| os.iter().any(|o| o.target == Target::Outside))
+    }
 }
 
 /// What a name resolves to at a point of the walk.
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
 enum Resolution {
+    /// A root binding of this frame: a parameter or a root-level `let`.
+    Root(usize),
     /// A registered arm/nested-block binder of this frame.
     Binder(usize),
     /// A binder belonging to a nested Python scope: its own slot, never an
@@ -183,24 +224,39 @@ impl Census {
             fresh: HashSet::new(),
             depth: 0,
             root_is_body,
+            module: false,
         }
     }
 
     fn occurrence(&mut self, name: &str) {
-        let binder = match self.env.get(name) {
+        let target = match self.env.get(name) {
             Some(Resolution::Inner) => return,
-            Some(Resolution::Binder(b)) => Some(*b),
-            None => None,
+            Some(Resolution::Binder(b)) => Target::Binder(*b),
+            Some(Resolution::Root(r)) => Target::Root(*r),
+            None => Target::Outside,
         };
         self.out
             .entry(name.to_string())
             .or_default()
             .push(Occurrence {
-                binder,
+                target,
                 in_closure: self.in_closure,
             });
     }
 
+    /// Bind `names` as root bindings of the frame from here on (a parameter, a
+    /// root-level `let`): a reference after this point reads the frame's own slot.
+    fn bind_root(&mut self, key: usize, names: &[String]) {
+        for n in names {
+            let r = if self.in_closure {
+                Resolution::Inner
+            } else {
+                Resolution::Root(key)
+            };
+            self.env.insert(n.clone(), r);
+        }
+    }
+
     /// Bind `names` for the extent of `f`: to a registered frame binder in the
     /// frame proper, to a nested scope's own slot inside a closure.
     fn scoped<F: FnOnce(&mut Census)>(&mut self, key: Option<usize>, names: &[String], f: F) {
@@ -220,8 +276,8 @@ impl Census {
         self.env = saved;
     }
 
-    /// Bind `names` from here on (no restore): a block `let` for the rest of its
-    /// block. `key` is `None` for a root-level `let` (a root binding).
+    /// Bind `names` from here on (no restore): a nested-block `let` for the rest
+    /// of its block, or an arm capture for the arm's extent.
     fn bind(&mut self, key: Option<usize>, names: &[String]) {
         for n in names {
             match (self.in_closure, key) {
@@ -366,13 +422,20 @@ impl Census {
     /// and binds after; a parameterised `let` is a nested def whose own name is
     /// bound before its body, so a recursive reference resolves to it.
     fn block_let(&mut self, b: &LetBinding, nested: bool) {
-        let key = nested.then(|| binder_key(b));
+        let key = binder_key(b);
         let names = b.bound_names();
+        let bind = |c: &mut Census| {
+            if nested {
+                c.bind(Some(key), &names);
+            } else {
+                c.bind_root(key, &names);
+            }
+        };
         if b.params.is_empty() {
             self.expr(&b.value);
-            self.bind(key, &names);
+            bind(self);
         } else {
-            self.bind(key, &names);
+            bind(self);
             self.closure_let(b);
         }
     }
@@ -396,7 +459,7 @@ impl Census {
                 CeItem::LetBang { target, value, .. } | CeItem::Let { target, value, .. } => {
                     self.expr(value);
                     let names = target.bound_names();
-                    self.bind(None, &names);
+                    self.bind_root(binder_key(it), &names);
                 }
                 CeItem::DoBang(e)
                 | CeItem::Return(e)
@@ -440,5 +503,6 @@ impl Census {
         } else {
             self.closure_let(b);
         }
+        self.bind_root(binder_key(b), &b.bound_names());
     }
 }

src/lowering/mod.rs

@@ -1327,21 +1327,26 @@ impl Lowerer {
         nested: bool,
     ) -> Result<(), LowerError> {
         let mut targets = HashMap::new();
-        if nested {
-            for name in b.bound_names() {
-                if targets.contains_key(&name) {
-                    continue;
-                }
-                let Some(frame) = self.frames.last() else {
-                    break;
-                };
-                if !frame.must_rename(&name, captures::binder_key(b)) {
-                    continue;
-                }
-                let fresh = self.fresh_capture_name(&name);
-                self.frames.last_mut().unwrap().fresh.insert(fresh.clone());
-                targets.insert(name, fresh);
+        for name in b.bound_names() {
+            if targets.contains_key(&name) {
+                continue;
             }
+            let Some(frame) = self.frames.last() else {
+                break;
+            };
+            // A nested `let` renames under the liveness rule; a root-level one
+            // only when a read in the frame means a binding outside it (#99).
+            let rename = if nested {
+                frame.must_rename(&name, captures::binder_key(b))
+            } else {
+                frame.must_rename_root(&name)
+            };
+            if !rename {
+                continue;
+            }
+            let fresh = self.fresh_capture_name(&name);
+            self.frames.last_mut().unwrap().fresh.insert(fresh.clone());
+            targets.insert(name, fresh);
         }
         self.enter_block_let(b);
         if !b.params.is_empty() {
@@ -1471,7 +1476,8 @@ impl Lowerer {
         let shadowed = self.shadow_local_fns(params);
         self.fn_local_stack.push(bound);
         // The body is its own Python frame for the capture-rename census.
-        self.frames.push(captures::Frame::of_body(body));
+        self.frames
+            .push(captures::Frame::of_body(prelude_params, body));
         let lowered = self.lower_return(body, inner);
         self.frames.pop();
         self.fn_local_stack.pop();