diff --git a/AGENTS.md b/AGENTS.md index 8030a67a..3183aa0d 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -103,7 +103,7 @@ Key codegen patterns: - Standalone lambdas compile as separate functions; free variables captured in a record-valued closure environment; multi-param lambdas (`\a b c -> body`) are curried into nested single-param lambdas at compile time - Runtime functions are pre-declared as imports; `call_rt`/`call_rt_void` helpers emit calls - `knot_relation_len` returns raw `usize`, not a boxed `Value` — use directly as loop bound -- Trait impl methods compile as mangled functions (`TraitName_TypeName_methodName`); a dispatcher function checks `knot_value_get_tag` at runtime and calls the matching impl; missing impls panic with a clear error message (except operator-mapped methods like `eq`/`compare`/`add`/`sub`/`mul`/`div`/`negate` which fall back to runtime functions for types without explicit impls) +- Trait impl methods compile as mangled functions (`TraitName_TypeName_methodName`). Method calls dispatch **statically** wherever the type is known: inference resolves each trait constraint's parameter to a concrete type and records it in `TraitCallTargets` (keyed by the method occurrence's span + trait name), and `static_impl_name` in codegen rewrites the occurrence to call the selected impl directly — both for applied calls and for methods referenced as values (`map area xs`). This is a correctness requirement, not just an optimization: a runtime tag does not identify a *type*, since two ADTs may declare the same constructor name (`data Shape = Circle … `, `data Blob = Circle …`), and a tag-keyed chain would run whichever impl was registered first. Only genuinely polymorphic occurrences (inside an `Area a => a -> Float` body, where no static type exists) fall back to the runtime dispatcher (`define_trait_dispatchers`), which checks `knot_value_get_tag` and matches constructor names. That fallback is unsound when two impl'd ADTs share a constructor name, so `check_ambiguous_dynamic_dispatch` rejects those programs at compile time rather than silently running the wrong impl (fixing them properly would need dictionary passing or monomorphization). Missing impls panic with a clear error message (except operator-mapped methods like `eq`/`compare`/`add`/`sub`/`mul`/`div`/`negate` which fall back to runtime functions for types without explicit impls). Note the operator paths (`compile_trait_binop`/`compile_comparison`/`compile_condition`) still dispatch purely on the runtime tag — they receive Cranelift values, not spans, so they cannot consult `TraitCallTargets` and remain vulnerable to the same shared-constructor ambiguity - Operator trait dispatch: arithmetic operators (`+`, `-`, `*`, `/`, `%`) dispatch through `Num` trait methods (`add`, `sub`, `mul`, `div`, `mod`); unary negation dispatches through `Num.negate`; `==` dispatches through `Eq.eq`; `!=` dispatches through `Eq.eq` then negates; comparison operators (`<`, `>`, `<=`, `>=`) dispatch through `Ord.compare` and check the resulting `Ordering` constructor tag; `&&`/`||`/`++` remain direct runtime calls (no trait). Primitive impls (Int, Float, Text, Bool) are registered as intrinsic codegen impls that delegate to runtime functions (`knot_value_mod` for `%`, etc.), avoiding circular dependencies (the prelude source does NOT contain these impls). `%` SQL-pushes down to SQLite's `%` operator inside comprehensions where the SQL lint pass already permits the surrounding expression. - Default trait methods: if an impl omits a method with a default body, the default is auto-compiled for that type - `deriving (TraitName)` on data types auto-generates impls using the trait's default method bodies diff --git a/crates/knot-compiler/src/codegen.rs b/crates/knot-compiler/src/codegen.rs index 21b397e6..e6d9cffa 100644 --- a/crates/knot-compiler/src/codegen.rs +++ b/crates/knot-compiler/src/codegen.rs @@ -7,6 +7,7 @@ use crate::infer::{MonadInfo, MonadKind}; use crate::types::{ResolvedType, TypeEnv}; +use knot::ast::Span; use cranelift_codegen::ir::condcodes::IntCC; use cranelift_codegen::ir::types; use cranelift_codegen::ir::{AbiParam, InstBuilder, StackSlotData, StackSlotKind, Value}; @@ -190,6 +191,13 @@ pub struct Codegen { // Resolved monad types for desugared do-blocks (from type inference) monad_info: MonadInfo, + /// Static dispatch type per trait-method occurrence, from inference. + trait_call_targets: crate::infer::TraitCallTargets, + /// Trait method name → the trait that declares it. + trait_method_traits: HashMap, + /// Trait method occurrences left to the runtime dispatcher because the site + /// is polymorphic. Checked for constructor-tag ambiguity after codegen. + dynamic_dispatch_sites: Vec<(String, Span)>, // Builtin relation impls that were actually registered (not already provided by user/prelude) registered_builtin_impls: HashSet, @@ -593,6 +601,7 @@ pub fn compile( from_json_targets: &crate::infer::FromJsonTargets, type_info: &crate::infer::TypeInfo, elem_pushdown_ok: &crate::infer::ElemPushdownOk, + trait_call_targets: &crate::infer::TraitCallTargets, compile_time_overrides: &HashMap, ) -> Result, Vec> { crate::stack::grow(|| { @@ -606,6 +615,7 @@ pub fn compile( from_json_targets, type_info, elem_pushdown_ok, + trait_call_targets, compile_time_overrides, ) }) @@ -622,6 +632,7 @@ fn compile_inner( from_json_targets: &crate::infer::FromJsonTargets, type_info: &crate::infer::TypeInfo, elem_pushdown_ok: &crate::infer::ElemPushdownOk, + trait_call_targets: &crate::infer::TraitCallTargets, compile_time_overrides: &HashMap, ) -> Result, Vec> { let mut cg = Codegen::new(); @@ -641,6 +652,7 @@ fn compile_inner( cg.type_aliases = type_env.aliases.clone(); cg.subset_constraints = type_env.subset_constraints.clone(); cg.monad_info = monad_info.clone(); + cg.trait_call_targets = trait_call_targets.clone(); cg.refine_targets = refine_targets.clone(); cg.refined_types = refined_types.clone(); cg.alias_ast = module @@ -816,6 +828,7 @@ fn compile_inner( cg.define_trampoline(tramp); } } + cg.check_ambiguous_dynamic_dispatch(); if !cg.diagnostics.is_empty() { return Err(cg.diagnostics); } @@ -898,6 +911,9 @@ impl Codegen { type_aliases: HashMap::new(), user_fn_trampolines: HashMap::new(), monad_info: HashMap::new(), + trait_call_targets: HashMap::new(), + trait_method_traits: HashMap::new(), + dynamic_dispatch_sites: Vec::new(), registered_builtin_impls: HashSet::new(), nullable_ctors: HashMap::new(), io_functions: HashSet::new(), @@ -1697,6 +1713,10 @@ impl Codegen { type_param_name.as_deref(), &ty.ty, ); + self.trait_method_traits.insert( + method_name.clone(), + trait_name.clone(), + ); self.trait_methods .entry(method_name.clone()) .and_modify(|info| { @@ -4182,7 +4202,14 @@ impl Codegen { // is compiled but never referenced — e.g. `now = 5` would emit // `knot_now_io` here, producing an `IO` value where the type // checker inferred `Int` (a runtime panic when later used). - if let Some((func_id, n_params)) = self.user_fns.get(name).copied() { + // A trait method referenced as a value (`map area shapes`) + // boxes the impl its static type selects, not the runtime tag + // dispatcher — the tag cannot tell two ADTs apart when they + // share a constructor name. + let static_impl = self.resolve_trait_call(name, expr.span); + let fn_name: &str = + static_impl.as_deref().unwrap_or(name.as_str()); + if let Some((func_id, n_params)) = self.user_fns.get(fn_name).copied() { if n_params == 0 { // 0-param function is a constant — call it directly let func_ref = @@ -4192,12 +4219,12 @@ impl Codegen { } else { // Create a trampoline that bridges (db, env, arg) calling // convention to the user function's (db, arg1, ...) convention. - let trampoline_id = self.get_or_create_trampoline(name, n_params); + let trampoline_id = self.get_or_create_trampoline(fn_name, n_params); let func_ref = self.module.declare_func_in_func(trampoline_id, builder.func); let fn_addr = builder.ins().func_addr(self.ptr_type, func_ref); let null = builder.ins().iconst(self.ptr_type, 0); - let (src_ptr, src_len) = self.string_ptr(builder, name); + let (src_ptr, src_len) = self.string_ptr(builder, fn_name); return self.call_rt(builder, "knot_value_function", &[fn_addr, null, src_ptr, src_len]); } } @@ -6586,8 +6613,14 @@ impl Codegen { ast::ExprKind::Var(name) if self.user_fns.contains_key(name) => { - let (func_id, expected_params) = - self.user_fns[name]; + // A trait method call resolves to the impl its static type + // selects; only genuinely polymorphic sites are left to the + // runtime tag dispatcher. + let static_impl = + self.resolve_trait_call(name, func_expr.span); + let fn_name: &str = + static_impl.as_deref().unwrap_or(name.as_str()); + let (func_id, expected_params) = self.user_fns[fn_name]; if compiled_args.len() == expected_params { let func_ref = self .module @@ -12697,6 +12730,133 @@ impl Codegen { } } + // ── Trait dispatch helpers ───────────────────────────────────── + + /// Resolve a trait method occurrence to the mangled name of the impl its + /// static type selects (`area` at a `Blob` site → `Area_Blob_area`). + /// + /// The runtime dispatcher keys on the value's constructor tag, which does + /// not identify a type: two ADTs may share a constructor name, and the tag + /// chain then picks whichever impl was registered first. Inference resolves + /// the trait's parameter to a concrete type at every monomorphic site, so + /// prefer that impl and never consult the tag. + /// + /// Returns `None` — leaving the occurrence on the runtime dispatcher — when + /// the site is still polymorphic (inside an `Area a => a -> Float` body the + /// type is genuinely unknown until run time), when the resolved type has no + /// impl of this trait, or when a user top-level fn shadows the method name + /// (no dispatcher is built in that case, and that fn must keep winning). + fn static_impl_name(&self, method: &str, span: Span) -> Option { + if !self.trait_dispatcher_fns.contains_key(method) { + return None; + } + let trait_name = self.trait_method_traits.get(method)?; + let type_name = + self.trait_call_targets.get(&(span, trait_name.clone()))?; + let info = self.trait_methods.get(method)?; + if !info.impls.iter().any(|e| &e.type_name == type_name) { + return None; + } + let mangled = format!("{}_{}_{}", trait_name, type_name, method); + // Every registered impl is also a `user_fns` entry under its mangled + // name. Requiring the arity to match the dispatcher's keeps the + // substitution transparent to call sites. + let (_, n_params) = self.user_fns.get(&mangled).copied()?; + (n_params == info.param_count).then_some(mangled) + } + + /// `static_impl_name`, additionally noting occurrences that fall back to + /// the runtime dispatcher so `check_ambiguous_dynamic_dispatch` can verify + /// the tag chain is actually able to tell the impls apart. + fn resolve_trait_call( + &mut self, + method: &str, + span: Span, + ) -> Option { + let resolved = self.static_impl_name(method, span); + if resolved.is_none() + && self.trait_dispatcher_fns.contains_key(method) + && self.trait_method_traits.contains_key(method) + { + self.dynamic_dispatch_sites + .push((method.to_string(), span)); + } + resolved + } + + /// Reject programs whose runtime trait dispatch cannot pick an impl. + /// + /// The dispatcher matches a value's constructor tag against each impl's + /// constructor set, so two ADTs declaring the same constructor name are + /// indistinguishable to it. Monomorphic call sites never reach it — they + /// resolve statically — but a polymorphic one (inside an `Area a => a -> + /// Float` body, say) has no static type, and the tag chain would silently + /// run whichever impl was registered first. Report that instead of + /// miscompiling it; the call needs a concrete type to dispatch on. + fn check_ambiguous_dynamic_dispatch(&mut self) { + let sites = std::mem::take(&mut self.dynamic_dispatch_sites); + let mut reported: HashSet = HashSet::new(); + let mut diags = Vec::new(); + for (method, span) in sites { + if !reported.insert(method.clone()) { + continue; + } + let Some(info) = self.trait_methods.get(&method) else { + continue; + }; + // Constructor name → the impl types that declare it. + let mut owners: HashMap<&str, Vec<&str>> = HashMap::new(); + for e in &info.impls { + let Some(ctors) = self.data_constructors.get(&e.type_name) + else { + continue; + }; + for c in ctors { + owners + .entry(c.as_str()) + .or_default() + .push(e.type_name.as_str()); + } + } + let mut clashes: Vec<(&str, Vec<&str>)> = owners + .into_iter() + .filter(|(_, types)| types.len() > 1) + .collect(); + clashes.sort(); + let Some((ctor, types)) = clashes.first() else { + continue; + }; + let trait_name = self + .trait_method_traits + .get(&method) + .cloned() + .unwrap_or_default(); + diags.push( + knot::diagnostic::Diagnostic::error(format!( + "cannot dispatch '{}' at run time: constructor '{}' is \ + declared by {}, which all implement '{}', so the value's \ + tag does not identify which impl to run", + method, + ctor, + types + .iter() + .map(|t| format!("'{}'", t)) + .collect::>() + .join(" and "), + trait_name, + )) + .label( + span, + format!( + "this call is polymorphic, so '{}' has no static type here", + method + ), + ), + ); + } + self.diagnostics.extend(diags); + } + // ── Operator trait dispatch helpers ──────────────────────────── /// Check if a trait method has any non-builtin implementation (a user or diff --git a/crates/knot-compiler/src/infer.rs b/crates/knot-compiler/src/infer.rs index 741cd7f7..53272b0b 100644 --- a/crates/knot-compiler/src/infer.rs +++ b/crates/knot-compiler/src/infer.rs @@ -101,6 +101,23 @@ pub type RefineTargets = HashMap; /// Refined type info exported for codegen: type_name → predicate expression. pub type RefinedTypeInfoMap = HashMap; +/// Resolved static dispatch types for trait method references, keyed by the +/// span of the method's `Var` occurrence and the trait it belongs to. +/// +/// Trait dispatch cannot key on a value's runtime constructor tag: two ADTs +/// may declare the same constructor name (`data Shape = Circle … `, +/// `data Blob = Circle …`), so a tag alone does not identify the type whose +/// impl should run. Inference already resolves the trait's parameter to a +/// concrete type at every monomorphic call site; recording it here lets +/// codegen call that impl directly instead of guessing from the tag. +/// +/// Keying on `(span, trait_name)` — not span alone — keeps a method's own +/// trait separate from any supertrait or bound constraints instantiated at +/// the same occurrence. A span is absent when the site is still polymorphic +/// (e.g. inside a `Area a => a -> Float` function body), where the type is +/// genuinely unknown until run time. +pub type TraitCallTargets = HashMap<(Span, String), String>; + /// Maps declaration names to their inferred type display strings. pub type TypeInfo = HashMap; @@ -588,6 +605,11 @@ struct Infer { /// Deferred trait constraint checks, resolved after inference. deferred_constraints: Vec, + + /// Trait constraint instantiation sites: (occurrence span, trait name, + /// freshened trait-param var). Resolved after inference into + /// `TraitCallTargets` so codegen can dispatch on the static type. + trait_call_vars: Vec<(Span, String, TyVar)>, /// Next sequence number to stamp onto a pushed `DeferredConstraint`. next_constraint_seq: u64, @@ -752,6 +774,7 @@ impl Infer { unify_depth: 0, traverse_calls: Vec::new(), from_json_calls: Vec::new(), + trait_call_vars: Vec::new(), trait_method_traits: HashMap::new(), trait_method_param_vars: HashMap::new(), known_impls: HashSet::new(), @@ -3031,6 +3054,14 @@ impl Infer { span, seq, }); + // The same variable that decides *whether* an impl exists also + // decides *which* impl runs. Remember it so codegen can dispatch + // on the static type rather than the value's constructor tag. + self.trait_call_vars.push(( + span, + c.trait_name.clone(), + target_var, + )); } // Freshen effect-union constraints alongside the type — each // instantiation gets its own copy so a polymorphic `\/`-typed @@ -10339,11 +10370,11 @@ fn value_references_source_inner( /// /// Runs on a grown stack: a desugared `do` block nests one `__bind` per /// statement, and `infer_expr` recurses through every level. -pub fn check(module: &mut ast::Module) -> (Vec, MonadInfo, TypeInfo, LocalTypeInfo, RefineTargets, RefinedTypeInfoMap, FromJsonTargets, ElemPushdownOk) { +pub fn check(module: &mut ast::Module) -> (Vec, MonadInfo, TypeInfo, LocalTypeInfo, RefineTargets, RefinedTypeInfoMap, FromJsonTargets, ElemPushdownOk, TraitCallTargets) { crate::stack::grow(|| check_inner(module)) } -fn check_inner(module: &mut ast::Module) -> (Vec, MonadInfo, TypeInfo, LocalTypeInfo, RefineTargets, RefinedTypeInfoMap, FromJsonTargets, ElemPushdownOk) { +fn check_inner(module: &mut ast::Module) -> (Vec, MonadInfo, TypeInfo, LocalTypeInfo, RefineTargets, RefinedTypeInfoMap, FromJsonTargets, ElemPushdownOk, TraitCallTargets) { let mut infer = Infer::new(); // Phase 1: Collect type aliases, data types, constructors @@ -10507,6 +10538,21 @@ fn check_inner(module: &mut ast::Module) -> (Vec, MonadInfo, TypeInf // Phase 4d: Compress substitution chains for faster resolution infer.compress_substitution(); + // Phase 4e: Resolve trait constraint sites to the concrete type whose impl + // should run there. Sites that stay polymorphic are left out — codegen + // falls back to runtime dispatch for those. + let mut trait_call_targets = TraitCallTargets::new(); + let trait_call_vars = std::mem::take(&mut infer.trait_call_vars); + for (span, trait_name, var) in trait_call_vars { + let resolved = infer.apply(&Ty::Var(var)); + if matches!(resolved.peel_alias(), Ty::Var(_)) { + continue; + } + if let Some(type_name) = infer.type_name_of(&resolved) { + trait_call_targets.insert((span, trait_name), type_name); + } + } + // Phase 5: Resolve monad types from desugared do-blocks let mut monad_info = MonadInfo::new(); let monad_vars = infer.monad_vars.clone(); @@ -10633,7 +10679,7 @@ fn check_inner(module: &mut ast::Module) -> (Vec, MonadInfo, TypeInf let local_type_info = infer.extract_local_type_info(); let elem_pushdown_ok = infer.elem_pushdown_ok.clone(); - (infer.to_diagnostics(), monad_info, type_info, local_type_info, refine_targets, refined_type_info, from_json_targets, elem_pushdown_ok) + (infer.to_diagnostics(), monad_info, type_info, local_type_info, refine_targets, refined_type_info, from_json_targets, elem_pushdown_ok, trait_call_targets) } @@ -11069,14 +11115,14 @@ mod tests { fn check_src(src: &str) -> Vec { let mut module = parse(src); crate::desugar::desugar(&mut module); - let (diags, _monad_info, _type_info, _local_types, _refine_targets, _refined_types, _from_json, _elem_pushdown) = check(&mut module); + let (diags, _monad_info, _type_info, _local_types, _refine_targets, _refined_types, _from_json, _elem_pushdown, _trait_calls) = check(&mut module); diags } fn type_info_for(src: &str) -> TypeInfo { let mut module = parse(src); crate::desugar::desugar(&mut module); - let (_diags, _monad_info, type_info, _local_types, _refine_targets, _refined_types, _from_json, _elem_pushdown) = check(&mut module); + let (_diags, _monad_info, type_info, _local_types, _refine_targets, _refined_types, _from_json, _elem_pushdown, _trait_calls) = check(&mut module); type_info } diff --git a/crates/knot-compiler/src/main.rs b/crates/knot-compiler/src/main.rs index 22ae3b35..7dcd826c 100644 --- a/crates/knot-compiler/src/main.rs +++ b/crates/knot-compiler/src/main.rs @@ -377,7 +377,7 @@ fn cmd_build(source_file: &str, output_override: Option<&std::path::Path>, overr let type_env = types::TypeEnv::from_module(&module); // Type inference - let (infer_diags, monad_info, type_info, _local_types, refine_targets, refined_types, from_json_targets, elem_pushdown_ok) = infer::check(&mut module); + let (infer_diags, monad_info, type_info, _local_types, refine_targets, refined_types, from_json_targets, elem_pushdown_ok, trait_call_targets) = infer::check(&mut module); if !infer_diags.is_empty() { for diag in &infer_diags { eprintln!("{}", diag.render(&source, &filename)); @@ -440,7 +440,7 @@ fn cmd_build(source_file: &str, output_override: Option<&std::path::Path>, overr } // Code generation - let obj_bytes = match codegen::compile(&module, &type_env, source_file, &monad_info, &refine_targets, &refined_types, &from_json_targets, &type_info, &elem_pushdown_ok, overrides) { + let obj_bytes = match codegen::compile(&module, &type_env, source_file, &monad_info, &refine_targets, &refined_types, &from_json_targets, &type_info, &elem_pushdown_ok, &trait_call_targets, overrides) { Ok(bytes) => bytes, Err(diags) => { for diag in &diags { diff --git a/crates/knot-compiler/tests/regress_analysis_fixes.rs b/crates/knot-compiler/tests/regress_analysis_fixes.rs index 7115070d..8f5cb5c2 100644 --- a/crates/knot-compiler/tests/regress_analysis_fixes.rs +++ b/crates/knot-compiler/tests/regress_analysis_fixes.rs @@ -34,7 +34,7 @@ fn parse(src: &str) -> knot::ast::Module { fn check_src(src: &str) -> Vec { let mut module = parse(src); knot_compiler::desugar::desugar(&mut module); - let (diags, _monad, _type_info, _local, _refine, _refined, _json, _elem) = + let (diags, _monad, _type_info, _local, _refine, _refined, _json, _elem, _trait_calls) = knot_compiler::infer::check(&mut module); diags } diff --git a/crates/knot-compiler/tests/regress_deep_do.rs b/crates/knot-compiler/tests/regress_deep_do.rs index 23e986b9..54368c1a 100644 --- a/crates/knot-compiler/tests/regress_deep_do.rs +++ b/crates/knot-compiler/tests/regress_deep_do.rs @@ -115,7 +115,7 @@ fn deep_do_block_reaches_codegen_without_overflow() { knot_compiler::desugar::desugar(&mut module); let type_env = knot_compiler::types::TypeEnv::from_module(&module); - let (diags, monad_info, type_info, _local, refine_targets, refined, from_json, elem) = + let (diags, monad_info, type_info, _local, refine_targets, refined, from_json, elem, trait_calls) = knot_compiler::infer::check(&mut module); assert!(errors(&diags).is_empty(), "{:?}", errors(&diags)); @@ -129,6 +129,7 @@ fn deep_do_block_reaches_codegen_without_overflow() { &from_json, &type_info, &elem, + &trait_calls, &std::collections::HashMap::new(), ) .expect("codegen should succeed on a deep do block"); diff --git a/crates/knot-compiler/tests/regress_infer.rs b/crates/knot-compiler/tests/regress_infer.rs index 096b7c3f..2d07ab66 100644 --- a/crates/knot-compiler/tests/regress_infer.rs +++ b/crates/knot-compiler/tests/regress_infer.rs @@ -22,7 +22,7 @@ fn check_full( ) -> (Vec, knot_compiler::infer::RefineTargets) { let mut module = parse(src); knot_compiler::desugar::desugar(&mut module); - let (diags, _monad, _type_info, _local, refine_targets, _refined, _json, _elem) = + let (diags, _monad, _type_info, _local, refine_targets, _refined, _json, _elem, _trait_calls) = knot_compiler::infer::check(&mut module); (diags, refine_targets) } diff --git a/crates/knot-compiler/tests/regress_infer_fixes.rs b/crates/knot-compiler/tests/regress_infer_fixes.rs index c37388f9..73957a27 100644 --- a/crates/knot-compiler/tests/regress_infer_fixes.rs +++ b/crates/knot-compiler/tests/regress_infer_fixes.rs @@ -26,7 +26,7 @@ fn parse(src: &str) -> knot::ast::Module { fn check_src(src: &str) -> Vec { let mut module = parse(src); knot_compiler::desugar::desugar(&mut module); - let (diags, _monad, _type_info, _local, _targets, _refined, _json, _elem) = + let (diags, _monad, _type_info, _local, _targets, _refined, _json, _elem, _trait_calls) = knot_compiler::infer::check(&mut module); diags } diff --git a/crates/knot-compiler/tests/regress_infer_fixes2.rs b/crates/knot-compiler/tests/regress_infer_fixes2.rs index 51850078..e1b492b4 100644 --- a/crates/knot-compiler/tests/regress_infer_fixes2.rs +++ b/crates/knot-compiler/tests/regress_infer_fixes2.rs @@ -41,7 +41,7 @@ fn check_src(src: &str) -> Vec { let mut module = parse(src); knot_compiler::base::inject_prelude(&mut module); knot_compiler::desugar::desugar(&mut module); - let (diags, _monad, _type_info, _local, _targets, _refined, _json, _elem) = + let (diags, _monad, _type_info, _local, _targets, _refined, _json, _elem, _trait_calls) = knot_compiler::infer::check(&mut module); diags } diff --git a/crates/knot-compiler/tests/regress_infer_fixes3.rs b/crates/knot-compiler/tests/regress_infer_fixes3.rs index 48c54b8e..794bf916 100644 --- a/crates/knot-compiler/tests/regress_infer_fixes3.rs +++ b/crates/knot-compiler/tests/regress_infer_fixes3.rs @@ -25,7 +25,7 @@ fn check_src(src: &str) -> Vec { let mut module = parse(src); knot_compiler::base::inject_prelude(&mut module); knot_compiler::desugar::desugar(&mut module); - let (diags, _monad, _type_info, _local, _targets, _refined, _json, _elem) = + let (diags, _monad, _type_info, _local, _targets, _refined, _json, _elem, _trait_calls) = knot_compiler::infer::check(&mut module); diags } diff --git a/crates/knot-compiler/tests/regress_infer_fixes4.rs b/crates/knot-compiler/tests/regress_infer_fixes4.rs index f981381a..a5a7b2cd 100644 --- a/crates/knot-compiler/tests/regress_infer_fixes4.rs +++ b/crates/knot-compiler/tests/regress_infer_fixes4.rs @@ -28,7 +28,7 @@ fn check_src(src: &str) -> Vec { let mut module = parse(src); knot_compiler::base::inject_prelude(&mut module); knot_compiler::desugar::desugar(&mut module); - let (diags, _monad, _type_info, _local, _targets, _refined, _json, _elem) = + let (diags, _monad, _type_info, _local, _targets, _refined, _json, _elem, _trait_calls) = knot_compiler::infer::check(&mut module); diags } diff --git a/crates/knot-compiler/tests/regress_infer_fixes5.rs b/crates/knot-compiler/tests/regress_infer_fixes5.rs index 2faadcf3..b14c32ea 100644 --- a/crates/knot-compiler/tests/regress_infer_fixes5.rs +++ b/crates/knot-compiler/tests/regress_infer_fixes5.rs @@ -31,7 +31,7 @@ fn check_src(src: &str) -> Vec { let mut module = parse(src); knot_compiler::base::inject_prelude(&mut module); knot_compiler::desugar::desugar(&mut module); - let (diags, _monad, _type_info, _local, _targets, _refined, _json, _elem) = + let (diags, _monad, _type_info, _local, _targets, _refined, _json, _elem, _trait_calls) = knot_compiler::infer::check(&mut module); diags } diff --git a/crates/knot-compiler/tests/regress_infer_fixes6.rs b/crates/knot-compiler/tests/regress_infer_fixes6.rs index d9518210..64286b07 100644 --- a/crates/knot-compiler/tests/regress_infer_fixes6.rs +++ b/crates/knot-compiler/tests/regress_infer_fixes6.rs @@ -30,7 +30,7 @@ fn check_src(src: &str) -> Vec { let mut module = parse(src); knot_compiler::base::inject_prelude(&mut module); knot_compiler::desugar::desugar(&mut module); - let (diags, _monad, _type_info, _local, _targets, _refined, _json, _elem) = + let (diags, _monad, _type_info, _local, _targets, _refined, _json, _elem, _trait_calls) = knot_compiler::infer::check(&mut module); diags } diff --git a/crates/knot-compiler/tests/regress_knot_issues.rs b/crates/knot-compiler/tests/regress_knot_issues.rs index b60cf2a6..4f69aa61 100644 --- a/crates/knot-compiler/tests/regress_knot_issues.rs +++ b/crates/knot-compiler/tests/regress_knot_issues.rs @@ -39,7 +39,7 @@ fn check_src(src: &str) -> Vec { let mut module = parse(src); knot_compiler::base::inject_prelude(&mut module); knot_compiler::desugar::desugar(&mut module); - let (diags, _monad, _type_info, _local, _refine, _refined, _json, _elem) = + let (diags, _monad, _type_info, _local, _refine, _refined, _json, _elem, _trait_calls) = knot_compiler::infer::check(&mut module); diags } diff --git a/crates/knot-compiler/tests/regress_trait_dispatch.rs b/crates/knot-compiler/tests/regress_trait_dispatch.rs new file mode 100644 index 00000000..d29a6a36 --- /dev/null +++ b/crates/knot-compiler/tests/regress_trait_dispatch.rs @@ -0,0 +1,227 @@ +//! Regression tests for trait dispatch when two ADTs share a constructor name. +//! +//! The codegen dispatcher (`define_trait_dispatchers`) selects an impl by +//! matching the value's runtime constructor tag against each impl's set of +//! constructor names. A tag does not identify a type: given +//! +//! data Shape = Circle {r: Float} | Square {s: Float} +//! data Blob = Circle {r: Float} | Blob2 {x: Int} +//! +//! a `Circle` value matches the `Shape` arm and the `Blob` arm alike, so the +//! chain ran whichever impl was registered first — `area (… : Blob)` silently +//! returned the `Area Shape` result. +//! +//! Inference already resolves the trait's parameter to a concrete type at every +//! monomorphic site (`TraitCallTargets`), so those sites now bypass the +//! dispatcher and call the selected impl directly. A polymorphic site (inside +//! an `Area a => a -> Float` body) has no static type to dispatch on; when the +//! impls there are tag-ambiguous the program is rejected rather than +//! miscompiled. + +use std::fs; +use std::path::PathBuf; +use std::process::Command; + +struct Compiled { + dir: PathBuf, + exe: PathBuf, +} + +impl Drop for Compiled { + fn drop(&mut self) { + let _ = fs::remove_dir_all(&self.dir); + } +} + +fn scratch_dir(test_name: &str) -> PathBuf { + let dir = std::env::temp_dir().join(format!( + "knot_regress_traitdisp_{}_{}", + test_name, + std::process::id() + )); + let _ = fs::remove_dir_all(&dir); + fs::create_dir_all(&dir).unwrap(); + dir +} + +/// Compile `source` in a fresh scratch dir; returns Ok(exe) or Err(stderr). +fn try_compile(test_name: &str, source: &str) -> Result { + let dir = scratch_dir(test_name); + let src_path = dir.join("prog.knot"); + fs::write(&src_path, source).unwrap(); + + let out = Command::new(env!("CARGO_BIN_EXE_knot")) + .arg("build") + .arg(&src_path) + .current_dir(&dir) + .output() + .expect("failed to spawn knot compiler"); + + if out.status.success() { + let exe = dir.join("prog"); + Ok(Compiled { dir, exe }) + } else { + let stderr = String::from_utf8_lossy(&out.stderr).into_owned(); + let _ = fs::remove_dir_all(&dir); + Err(stderr) + } +} + +/// Compile and run; returns stdout. Panics if compilation or the run fails. +fn compile_and_run(test_name: &str, source: &str) -> String { + let c = try_compile(test_name, source) + .unwrap_or_else(|e| panic!("knot build failed for {test_name}:\n{e}")); + let out = Command::new(&c.exe) + .current_dir(&c.dir) + .output() + .expect("failed to run compiled program"); + assert!( + out.status.success(), + "program {test_name} exited with failure:\nstdout: {}\nstderr: {}", + String::from_utf8_lossy(&out.stdout), + String::from_utf8_lossy(&out.stderr), + ); + String::from_utf8_lossy(&out.stdout).into_owned() +} + +/// Two ADTs sharing the `Circle` constructor. `Area Shape` is declared first, +/// so a tag-keyed dispatcher answers 1.0 for *both* types. +const SHARED_CTOR_PRELUDE: &str = r#"data Shape = Circle {r: Float} | Square {s: Float} +data Blob = Circle {r: Float} | Blob2 {x: Int} + +trait Area a where + area : a -> Float + scaled : a -> Float -> Float + +impl Area Shape where + area sh = 1.0 + scaled sh k = 1.0 * k + +impl Area Blob where + area b = 2.0 + scaled b k = 2.0 * k +"#; + +#[test] +fn shared_constructor_dispatches_on_static_type() { + // The reported bug: `area (Circle {…} : Blob)` ran the `Area Shape` impl + // because both types spell the constructor `Circle`. + let src = format!( + "{SHARED_CTOR_PRELUDE} +main = do + let s = Circle {{r: 2.0}} : Shape + let b = Circle {{r: 2.0}} : Blob + println (show (area s)) + println (show (area b)) +" + ); + let stdout = compile_and_run("static_type", &src); + let lines: Vec<&str> = stdout.lines().collect(); + assert_eq!(lines[0], "\"1.0\"", "Shape must run the Area Shape impl"); + assert_eq!( + lines[1], "\"2.0\"", + "Blob must run the Area Blob impl, not Shape's (stdout: {stdout})" + ); +} + +#[test] +fn shared_constructor_dispatches_multi_arg_method() { + // Dispatch is on the first param; the extra arg must still be forwarded. + let src = format!( + "{SHARED_CTOR_PRELUDE} +main = do + let s = Circle {{r: 2.0}} : Shape + let b = Circle {{r: 2.0}} : Blob + println (show (scaled s 10.0)) + println (show (scaled b 10.0)) +" + ); + let stdout = compile_and_run("multi_arg", &src); + let lines: Vec<&str> = stdout.lines().collect(); + assert_eq!(lines[0], "\"10.0\""); + assert_eq!( + lines[1], "\"20.0\"", + "Blob must run the Area Blob impl (stdout: {stdout})" + ); +} + +#[test] +fn shared_constructor_dispatches_when_method_is_a_bare_value() { + // `map area xs` boxes the method as a function value. That path used to + // box the tag dispatcher; it must box the statically selected impl. + let src = format!( + "{SHARED_CTOR_PRELUDE} +main = do + let shapes = [Circle {{r: 1.0}} : Shape] + let blobs = [Circle {{r: 1.0}} : Blob] + println (show (map area shapes)) + println (show (map area blobs)) +" + ); + let stdout = compile_and_run("bare_value", &src); + let lines: Vec<&str> = stdout.lines().collect(); + assert_eq!(lines[0], "\"[1.0]\""); + assert_eq!( + lines[1], "\"[2.0]\"", + "Blob must run the Area Blob impl (stdout: {stdout})" + ); +} + +#[test] +fn ambiguous_polymorphic_dispatch_is_rejected() { + // Inside `Area a => a -> Float` the type is unknown until run time, and the + // shared `Circle` tag cannot select between the two impls. Reject the + // program — previously it compiled and silently ran the first impl. + let src = format!( + "{SHARED_CTOR_PRELUDE} +describe : Area a => a -> Float +describe = \\x -> area x + +main = do + println (show (describe (Circle {{r: 2.0}} : Blob))) +" + ); + let err = try_compile("ambiguous_poly", &src) + .err() + .expect("a tag-ambiguous polymorphic dispatch must not compile"); + assert!( + err.contains("cannot dispatch 'area' at run time"), + "expected an ambiguous-dispatch error, got:\n{err}" + ); + assert!( + err.contains("'Shape'") && err.contains("'Blob'"), + "error should name both clashing types, got:\n{err}" + ); +} + +#[test] +fn polymorphic_dispatch_still_works_without_a_tag_clash() { + // The ambiguity guard must not overfire: with distinct constructor names + // the runtime dispatcher is still sound and polymorphic calls must work. + let src = r#"data Shape = Circle {r: Float} | Square {s: Float} +data Blob = Blob1 {r: Float} | Blob2 {x: Int} + +trait Area a where + area : a -> Float + +impl Area Shape where + area sh = 1.0 + +impl Area Blob where + area b = 2.0 + +describe : Area a => a -> Float +describe = \x -> area x + +main = do + println (show (describe (Circle {r: 2.0} : Shape))) + println (show (describe (Blob1 {r: 2.0} : Blob))) +"#; + let stdout = compile_and_run("poly_no_clash", src); + let lines: Vec<&str> = stdout.lines().collect(); + assert_eq!(lines[0], "\"1.0\""); + assert_eq!( + lines[1], "\"2.0\"", + "runtime tag dispatch is sound here (stdout: {stdout})" + ); +} diff --git a/crates/knot-lsp/src/analysis.rs b/crates/knot-lsp/src/analysis.rs index 32bd97b9..26626b68 100644 --- a/crates/knot-lsp/src/analysis.rs +++ b/crates/knot-lsp/src/analysis.rs @@ -606,6 +606,7 @@ pub fn analyze_document( refined_type_info, _from_json, _elem_pushdown, + _trait_calls, ) = knot_compiler::infer::check(&mut analysis_module); all_diags.extend(infer_diags.into_iter().filter(anchored_in_user)); type_info = inferred_types; diff --git a/crates/knot-lsp/src/workspace_diagnostics.rs b/crates/knot-lsp/src/workspace_diagnostics.rs index 75534840..d04a16be 100644 --- a/crates/knot-lsp/src/workspace_diagnostics.rs +++ b/crates/knot-lsp/src/workspace_diagnostics.rs @@ -819,7 +819,7 @@ fn analyze_unopened_file_inner( knot_compiler::base::inject_prelude(&mut analysis_module); knot_compiler::desugar::desugar(&mut analysis_module); - let (infer_diags, _, _, _, _, _, _, _) = knot_compiler::infer::check(&mut analysis_module); + let (infer_diags, _, _, _, _, _, _, _, _) = knot_compiler::infer::check(&mut analysis_module); all_diags.extend(infer_diags.into_iter().filter(anchored_in_importer)); let (effect_diags, _) = knot_compiler::effects::check_with_effects(&analysis_module);