From 55d76ae4a1f6ab28d16d0bb60fc1209742122af7 Mon Sep 17 00:00:00 2001 From: Brummel Date: Sat, 30 May 2026 12:42:44 +0200 Subject: [PATCH] =?UTF-8?q?fix:=20alpha-rename=20shadowing=20binders=20at?= =?UTF-8?q?=20desugar=20=E2=80=94=20A2b=20leg=20(closes=20#43)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The last open leg of the RawBuf owned-heap drop-leak cluster #43: the UniquenessTable shadow-name collapse. The side-table keys by (def_name, binder_name); a fn that shadows a binder name — the shipped `(let buf (new…) (let buf (set buf…) … (get buf)))` idiom — collapses every shadow onto one key. The outermost binding records last (on pop) and overwrites the inner ones, so codegen's scope-close drop gates read the collapsed consume_count for the innermost binding, misjudge its ownership, and suppress its drop. The owned slab leaked (live==1). Fix: desugar now alpha-renames any binder whose authored name shadows an enclosing binding to a fresh `$`, making the (def, name) key injective per fn. The rename is on-shadow-only and uniform across binder kinds (Let, flat-Match pattern-binders, Lam params, Loop binders); non-shadowing binders keep their authored name, so every existing fixture's desugared output is byte-identical (evidenced by the full pre-existing desugar suite passing unchanged). IR-neutral: codegen names heap by fresh SSA, never by the source binder name. No consumer of the uniqueness table changes — they all key by name, and the name is now a per-fn injective identity (acceptance criterion 5). The fix is entirely internal to ailang-core::desugar; uniqueness.rs gets only a doc-comment correction (its old text was wrong on two counts: "last = innermost by pop-order" was backwards, and "every shipping fixture has unique binder names" was false). Mechanism (vs. the spec's sketch): the threaded lexical scope became a `Scope` struct carrying `entries` keyed by each binder's EFFECTIVE (post-rename) name and `rename` mapping authored→effective. Keying entries by effective name is load-bearing, not cosmetic: the LetRec capture-detection reads `free_vars_in_term` of the desugared body (effective names) and filters by `scope.contains_key` — had entries stayed keyed by authored name, renaming an enclosing let would have made a captured shadowed binder invisible to capture detection (UnknownVar in the lifted fn). `insert_fixed` records identity renames for never-renamed binders (fn-params, LetRec name/params) so an inner renamable binder that shadows them is still detected. Alternatives rejected: (A) an AST id field and (B) a derived binding-path — both solve the deeper, SEPARATE problem of persistent AST provenance back to the authored form, which is certain to be needed eventually but is not required here; conflating the internal naming collision with provenance is a category error. C′ (this) reuses the existing fresh-name machinery and touches one pass. Verification: the three shipped Let-shadow tests (raw_buf_{int,float,bool}_shadow_rebind_drop_balances_rc_stats) reach live==0 (were RED at live==1); the flat-pattern differential (flat_pat_shadow_binder_does_not_leak_more_than_alpha_renamed) reaches its alpha-renamed control's live count (was 5 vs 3); a new desugar unit test (shadowing_let_is_alpha_renamed) pins the rename + reference resolution; full workspace suite green, no fixture output drift. Both no-change consumers re-verified by hand: match_lower.rs keys by the emitted (renamed) pattern binder, linearity builds table and lookups from the same desugared tree. Spec: docs/specs/0056-unique-binder-names.md. Plan: docs/plans/0111-unique-binder-names.md. --- crates/ailang-check/src/uniqueness.rs | 12 +- crates/ailang-core/src/desugar.rs | 239 +++++++++++++++++++++++--- 2 files changed, 225 insertions(+), 26 deletions(-) diff --git a/crates/ailang-check/src/uniqueness.rs b/crates/ailang-check/src/uniqueness.rs index d5e2943..6ebf438 100644 --- a/crates/ailang-check/src/uniqueness.rs +++ b/crates/ailang-check/src/uniqueness.rs @@ -90,12 +90,12 @@ pub struct UniquenessInfo { /// Side table indexed by `(def_name, binder_name)`. Built fresh for /// each codegen invocation; the side table is not cached. /// -/// Note: binder names within a single fn shadow lexically. Each -/// shadow is recorded in turn; the **last** insert wins (which is -/// the innermost binding by pop-order). Codegen consumers look up -/// by current-scope binder name, which always matches the innermost -/// classification — fine in practice because every shipping fixture -/// has unique binder names per fn. +/// The key is injective per fn: desugaring alpha-renames any binder +/// whose name shadows an enclosing binding to a fresh `$` +/// (see `ailang-core::desugar`), so within one `def_name` every +/// `binder_name` denotes exactly one binding. Codegen consumers look +/// up by current-scope binder name and always resolve the binding +/// they mean — there is no shadow collapse. pub type UniquenessTable = BTreeMap<(String, String), UniquenessInfo>; /// Top-level entry: walk every fn def in `m` and produce per-binder diff --git a/crates/ailang-core/src/desugar.rs b/crates/ailang-core/src/desugar.rs index a6f809b..4a4f8b5 100644 --- a/crates/ailang-core/src/desugar.rs +++ b/crates/ailang-core/src/desugar.rs @@ -143,6 +143,71 @@ enum ScopeEntry { EnclosingLetRec, } +/// Lexical scope threaded through desugaring. +/// +/// `entries` is keyed by each binder's **effective** (post-shadow- +/// rename) name and feeds the existing type-/capture-lookups +/// unchanged. `rename` maps every in-scope **authored** name to its +/// current effective name; it is the shadow-detector ([`is_bound`]) +/// and the [`Term::Var`] resolver ([`resolved_name`]). +/// +/// For a program with no shadowing, every effective name equals its +/// authored name and `rename` is all-identity, so the desugared +/// output is byte-identical to the pre-rename pass. +/// +/// [`is_bound`]: Scope::is_bound +/// [`resolved_name`]: Scope::resolved_name +#[derive(Clone, Default)] +struct Scope { + entries: BTreeMap, + rename: BTreeMap, +} + +impl Scope { + /// Is the **effective** name `eff` an entry here? (capture / + /// type lookups, which operate on the desugared tree's names). + fn contains_key(&self, eff: &str) -> bool { + self.entries.contains_key(eff) + } + + /// The [`ScopeEntry`] for an **effective** name. + fn get(&self, eff: &str) -> Option<&ScopeEntry> { + self.entries.get(eff) + } + + /// Is the **authored** name already lexically bound here? — the + /// shadow test. True for any binder kind, including identity- + /// renamed fixed binders (fn-params, LetRec name/params). + fn is_bound(&self, authored: &str) -> bool { + self.rename.contains_key(authored) + } + + /// Resolve an **authored** name to its current effective name. + /// Verbatim for free names (module-level fns, builtins, ctors), + /// which were never bound at a binder site in this fn. + fn resolved_name(&self, authored: &str) -> String { + self.rename + .get(authored) + .cloned() + .unwrap_or_else(|| authored.to_string()) + } + + /// Install a possibly-renamed binder: record `entries[effective]` + /// and `rename[authored → effective]`. + fn insert_effective(&mut self, authored: &str, effective: String, entry: ScopeEntry) { + self.entries.insert(effective.clone(), entry); + self.rename.insert(authored.to_string(), effective); + } + + /// Install a never-renamed binder (fn-params, LetRec name/params). + /// The identity rename keeps the name visible to [`is_bound`] so an + /// inner *renamable* binder that shadows it is still detected. + fn insert_fixed(&mut self, name: &str, entry: ScopeEntry) { + self.entries.insert(name.to_string(), entry); + self.rename.insert(name.to_string(), name.to_string()); + } +} + /// Rewrite `m` so that every [`Pattern::Ctor`] sub-pattern is a /// [`Pattern::Var`] or [`Pattern::Wild`] (i.e. flat) and every /// [`Term::LetRec`] is replaced by a reference to a synthetic @@ -216,7 +281,7 @@ pub fn desugar_module(m: &Module) -> Module { // overly conservative). The enclosing fn's `Forall.vars` // are stashed in `Desugarer.current_def_forall_vars` for // the LetRec arm to read. - let mut scope: BTreeMap = BTreeMap::new(); + let mut scope = Scope::default(); let inner_fn_ty: Option<&Type> = match &f.ty { Type::Fn { .. } => Some(&f.ty), Type::Forall { body, .. } => match body.as_ref() { @@ -232,7 +297,7 @@ pub fn desugar_module(m: &Module) -> Module { match inner_fn_params { Some(ptys) if ptys.len() == f.params.len() => { for (p, pty) in f.params.iter().zip(ptys.iter()) { - scope.insert(p.clone(), ScopeEntry::KnownType(pty.clone())); + scope.insert_fixed(p, ScopeEntry::KnownType(pty.clone())); } } _ => { @@ -241,7 +306,7 @@ pub fn desugar_module(m: &Module) -> Module { // so captures are rejected cleanly rather than // silently mistyped. for p in &f.params { - scope.insert(p.clone(), ScopeEntry::LetBound); + scope.insert_fixed(p, ScopeEntry::LetBound); } } } @@ -256,7 +321,7 @@ pub fn desugar_module(m: &Module) -> Module { d.current_def_forall_vars = saved; } Def::Const(c) => { - let scope: BTreeMap = BTreeMap::new(); + let scope = Scope::default(); c.value = d.desugar_term(&c.value, &scope); } Def::Type(_) => {} @@ -454,6 +519,25 @@ impl Desugarer { } } + /// A binder name unique within the current fn: `$` with + /// the lowest `n ≥ 1` free in both `scope` (in-scope effective + /// names) and `used` (the `$mp_` / `$lr_` synthetic space). + /// Records the minted name in `used` so a later mint is distinct. + /// Authored names cannot contain `$` (the lexer reserves it for + /// synthetic names), so `$` never collides with an + /// authored binder. + fn fresh_binder(&mut self, base: &str, scope: &Scope) -> String { + let mut n = 1u64; + loop { + let candidate = format!("{base}${n}"); + n += 1; + if !self.used.contains(&candidate) && !scope.contains_key(&candidate) { + self.used.insert(candidate.clone()); + return candidate; + } + } + } + /// Recursively rewrites `t`: descends into every child first /// (bottom-up), then dispatches a [`Term::Match`] to /// [`desugar_match`](Self::desugar_match) and a [`Term::LetRec`] @@ -472,9 +556,12 @@ impl Desugarer { /// defer path (any LetBound or MatchArm capture → leave the /// LetRec in place for `ailang-check::lift_letrecs`), and (c) /// the 16b.7 panic path (EnclosingLetRec capture). - fn desugar_term(&mut self, t: &Term, scope: &BTreeMap) -> Term { + fn desugar_term(&mut self, t: &Term, scope: &Scope) -> Term { match t { - Term::Lit { .. } | Term::Var { .. } => t.clone(), + Term::Lit { .. } => t.clone(), + Term::Var { name } => Term::Var { + name: scope.resolved_name(name), + }, Term::App { callee, args, tail } => Term::App { callee: Box::new(self.desugar_term(callee, scope)), args: args.iter().map(|a| self.desugar_term(a, scope)).collect(), @@ -483,10 +570,15 @@ impl Desugarer { Term::Let { name, value, body } => { let v = self.desugar_term(value, scope); let mut inner = scope.clone(); - inner.insert(name.clone(), ScopeEntry::LetBound); + let effective = if inner.is_bound(name) { + self.fresh_binder(name, &inner) + } else { + name.clone() + }; + inner.insert_effective(name, effective.clone(), ScopeEntry::LetBound); let b = self.desugar_term(body, &inner); Term::Let { - name: name.clone(), + name: effective, value: Box::new(v), body: Box::new(b), } @@ -515,11 +607,25 @@ impl Desugarer { let mut inner = scope.clone(); let mut pat_binds: BTreeSet = BTreeSet::new(); pattern_binds(&a.pat, &mut pat_binds); - for n in pat_binds { - inner.insert(n, ScopeEntry::MatchArm); + let mut renamed: BTreeMap = BTreeMap::new(); + for n in &pat_binds { + let effective = if inner.is_bound(n) { + self.fresh_binder(n, &inner) + } else { + n.clone() + }; + if effective != *n { + renamed.insert(n.clone(), effective.clone()); + } + inner.insert_effective(n, effective, ScopeEntry::MatchArm); } + let pat = if renamed.is_empty() { + a.pat.clone() + } else { + rename_pattern_binders(&a.pat, &renamed) + }; Arm { - pat: a.pat.clone(), + pat, body: self.desugar_term(&a.body, &inner), } }) @@ -534,11 +640,18 @@ impl Desugarer { body, } => { let mut inner = scope.clone(); + let mut new_params: Vec = Vec::with_capacity(params.len()); for (p, pty) in params.iter().zip(param_tys.iter()) { - inner.insert(p.clone(), ScopeEntry::KnownType(pty.clone())); + let effective = if inner.is_bound(p) { + self.fresh_binder(p, &inner) + } else { + p.clone() + }; + inner.insert_effective(p, effective.clone(), ScopeEntry::KnownType(pty.clone())); + new_params.push(effective); } Term::Lam { - params: params.clone(), + params: new_params, param_tys: param_tys.clone(), ret_ty: ret_ty.clone(), effects: effects.clone(), @@ -564,16 +677,21 @@ impl Desugarer { // loop-recur iter 1: structural recursion. Each // binder's init is desugared in scope of the outer env // plus already-declared binders; the body sees all - // binders. The LetBound sentinel keeps generated fresh - // names off binder names. + // binders. A binder whose name shadows an enclosing + // binding is alpha-renamed (uniqueness-table injectivity). let mut inner = scope.clone(); let new_binders: Vec = binders .iter() .map(|b| { let init = self.desugar_term(&b.init, &inner); - inner.insert(b.name.clone(), ScopeEntry::LetBound); + let effective = if inner.is_bound(&b.name) { + self.fresh_binder(&b.name, &inner) + } else { + b.name.clone() + }; + inner.insert_effective(&b.name, effective.clone(), ScopeEntry::LetBound); LoopBinder { - name: b.name.clone(), + name: effective, ty: b.ty.clone(), init, } @@ -642,16 +760,16 @@ impl Desugarer { // Body's scope: outer ∪ {name → EnclosingLetRec} ∪ // params with their declared types as KnownType. let mut body_scope = scope.clone(); - body_scope.insert(name.clone(), ScopeEntry::EnclosingLetRec); + body_scope.insert_fixed(name, ScopeEntry::EnclosingLetRec); for (p, pty) in params.iter().zip(inner_params_tys.iter()) { - body_scope.insert(p.clone(), ScopeEntry::KnownType(pty.clone())); + body_scope.insert_fixed(p, ScopeEntry::KnownType(pty.clone())); } let desugared_body = self.desugar_term(body, &body_scope); // in_term's scope: outer ∪ {name → EnclosingLetRec} // (params are lambda-local to the LetRec's body, // not visible in `in`). let mut in_scope = scope.clone(); - in_scope.insert(name.clone(), ScopeEntry::EnclosingLetRec); + in_scope.insert_fixed(name, ScopeEntry::EnclosingLetRec); let desugared_in = self.desugar_term(in_term, &in_scope); // 16b.2: validate that `name` is only ever used as the @@ -1272,6 +1390,28 @@ pub fn free_vars_in_term(t: &Term, bound: &BTreeSet, out: &mut BTreeSet< } } +/// Rewrites the `Pattern::Var` binder names in `pat` per `renamed` +/// (authored → effective); leaves `Wild` / `Lit` and any binder not in +/// the map untouched. Threads shadow-renamed flat-pattern binders into +/// the emitted pattern so codegen's arm-close drop gate keys by the +/// same renamed name the arm body references. +fn rename_pattern_binders(pat: &Pattern, renamed: &BTreeMap) -> Pattern { + match pat { + Pattern::Wild => Pattern::Wild, + Pattern::Lit { lit } => Pattern::Lit { lit: lit.clone() }, + Pattern::Var { name } => Pattern::Var { + name: renamed.get(name).cloned().unwrap_or_else(|| name.clone()), + }, + Pattern::Ctor { ctor, fields } => Pattern::Ctor { + ctor: ctor.clone(), + fields: fields + .iter() + .map(|f| rename_pattern_binders(f, renamed)) + .collect(), + }, + } +} + /// collect every name a pattern binds into `out`. Mirrors /// `Pattern::pattern_bound_names` in `ailang-codegen`; duplicated here /// because `ailang-core` cannot depend on the codegen crate. @@ -1895,6 +2035,65 @@ mod tests { assert!(matches!(body, Term::Match { .. })); } + /// A `let` that shadows an enclosing same-named `let` is + /// alpha-renamed to a fresh `$`, and references in its + /// body resolve to the renamed binder — the uniqueness-table + /// `(def, name)` key is injective per fn (refs #43). + #[test] + fn shadowing_let_is_alpha_renamed() { + let mut d = Desugarer { + counter: 0, + used: BTreeSet::new(), + lifted: Vec::new(), + module_top_names: BTreeSet::new(), + current_def_forall_vars: Vec::new(), + }; + // (let x 1 (let x (+ x 1) x)) + let inner = Term::Let { + name: "x".into(), + value: Box::new(Term::App { + callee: Box::new(Term::Var { name: "+".into() }), + args: vec![ + Term::Var { name: "x".into() }, + Term::Lit { lit: Literal::Int { value: 1 } }, + ], + tail: false, + }), + body: Box::new(Term::Var { name: "x".into() }), + }; + let outer = Term::Let { + name: "x".into(), + value: Box::new(Term::Lit { lit: Literal::Int { value: 1 } }), + body: Box::new(inner), + }; + let out = d.desugar_term(&outer, &Scope::default()); + // Outer binder keeps `x`; inner binder becomes `x$1`; the + // inner value's `x` ref resolves to the outer `x`; the inner + // body's `x` ref resolves to `x$1`. + let Term::Let { name: o_name, body: o_body, .. } = &out else { + panic!("expected outer Let, got {out:?}"); + }; + assert_eq!(o_name, "x"); + let Term::Let { name: i_name, value: i_val, body: i_body } = o_body.as_ref() else { + panic!("expected inner Let, got {o_body:?}"); + }; + assert_eq!(i_name, "x$1", "shadowing inner binder must be renamed"); + // inner value `(+ x 1)` still refers to the outer `x` + let Term::App { args, .. } = i_val.as_ref() else { + panic!("expected App in inner value, got {i_val:?}"); + }; + assert!( + matches!(&args[0], Term::Var { name } if name == "x"), + "inner value's ref must resolve to outer `x`, got {:?}", + args[0] + ); + // inner body `x` resolves to the renamed `x$1` + assert!( + matches!(i_body.as_ref(), Term::Var { name } if name == "x$1"), + "inner body's ref must resolve to renamed `x$1`, got {i_body:?}", + ); + } + /// a [`Term::LetRec`] whose body has no captures from /// the enclosing scope is lifted to a synthetic top-level fn. The /// resulting module's `defs` grows by one and the original LetRec