fix: alpha-rename shadowing binders at desugar — A2b leg (closes #43)
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 `<name>$<n>`, 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.
This commit is contained in:
@@ -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 `<name>$<n>`
|
||||
/// (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
|
||||
|
||||
@@ -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<String, ScopeEntry>,
|
||||
rename: BTreeMap<String, String>,
|
||||
}
|
||||
|
||||
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<String, ScopeEntry> = 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<String, ScopeEntry> = 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: `<base>$<n>` 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 `<base>$<n>` 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<String, ScopeEntry>) -> 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<String> = BTreeSet::new();
|
||||
pattern_binds(&a.pat, &mut pat_binds);
|
||||
for n in pat_binds {
|
||||
inner.insert(n, ScopeEntry::MatchArm);
|
||||
let mut renamed: BTreeMap<String, String> = 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<String> = 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<LoopBinder> = 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<String>, 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<String, String>) -> 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 `<name>$<n>`, 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
|
||||
|
||||
Reference in New Issue
Block a user