From 7f260b82ad08697bfd4b2636ce780478f2508887 Mon Sep 17 00:00:00 2001 From: Brummel Date: Sat, 9 May 2026 21:21:33 +0200 Subject: [PATCH] =?UTF-8?q?iter=2022b.3.6:=20fix=20=E2=80=94=20quality-rev?= =?UTF-8?q?iew=20(shadow-aware=20walker,=20cross-mod=20test,=20Option-type?= =?UTF-8?q?d=20targets,=20const=20helper)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- crates/ail/tests/typeclass_22b3.rs | 159 +++++++++++++ crates/ailang-check/src/mono.rs | 219 ++++++++++++------ examples/test_22b2_xmod_classmod.ail.json | 8 +- .../test_22b3_shadow_class_method.ail.json | 90 +++++++ 4 files changed, 408 insertions(+), 68 deletions(-) create mode 100644 examples/test_22b3_shadow_class_method.ail.json diff --git a/crates/ail/tests/typeclass_22b3.rs b/crates/ail/tests/typeclass_22b3.rs index 6b4a626..bca5745 100644 --- a/crates/ail/tests/typeclass_22b3.rs +++ b/crates/ail/tests/typeclass_22b3.rs @@ -443,6 +443,165 @@ fn fixpoint_handles_chained_class_method_calls() { ); } +/// Property: when a fn's body locally shadows a class-method name +/// (here `show` is rebound by a `let`-binding), synth resolves the +/// shadowed `Var` against the local and pushes NO residual for it. +/// The rewrite walker must mirror that exact lookup-precedence rule +/// — when a `Term::Var { name }` is in the local scope, the walker +/// must NOT advance the per-callsite cursor. Otherwise a subsequent +/// (real) class-method call site is re-aligned to the wrong slot of +/// `ordered_targets`, and the user-visible diff is two corruptions: +/// the shadowed Var gets a wrong mono-symbol, and the next real call +/// site is left un-rewritten (or rewritten to yet another mismatched +/// target). +/// +/// Body shape (`main`): +/// let _t = show 5 in -- residual #0: show@Int +/// let show = "shadow" in -- shadow `show` +/// let _u = show in -- shadowed Var (no residual) +/// foo 7 -- residual #1: foo@Int +/// +/// `synth` pushes exactly two residuals (in order: show@Int, foo@Int). +/// The shadow-aware walker must: +/// - rewrite the outer `show` callee → `show#Int`, +/// - leave the inner shadowed `show` Var literally as `show`, +/// - rewrite the `foo` callee → `foo#Int`. +/// +/// A shadow-blind walker would advance the cursor on the inner Var, +/// rewriting it to `foo#Int` (using residual #1) AND leaving the real +/// `foo 7` callee untouched (cursor walks off the end of the slice). +#[test] +fn rewrite_walker_skips_locally_shadowed_class_method() { + use ailang_core::ast::{Def, Term}; + + let entry = examples_dir().join("test_22b3_shadow_class_method.ail.json"); + let ws = ailang_core::load_workspace(&entry).expect("load"); + let diags = ailang_check::check_workspace(&ws); + assert!(diags.is_empty(), "fixture must typecheck: {:?}", diags); + + let ws = ailang_check::monomorphise_workspace(&ws).expect("mono"); + + let main_module = ws.modules.get("test_22b3_shadow_class_method").unwrap(); + let main_fn = main_module + .defs + .iter() + .find_map(|d| match d { + Def::Fn(f) if f.name == "main" => Some(f), + _ => None, + }) + .expect("main"); + + // Body: Let _t (= App show 5) (Let show "shadow" (Let _u (Var show) (App foo 7))) + let (outer_value, outer_body): (&Term, &Term) = match &main_fn.body { + Term::Let { value, body, .. } => (value.as_ref(), body.as_ref()), + other => panic!("outer body is not Let: {:?}", other), + }; + let outer_callee = match outer_value { + Term::App { callee, .. } => match callee.as_ref() { + Term::Var { name } => name.as_str(), + other => panic!("outer callee is not Var: {:?}", other), + }, + other => panic!("outer value is not App: {:?}", other), + }; + assert_eq!( + outer_callee, "show#Int", + "the outer class-method call site must be rewritten to show#Int" + ); + + // Step inside `let show = "shadow" in `. + let shadow_let_body = match outer_body { + Term::Let { name, body, .. } => { + assert_eq!(name, "show", "expected shadowing let to bind `show`"); + body.as_ref() + } + other => panic!("expected shadowing Let, got {:?}", other), + }; + // Step inside `let _u = in `. + let (inner_value, inner_body): (&Term, &Term) = match shadow_let_body { + Term::Let { value, body, .. } => (value.as_ref(), body.as_ref()), + other => panic!("expected inner Let, got {:?}", other), + }; + let shadowed_var_name = match inner_value { + Term::Var { name } => name.as_str(), + other => panic!("inner value is not Var: {:?}", other), + }; + // The locally-shadowed `show` Var must NOT be rewritten. + assert_eq!( + shadowed_var_name, "show", + "locally-shadowed `show` Var must remain literal `show`, \ + not be re-aligned to a later residual slot" + ); + // The real `foo 7` call site — the second residual — must be + // rewritten correctly to `foo#Int`. A shadow-blind walker would + // have spent residual #1 on the shadowed Var and left this Var + // untouched. + let foo_callee = match inner_body { + Term::App { callee, .. } => match callee.as_ref() { + Term::Var { name } => name.as_str(), + other => panic!("foo callee is not Var: {:?}", other), + }, + other => panic!("expected App for `foo 7`, got {:?}", other), + }; + assert_eq!( + foo_callee, "foo#Int", + "the post-shadow `foo` call site must align to residual #1" + ); +} + +/// Property: a class-method call site in module A whose `defining_module` +/// (per the registry's instance entry) is module B is rewritten to the +/// qualified name `.` — defending the cross-module +/// resolution branch in the walker. Also asserts the synth-def +/// placement contract: the synthesised FnDef lives in B, not A. +#[test] +fn rewrite_uses_qualified_name_for_cross_module_call_site() { + let entry = examples_dir().join("test_22b2_xmod_instance_present.ail.json"); + let ws = ailang_core::load_workspace(&entry).expect("load"); + let diags = ailang_check::check_workspace(&ws); + assert!(diags.is_empty(), "fixture must typecheck: {:?}", diags); + + let ws = ailang_check::monomorphise_workspace(&ws).expect("mono"); + + let main_module = ws.modules.get("test_22b2_xmod_instance_present").unwrap(); + let main_fn = main_module + .defs + .iter() + .find_map(|d| match d { + ailang_core::ast::Def::Fn(f) if f.name == "main" => Some(f), + _ => None, + }) + .expect("main"); + + let callee_name: &str = match &main_fn.body { + ailang_core::ast::Term::App { callee, .. } => match callee.as_ref() { + ailang_core::ast::Term::Var { name } => name.as_str(), + other => panic!("callee is not Var: {:?}", other), + }, + other => panic!("body is not App: {:?}", other), + }; + // class+instance live in test_22b2_xmod_classmod; caller in + // test_22b2_xmod_instance_present → qualified call. + assert_eq!(callee_name, "test_22b2_xmod_classmod.show#Int"); + + // Placement: the synthesised def must live in the defining module, + // not the caller's module. + let classmod = ws.modules.get("test_22b2_xmod_classmod").unwrap(); + let has_synth = classmod.defs.iter().any(|d| { + matches!(d, ailang_core::ast::Def::Fn(f) if f.name == "show#Int") + }); + assert!( + has_synth, + "show#Int must live in the instance's defining module" + ); + let caller_has_synth = main_module.defs.iter().any(|d| { + matches!(d, ailang_core::ast::Def::Fn(f) if f.name == "show#Int") + }); + assert!( + !caller_has_synth, + "show#Int must NOT live in the caller's module" + ); +} + /// After the mono pass, the body of `main` in /// test_22b2_instance_present must reference `show#Int` instead /// of `show`. diff --git a/crates/ailang-check/src/mono.rs b/crates/ailang-check/src/mono.rs index 3e9bfd4..3cfdd21 100644 --- a/crates/ailang-check/src/mono.rs +++ b/crates/ailang-check/src/mono.rs @@ -51,7 +51,7 @@ //! the implementation so the constraint is visible to anyone //! extending the skeleton. -use ailang_core::ast::{Arm, ClassDef, Def, FnDef as AstFnDef, InstanceDef, Term, Type}; +use ailang_core::ast::{Arm, ClassDef, ConstDef, Def, FnDef as AstFnDef, InstanceDef, Pattern, Term, Type}; use ailang_core::workspace::Workspace; use crate::Result; use indexmap::IndexMap; @@ -156,47 +156,35 @@ pub fn monomorphise_workspace(ws: &Workspace) -> Result { for mname in &module_names { let n_defs = ws_owned.modules[mname].defs.len(); for i in 0..n_defs { - let (ordered, defining_module_name): (Vec>, String) = { + let ordered: Vec> = { let m = &ws_owned.modules[mname]; let d = &m.defs[i]; match d { - Def::Fn(f) => (collect_residuals_ordered(f, mname, &env)?, mname.clone()), + Def::Fn(f) => collect_residuals_ordered(f, mname, &env)?, Def::Const(c) => { - let pseudo = AstFnDef { - name: c.name.clone(), - ty: c.ty.clone(), - params: Vec::new(), - body: c.value.clone(), - doc: None, - suppress: Vec::new(), - }; - (collect_residuals_ordered(&pseudo, mname, &env)?, mname.clone()) + let pseudo = const_as_pseudo_fn(c); + collect_residuals_ordered(&pseudo, mname, &env)? } _ => continue, } }; let m = ws_owned.modules.get_mut(mname).unwrap(); let d = &mut m.defs[i]; - let ordered_concrete: Vec = ordered - .into_iter() - .map(|o| { - o.unwrap_or_else(|| MonoTarget { - class: String::new(), - method: String::new(), - type_: Type::unit(), - defining_module: defining_module_name.clone(), - }) - }) - .collect(); let mut cursor = 0usize; + let mut locals: BTreeSet = BTreeSet::new(); match d { Def::Fn(f) => { + // Top-level fn params shadow class-method names too. + for p in &f.params { + locals.insert(p.clone()); + } rewrite_class_method_calls( &mut f.body, &env.class_methods, mname, - &ordered_concrete, + &ordered, &mut cursor, + &mut locals, ); } Def::Const(c) => { @@ -204,8 +192,9 @@ pub fn monomorphise_workspace(ws: &Workspace) -> Result { &mut c.value, &env.class_methods, mname, - &ordered_concrete, + &ordered, &mut cursor, + &mut locals, ); } _ => {} @@ -233,14 +222,7 @@ fn collect_targets_workspace_wide( out.extend(collect_mono_targets(f, mname, env)?); } Def::Const(c) => { - let pseudo = AstFnDef { - name: c.name.clone(), - ty: c.ty.clone(), - params: Vec::new(), - body: c.value.clone(), - doc: None, - suppress: Vec::new(), - }; + let pseudo = const_as_pseudo_fn(c); out.extend(collect_mono_targets(&pseudo, mname, env)?); } _ => {} @@ -250,6 +232,24 @@ fn collect_targets_workspace_wide( Ok(out) } +/// Iter 22b.3.6: wrap a [`ConstDef`] in a synthetic zero-arg +/// [`AstFnDef`] for residual-collection / rewrite purposes. Const +/// bodies have no parameter list, so the residual gathering only +/// depends on body shape — the wrapping is a structural adapter, +/// not a semantic conversion. Two callers (Phase 1 collection and +/// Phase 3 rewrite) need the same shape; sharing the constructor +/// keeps them in lockstep. +fn const_as_pseudo_fn(c: &ConstDef) -> AstFnDef { + AstFnDef { + name: c.name.clone(), + ty: c.ty.clone(), + params: Vec::new(), + body: c.value.clone(), + doc: None, + suppress: Vec::new(), + } +} + /// Iter 22b.3: workspace-wide `class-name -> ClassDef` index. /// Used by the fixpoint to look up the matching class definition /// when synthesising a fn. @@ -574,30 +574,48 @@ pub fn synthesise_mono_fn( /// synth-replay run on the same body. /// /// The walker increments a positional counter at each -/// class-method-named `Term::Var` it encounters. The counter must -/// match the order in which `synth` pushes residuals (pre-order -/// AST walk; child evaluation order matches the `match` arms in -/// `crate::synth`). When this invariant holds, `ordered_targets[idx]` -/// is the target resolved at the i-th class-method call site. +/// class-method-named `Term::Var` it encounters that is NOT +/// shadowed by a local binding. The counter must match the order +/// in which `synth` pushes residuals (pre-order AST walk; child +/// evaluation order matches the `match` arms in [`crate::synth`]). +/// When this invariant holds, `ordered_targets[idx]` is the target +/// resolved at the i-th class-method call site. /// /// `caller_module`: name of the module enclosing this body. If /// the target's `defining_module` differs, the rewritten name is /// qualified `.` (cross-module -/// resolution path); otherwise unqualified. Targets whose `class` -/// is empty are sentinel placeholders for non-concrete residuals -/// — the cursor advances over them and the name is left unchanged. +/// resolution path); otherwise unqualified. +/// +/// `locals` mirrors `synth`'s lookup-precedence rule: a `Term::Var` +/// whose `name` is in `locals` resolves to the local binding and +/// `synth` pushes NO residual for it, so the walker must NOT +/// advance the cursor either. Each binding-form pushes its binders +/// before recursing into the relevant scope and pops them after. +/// +/// `ordered_targets[idx]`: +/// - `Some(t)` — concrete instance found; rewrite the name. +/// - `None` — residual was non-concrete or had no instance; cursor +/// still advances (to keep alignment) but the name is left +/// unchanged. fn rewrite_class_method_calls( body: &mut Term, class_methods: &BTreeMap, caller_module: &str, - ordered_targets: &[MonoTarget], + ordered_targets: &[Option], cursor: &mut usize, + locals: &mut BTreeSet, ) { match body { Term::Var { name } => { if class_methods.contains_key(name) { - if let Some(t) = ordered_targets.get(*cursor) { - if !t.class.is_empty() { + // Mirror synth's lookup-precedence rule: if `name` is + // shadowed by a local, synth resolves to the local + // and pushes no residual — we must not advance. + if locals.contains(name) { + return; + } + if let Some(slot) = ordered_targets.get(*cursor) { + if let Some(t) = slot { let sym = mono_symbol(&t.method, &t.type_); let new_name = if t.defining_module == caller_module { sym @@ -612,58 +630,125 @@ fn rewrite_class_method_calls( } } Term::App { callee, args, .. } => { - rewrite_class_method_calls(callee, class_methods, caller_module, ordered_targets, cursor); + rewrite_class_method_calls(callee, class_methods, caller_module, ordered_targets, cursor, locals); for a in args { - rewrite_class_method_calls(a, class_methods, caller_module, ordered_targets, cursor); + rewrite_class_method_calls(a, class_methods, caller_module, ordered_targets, cursor, locals); } } - Term::Let { value, body, .. } => { - rewrite_class_method_calls(value, class_methods, caller_module, ordered_targets, cursor); - rewrite_class_method_calls(body, class_methods, caller_module, ordered_targets, cursor); + Term::Let { name, value, body } => { + // synth: `value` is in the outer scope; `body` sees `name`. + rewrite_class_method_calls(value, class_methods, caller_module, ordered_targets, cursor, locals); + let inserted = locals.insert(name.clone()); + rewrite_class_method_calls(body, class_methods, caller_module, ordered_targets, cursor, locals); + if inserted { + locals.remove(name); + } } - Term::LetRec { body, in_term, .. } => { - rewrite_class_method_calls(body, class_methods, caller_module, ordered_targets, cursor); - rewrite_class_method_calls(in_term, class_methods, caller_module, ordered_targets, cursor); + Term::LetRec { name, params, body, in_term, .. } => { + // synth: `name` is in scope in BOTH `body` (with `params` + // also in scope) AND `in_term` (without `params`). + let name_inserted = locals.insert(name.clone()); + // Body scope: name + params. + let mut params_inserted: Vec = Vec::new(); + for p in params { + if locals.insert(p.clone()) { + params_inserted.push(p.clone()); + } + } + rewrite_class_method_calls(body, class_methods, caller_module, ordered_targets, cursor, locals); + for p in ¶ms_inserted { + locals.remove(p); + } + // In-clause scope: name only. + rewrite_class_method_calls(in_term, class_methods, caller_module, ordered_targets, cursor, locals); + if name_inserted { + locals.remove(name); + } } Term::If { cond, then, else_ } => { - rewrite_class_method_calls(cond, class_methods, caller_module, ordered_targets, cursor); - rewrite_class_method_calls(then, class_methods, caller_module, ordered_targets, cursor); - rewrite_class_method_calls(else_, class_methods, caller_module, ordered_targets, cursor); + rewrite_class_method_calls(cond, class_methods, caller_module, ordered_targets, cursor, locals); + rewrite_class_method_calls(then, class_methods, caller_module, ordered_targets, cursor, locals); + rewrite_class_method_calls(else_, class_methods, caller_module, ordered_targets, cursor, locals); } Term::Do { args, .. } => { for a in args { - rewrite_class_method_calls(a, class_methods, caller_module, ordered_targets, cursor); + rewrite_class_method_calls(a, class_methods, caller_module, ordered_targets, cursor, locals); } } Term::Ctor { args, .. } => { for a in args { - rewrite_class_method_calls(a, class_methods, caller_module, ordered_targets, cursor); + rewrite_class_method_calls(a, class_methods, caller_module, ordered_targets, cursor, locals); } } Term::Match { scrutinee, arms } => { - rewrite_class_method_calls(scrutinee, class_methods, caller_module, ordered_targets, cursor); - for Arm { body, .. } in arms { - rewrite_class_method_calls(body, class_methods, caller_module, ordered_targets, cursor); + // synth: scrutinee is in outer scope; each arm's body sees + // its pattern's binders. + rewrite_class_method_calls(scrutinee, class_methods, caller_module, ordered_targets, cursor, locals); + for Arm { pat, body } in arms { + let binders = pattern_binders(pat); + let mut inserted: Vec = Vec::new(); + for b in &binders { + if locals.insert(b.clone()) { + inserted.push(b.clone()); + } + } + rewrite_class_method_calls(body, class_methods, caller_module, ordered_targets, cursor, locals); + for b in &inserted { + locals.remove(b); + } } } - Term::Lam { body, .. } => { - rewrite_class_method_calls(body, class_methods, caller_module, ordered_targets, cursor); + Term::Lam { params, body, .. } => { + // synth: params are in scope in `body`. + let mut inserted: Vec = Vec::new(); + for p in params { + if locals.insert(p.clone()) { + inserted.push(p.clone()); + } + } + rewrite_class_method_calls(body, class_methods, caller_module, ordered_targets, cursor, locals); + for p in &inserted { + locals.remove(p); + } } Term::Seq { lhs, rhs } => { - rewrite_class_method_calls(lhs, class_methods, caller_module, ordered_targets, cursor); - rewrite_class_method_calls(rhs, class_methods, caller_module, ordered_targets, cursor); + rewrite_class_method_calls(lhs, class_methods, caller_module, ordered_targets, cursor, locals); + rewrite_class_method_calls(rhs, class_methods, caller_module, ordered_targets, cursor, locals); } Term::Clone { value } => { - rewrite_class_method_calls(value, class_methods, caller_module, ordered_targets, cursor); + rewrite_class_method_calls(value, class_methods, caller_module, ordered_targets, cursor, locals); } Term::ReuseAs { source, body } => { - rewrite_class_method_calls(source, class_methods, caller_module, ordered_targets, cursor); - rewrite_class_method_calls(body, class_methods, caller_module, ordered_targets, cursor); + rewrite_class_method_calls(source, class_methods, caller_module, ordered_targets, cursor, locals); + rewrite_class_method_calls(body, class_methods, caller_module, ordered_targets, cursor, locals); } Term::Lit { .. } => {} } } +/// Iter 22b.3.6: collect the variable binders introduced by a +/// match pattern. Used by the walker's `Term::Match` arm to extend +/// `locals` for each arm body. Mirrors `Pattern`'s shape: +/// - [`Pattern::Wild`] / [`Pattern::Lit`] bind nothing, +/// - [`Pattern::Var`] binds its `name`, +/// - [`Pattern::Ctor`] recurses through its `fields`. +fn pattern_binders(pat: &Pattern) -> Vec { + let mut out: Vec = Vec::new(); + fn rec(pat: &Pattern, out: &mut Vec) { + match pat { + Pattern::Wild | Pattern::Lit { .. } => {} + Pattern::Var { name } => out.push(name.clone()), + Pattern::Ctor { fields, .. } => { + for f in fields { + rec(f, out); + } + } + } + } + rec(pat, &mut out); + out +} + /// Iter 22b.3: traversal-ordered residual collection — used by /// the rewrite walker to align cursor positions. Unlike /// [`collect_mono_targets`], this includes non-concrete residuals diff --git a/examples/test_22b2_xmod_classmod.ail.json b/examples/test_22b2_xmod_classmod.ail.json index 7a3d78c..baa95ab 100644 --- a/examples/test_22b2_xmod_classmod.ail.json +++ b/examples/test_22b2_xmod_classmod.ail.json @@ -26,7 +26,13 @@ "methods": [ { "name": "show", - "body": { "t": "lit", "lit": { "kind": "str", "value": "n" } } + "body": { + "t": "lam", + "params": ["x"], + "paramTypes": [{ "k": "var", "name": "a" }], + "retType": { "k": "con", "name": "Str" }, + "body": { "t": "lit", "lit": { "kind": "str", "value": "n" } } + } } ] } diff --git a/examples/test_22b3_shadow_class_method.ail.json b/examples/test_22b3_shadow_class_method.ail.json new file mode 100644 index 0000000..c6ffbeb --- /dev/null +++ b/examples/test_22b3_shadow_class_method.ail.json @@ -0,0 +1,90 @@ +{ + "schema": "ailang/v0", + "name": "test_22b3_shadow_class_method", + "imports": [], + "defs": [ + { + "kind": "class", "name": "Show", "param": "a", + "methods": [{ + "name": "show", + "type": { + "k": "fn", "params": [{ "k": "var", "name": "a" }], + "ret": { "k": "con", "name": "Str" }, "effects": [] + } + }] + }, + { + "kind": "class", "name": "Foo", "param": "a", + "methods": [{ + "name": "foo", + "type": { + "k": "fn", "params": [{ "k": "var", "name": "a" }], + "ret": { "k": "con", "name": "Str" }, "effects": [] + } + }] + }, + { + "kind": "instance", + "class": "Show", + "type": { "k": "con", "name": "Int" }, + "methods": [{ + "name": "show", + "body": { + "t": "lam", + "params": ["x"], + "paramTypes": [{ "k": "var", "name": "a" }], + "retType": { "k": "con", "name": "Str" }, + "body": { "t": "lit", "lit": { "kind": "str", "value": "s" } } + } + }] + }, + { + "kind": "instance", + "class": "Foo", + "type": { "k": "con", "name": "Int" }, + "methods": [{ + "name": "foo", + "body": { + "t": "lam", + "params": ["x"], + "paramTypes": [{ "k": "var", "name": "a" }], + "retType": { "k": "con", "name": "Str" }, + "body": { "t": "lit", "lit": { "kind": "str", "value": "f" } } + } + }] + }, + { + "kind": "fn", + "name": "main", + "type": { + "k": "fn", "params": [], "ret": { "k": "con", "name": "Str" }, + "effects": [] + }, + "params": [], + "body": { + "t": "let", + "name": "_t", + "value": { + "t": "app", + "fn": { "t": "var", "name": "show" }, + "args": [{ "t": "lit", "lit": { "kind": "int", "value": 5 } }] + }, + "body": { + "t": "let", + "name": "show", + "value": { "t": "lit", "lit": { "kind": "str", "value": "shadow" } }, + "body": { + "t": "let", + "name": "_u", + "value": { "t": "var", "name": "show" }, + "body": { + "t": "app", + "fn": { "t": "var", "name": "foo" }, + "args": [{ "t": "lit", "lit": { "kind": "int", "value": 7 } }] + } + } + } + } + } + ] +}