diff --git a/compiler/noirc_frontend/src/elaborator/function.rs b/compiler/noirc_frontend/src/elaborator/function.rs index 24859991325..28e3eedd470 100644 --- a/compiler/noirc_frontend/src/elaborator/function.rs +++ b/compiler/noirc_frontend/src/elaborator/function.rs @@ -125,6 +125,7 @@ impl Elaborator<'_> { ) { // Set up trait impl state self.current_trait_impl = trait_impl.impl_id; + self.current_trait = trait_impl.trait_id; self.self_type = trait_impl.methods.self_type.clone(); self.generics = generics; @@ -137,6 +138,7 @@ impl Elaborator<'_> { // Cleanup self.self_type = None; self.current_trait_impl = None; + self.current_trait = None; self.generics.clear(); } @@ -176,6 +178,13 @@ impl Elaborator<'_> { let mut trait_constraints = self.resolve_trait_constraints_and_add_to_scope(&func.def.where_clause); + + // Add constraints for parent traits that have associated types. + let (parent_generics, parent_constraints) = + self.add_parent_associated_type_constraints(&trait_constraints); + generics.extend(parent_generics); + trait_constraints.extend(parent_constraints); + let mut extra_trait_constraints = vecmap(extra_trait_constraints, |(constraint, _)| constraint.clone()); extra_trait_constraints.extend(associated_generics_trait_constraints); @@ -486,6 +495,7 @@ impl Elaborator<'_> { self.local_module = Some(func_meta.source_module); self.self_type = func_meta.self_type.clone(); self.current_trait_impl = func_meta.trait_impl; + self.current_trait = func_meta.trait_id; self.scopes.start_function(); let old_item = self.current_item.replace(DependencyId::Function(id)); diff --git a/compiler/noirc_frontend/src/elaborator/mod.rs b/compiler/noirc_frontend/src/elaborator/mod.rs index 9227615b868..075802011a1 100644 --- a/compiler/noirc_frontend/src/elaborator/mod.rs +++ b/compiler/noirc_frontend/src/elaborator/mod.rs @@ -226,7 +226,9 @@ pub struct Elaborator<'context> { /// to the corresponding trait impl ID. current_trait_impl: Option, - /// The trait we're currently resolving, if we are resolving one. + /// The trait we're currently resolving or implementing, if any. + /// Set during both trait definitions (`trait Foo { ... }`) and + /// trait impl elaboration (`impl Foo for Bar { ... }`). current_trait: Option, /// In-resolution names @@ -661,6 +663,7 @@ impl<'context> Elaborator<'context> { self.generics = trait_impl.resolved_generics.clone(); self.current_trait_impl = trait_impl.impl_id; + self.current_trait = trait_impl.trait_id; self.add_trait_impl_assumed_trait_implementations(trait_impl.impl_id); self.check_trait_impl_where_clause_matches_trait_where_clause(&trait_impl); @@ -681,6 +684,7 @@ impl<'context> Elaborator<'context> { self.self_type = None; self.current_trait_impl = None; + self.current_trait = None; self.generics.clear(); } diff --git a/compiler/noirc_frontend/src/elaborator/trait_impls.rs b/compiler/noirc_frontend/src/elaborator/trait_impls.rs index e1d17230f52..293883c202e 100644 --- a/compiler/noirc_frontend/src/elaborator/trait_impls.rs +++ b/compiler/noirc_frontend/src/elaborator/trait_impls.rs @@ -69,6 +69,8 @@ impl Elaborator<'_> { let previous_local_module = self.local_module.replace(trait_impl.module_id); let previous_current_trait_impl = std::mem::replace(&mut self.current_trait_impl, trait_impl.impl_id); + let previous_current_trait = + std::mem::replace(&mut self.current_trait, trait_impl.trait_id); let self_type = trait_impl.methods.self_type.clone(); let self_type = @@ -270,6 +272,7 @@ impl Elaborator<'_> { self.local_module = previous_local_module; self.current_trait_impl = previous_current_trait_impl; + self.current_trait = previous_current_trait; self.self_type = previous_self_type; } diff --git a/compiler/noirc_frontend/src/elaborator/traits.rs b/compiler/noirc_frontend/src/elaborator/traits.rs index 7fc078b983c..e41cebf18a3 100644 --- a/compiler/noirc_frontend/src/elaborator/traits.rs +++ b/compiler/noirc_frontend/src/elaborator/traits.rs @@ -188,7 +188,7 @@ use crate::{ }, hir_def::{ function::FuncMeta, - traits::{ResolvedTraitBound, TraitConstraint, TraitFunction}, + traits::{NamedType, ResolvedTraitBound, TraitConstraint, TraitFunction}, }, node_interner::{ DependencyId, FuncId, ImplSearchErrorKind, NodeInterner, ReferenceId, TraitId, @@ -589,6 +589,130 @@ impl Elaborator<'_> { Some(constraint) } + /// For each resolved trait constraint, add constraints for parent traits that have + /// associated types. This creates fresh type variables for the parent associated types + /// so that `M::Key` syntax can be resolved via `self.trait_bounds`. + /// + /// The parent trait bounds are obtained from `Trait.trait_bounds` (already resolved + /// during `collect_traits` with associated type variables) and instantiated via + /// `instantiate_parent_trait_bound` to substitute the child trait's bindings. The + /// named (associated) types are then replaced with fresh per-function type variables + /// so they can be wrapped in `Type::Forall` and freshened at each call site. + /// + /// Returns (new_generics, new_constraints) to be added to the function's generics + /// and trait constraints respectively. + pub(super) fn add_parent_associated_type_constraints( + &mut self, + constraints: &[TraitConstraint], + ) -> (Vec, Vec) { + let mut new_generics = Vec::new(); + let mut new_constraints = Vec::new(); + let mut visited = rustc_hash::FxHashSet::default(); + + for constraint in constraints { + self.collect_parent_associated_types( + &constraint.typ, + &constraint.trait_bound, + &mut new_generics, + &mut new_constraints, + &mut visited, + ); + visited.clear(); + } + + (new_generics, new_constraints) + } + + /// Recursively walk parent trait hierarchies and create fresh type variables + /// for any associated types found on parent traits. The new constraints are + /// pushed to `self.trait_bounds` and returned via the output parameters. + fn collect_parent_associated_types( + &mut self, + object_type: &Type, + trait_bound: &ResolvedTraitBound, + new_generics: &mut Vec, + new_constraints: &mut Vec, + visited: &mut rustc_hash::FxHashSet, + ) { + let trait_id = trait_bound.trait_id; + if !visited.insert(trait_id) { + return; + } + + let parent_bounds: Vec<_> = self + .interner + .try_get_trait(trait_id) + .map(|t| t.trait_bounds.clone()) + .unwrap_or_default(); + + for parent_bound in &parent_bounds { + // Substitute the child trait's bindings into the parent bound. + let instantiated = self.instantiate_parent_trait_bound(trait_bound, parent_bound); + + // Skip if there are no associated types on this parent trait, + // or if we already have a constraint for this type + parent trait. + let has_named = !instantiated.trait_generics.named.is_empty(); + let already_has = self + .trait_bounds + .iter() + .any(|c| c.trait_bound.trait_id == instantiated.trait_id && c.typ == *object_type); + + if has_named && !already_has { + // Replace the named (associated) type variables with fresh per-function + // ones so they can be included in Type::Forall and freshened at call sites. + let parent_trait = self.interner.get_trait(instantiated.trait_id); + let parent_trait_name = parent_trait.name.to_string(); + let object_name = object_type.to_string(); + + let named = vecmap(&instantiated.trait_generics.named, |named_type| { + let fresh_id = self.interner.next_type_variable_id(); + let kind = named_type.typ.kind(); + let type_var = TypeVariable::unbound(fresh_id, kind); + + let assoc_type_id = parent_trait + .associated_types + .iter() + .find(|a| a.name.as_ref() == named_type.name.as_str()) + .map_or(fresh_id, |a| a.type_var.id()); + + let fresh_type = type_var.clone().into_implicit_named_generic( + &Rc::new(named_type.name.to_string()), + Some((object_name.as_str(), parent_trait_name.as_str())), + assoc_type_id, + ); + + new_generics.push(type_var); + NamedType { + name: Ident::new(named_type.name.to_string(), instantiated.location), + typ: fresh_type, + } + }); + + let trait_generics = + TraitGenerics { ordered: instantiated.trait_generics.ordered.clone(), named }; + let parent_constraint = TraitConstraint { + typ: object_type.clone(), + trait_bound: ResolvedTraitBound { + trait_id: instantiated.trait_id, + trait_generics, + location: instantiated.location, + }, + }; + self.trait_bounds.push(parent_constraint.clone()); + new_constraints.push(parent_constraint); + } + + // Recurse for grandparent traits + self.collect_parent_associated_types( + object_type, + &instantiated, + new_generics, + new_constraints, + visited, + ); + } + } + /// Adds an assumed trait implementation for the given object type and trait bound. /// /// This also recursively adds assumed implementations for any parent traits. diff --git a/compiler/noirc_frontend/src/elaborator/types.rs b/compiler/noirc_frontend/src/elaborator/types.rs index d1ac228d095..2e65f36cd67 100644 --- a/compiler/noirc_frontend/src/elaborator/types.rs +++ b/compiler/noirc_frontend/src/elaborator/types.rs @@ -35,6 +35,7 @@ use crate::{ modules::{get_ancestor_module_reexport, module_def_id_is_visible}, node_interner::{ DependencyId, ExprId, FuncId, GlobalValue, TraitId, TraitImplKind, TraitItemId, + TraitLookupMode, }, shared::Signedness, }; @@ -285,25 +286,121 @@ impl Elaborator<'_> { } /// Resolve `Self::Foo` to an associated type on the current trait or trait impl. + /// Also searches parent traits. fn lookup_associated_type_on_self(&self, path: &TypedPath) -> Option { if path.segments.len() == 2 && path.first_name() == Some(SELF_TYPE_NAME) { - if let Some(trait_id) = self.current_trait { - let the_trait = self.interner.get_trait(trait_id); - if let Some(typ) = the_trait.get_associated_type(path.last_name()) { - return Some( - typ.clone() - .into_named_generic(Some((SELF_TYPE_NAME, the_trait.name.as_str()))), - ); - } + let name = path.last_name(); + + // Inside a trait definition (not an impl): check this trait and its parent traits. + if self.current_trait_impl.is_none() + && let Some(trait_id) = self.current_trait + && let Some(typ) = self.lookup_associated_type_in_parent_traits(trait_id, name) + { + return Some(typ); } + // Inside a trait impl: check the impl's own types, then parent trait impls. if let Some(impl_id) = self.current_trait_impl { - let name = path.last_name(); if let Some(typ) = self.interner.find_associated_type_for_impl(impl_id, name) { return Some(typ.clone()); } + + if let Some(trait_id) = self.current_trait + && let Some(typ) = self.lookup_associated_type_in_parent_impls(trait_id, name) + { + return Some(typ); + } + } + } + None + } + + /// Search for an associated type in a trait and its parent trait hierarchy. + /// Used inside trait definitions to resolve `Self::Foo` when `Foo` may be + /// defined on a parent trait. + fn lookup_associated_type_in_parent_traits( + &self, + trait_id: TraitId, + name: &str, + ) -> Option { + let the_trait = self.interner.get_trait(trait_id); + if let Some(typ) = the_trait.get_associated_type(name) { + return Some( + typ.clone().into_named_generic(Some((SELF_TYPE_NAME, the_trait.name.as_str()))), + ); + } + + let parent_trait_ids: Vec<_> = + the_trait.trait_bounds.iter().map(|bound| bound.trait_id).collect(); + for parent_id in parent_trait_ids { + if let Some(typ) = self.lookup_associated_type_in_parent_traits(parent_id, name) { + return Some(typ); } } + + None + } + + /// Search for an associated type in parent 'trait impls'. + /// Used inside trait impls to resolve `Self::Foo` when `Foo` is defined on a parent trait. + /// For example, in `impl Lookup for u32`, resolving `Self::Key` where `Key` is from `KeyType`. + fn lookup_associated_type_in_parent_impls( + &self, + trait_id: TraitId, + name: &str, + ) -> Option { + let the_trait = self.interner.get_trait(trait_id); + let parent_bounds = the_trait.trait_bounds.clone(); + let self_type = self.self_type.as_ref()?; + + for parent_bound in &parent_bounds { + let result = self.interner.try_lookup_trait_implementation( + self_type, + parent_bound.trait_id, + &parent_bound.trait_generics.ordered, + &parent_bound.trait_generics.named, + TraitLookupMode::Default, + ); + + match result { + Ok(( + TraitImplKind::Normal(parent_impl_id) + | TraitImplKind::Prepared(parent_impl_id, _), + _, + _, + )) => { + if let Some(typ) = + self.interner.find_associated_type_for_impl(parent_impl_id, name) + { + return Some(typ.clone()); + } + } + Ok((TraitImplKind::Assumed { trait_generics, .. }, _, _)) => { + for named in &trait_generics.named { + if named.name.as_str() == name { + return Some(named.typ.clone()); + } + } + } + _ => { + // Lookup failed (e.g. self type is too generic). Fall back to + // searching the parent trait's definition for the associated type. + if let Some(typ) = + self.lookup_associated_type_in_parent_traits(parent_bound.trait_id, name) + { + return Some(typ); + } + } + } + + // Recurse into grandparent traits + if let Some(typ) = + self.lookup_associated_type_in_parent_impls(parent_bound.trait_id, name) + { + return Some(typ); + } + } + None } @@ -311,6 +408,7 @@ impl Elaborator<'_> { /// /// For example, in `impl Foo for T { type Bar = T::Qux; }`, this resolves `T::Qux` /// by finding that `T` has a bound `Baz` which defines the associated type `Qux`. + /// Also searches parent traits. fn lookup_associated_type_on_generic(&mut self, path: &TypedPath) -> Option { if self.trait_bounds.is_empty() { return None; @@ -326,24 +424,23 @@ impl Elaborator<'_> { // Check if first segment is a generic parameter self.find_generic(type_name)?; - // Search trait bounds for this generic to find the associated type - let mut found_types = self - .trait_bounds - .iter() - .filter_map(|constraint| { - if let Type::NamedGeneric(generic) = &constraint.typ - && generic.name.as_ref() == type_name - { - for named_generic in &constraint.trait_bound.trait_generics.named { - if named_generic.name.as_str() == assoc_name { - let trait_id = constraint.trait_bound.trait_id; - return Some((trait_id, named_generic.typ.clone())); - } + // Search trait bounds for this generic to find the associated type directly. + // Parent associated types are expected to be in `self.trait_bounds` already, + // added during function elaboration. + let mut found_types = Vec::new(); + + for constraint in &self.trait_bounds { + if let Type::NamedGeneric(generic) = &constraint.typ + && generic.name.as_ref() == type_name + { + for named_generic in &constraint.trait_bound.trait_generics.named { + if named_generic.name.as_str() == assoc_name { + let trait_id = constraint.trait_bound.trait_id; + found_types.push((trait_id, named_generic.typ.clone())); } } - None - }) - .collect::>(); + } + } match found_types.len() { 0 => None, // Fall through to normal resolution diff --git a/compiler/noirc_frontend/src/node_interner/trait_impl.rs b/compiler/noirc_frontend/src/node_interner/trait_impl.rs index 3fd76290aac..33ca9b0c7cd 100644 --- a/compiler/noirc_frontend/src/node_interner/trait_impl.rs +++ b/compiler/noirc_frontend/src/node_interner/trait_impl.rs @@ -90,7 +90,34 @@ impl NodeInterner { )); Ok(true) } - Ok(_) => Ok(false), + Ok(_) => { + // When a parent trait constraint provides fresh type variables for + // associated types, replace the existing type variables + // with the new ones so they share the same binding. + if !trait_generics.named.is_empty() + && let Some(entries) = self.trait_implementation_map.get_mut(&trait_id) + { + for (_, impl_kind) in entries.iter_mut() { + if let TraitImplKind::Assumed { + object_type: existing_obj, + trait_generics: existing_generics, + } = impl_kind + && *existing_obj == object_type + { + // Replace existing named generics with new ones by name + for new_named in &trait_generics.named { + for existing_named in &mut existing_generics.named { + if existing_named.name.as_str() == new_named.name.as_str() { + existing_named.typ = new_named.typ.clone(); + } + } + } + return Ok(true); + } + } + } + Ok(false) + } Err( error @ (ImplSearchErrorKind::NoImplFound(_) | ImplSearchErrorKind::MultipleMatching(_) diff --git a/compiler/noirc_frontend/src/tests/traits/trait_associated_items.rs b/compiler/noirc_frontend/src/tests/traits/trait_associated_items.rs index 2397e02c30b..36689db2174 100644 --- a/compiler/noirc_frontend/src/tests/traits/trait_associated_items.rs +++ b/compiler/noirc_frontend/src/tests/traits/trait_associated_items.rs @@ -1561,11 +1561,9 @@ fn associated_type_shorthand_simple_identity() { assert_no_errors(src); } -/// TODO(https://github.com/noir-lang/noir/issues/11562): remove should_panic once fixed +/// Regression test for https://github.com/noir-lang/noir/issues/11562 #[test] -#[should_panic(expected = "Expected no errors")] fn associated_type_of_self_generic_in_param_position_for_parent() { - // Bug: Self::Key cannot be resolved let src = r#" trait KeyType { type Key; @@ -1586,11 +1584,10 @@ fn associated_type_of_self_generic_in_param_position_for_parent() { assert_no_errors(src); } -/// TODO(https://github.com/noir-lang/noir/issues/11562): remove should_panic once fixed +/// Regression test for https://github.com/noir-lang/noir/issues/11562 #[test] -#[should_panic(expected = "Expected no errors")] fn associated_type_of_non_self_generic_in_param_position_for_parent() { - // Bug: Self::Key cannot be resolved + // M::Key should resolve when Key is defined on parent trait KeyType let src = r#" trait KeyType { type Key; @@ -1598,7 +1595,15 @@ fn associated_type_of_non_self_generic_in_param_position_for_parent() { trait Lookup: KeyType {} - fn find(m: M, key: M::Key) {} + struct Map {} + impl KeyType for Map { type Key = Field; } + impl Lookup for Map {} + + fn find(_m: M, _key: M::Key) {} + + fn main() { + find(Map {}, 1); + } "#; assert_no_errors(src); } @@ -1863,6 +1868,117 @@ fn associated_constant_in_return_type_with_generic_impl_forwarding() { assert_no_errors(src); } +/// Regression test for https://github.com/noir-lang/noir/issues/11562 +/// M::Key resolves through a grandparent chain: A: B, B: C, type Key on C. +#[test] +fn associated_type_of_generic_resolves_through_grandparent() { + let src = r#" + trait Level1 { + type Key; + } + + trait Level2: Level1 {} + + trait Level3: Level2 {} + + struct Data {} + impl Level1 for Data { type Key = Field; } + impl Level2 for Data {} + impl Level3 for Data {} + + fn use_key(_m: M, _k: M::Key) {} + + fn main() { + use_key(Data {}, 42); + } + "#; + assert_no_errors(src); +} + +/// Regression test for https://github.com/noir-lang/noir/issues/11562 +/// Diamond inheritance: both paths to the same associated type should not cause ambiguity. +/// C inherits from A and B, both of which inherit from Base which defines Key. +#[test] +fn associated_type_of_generic_diamond_inheritance() { + let src = r#" + trait Base { + type Key; + } + + trait Left: Base {} + trait Right: Base {} + + trait Child: Left + Right {} + + struct S {} + impl Base for S { type Key = Field; } + impl Left for S {} + impl Right for S {} + impl Child for S {} + + fn use_key(_m: M, _k: M::Key) {} + + fn main() { + use_key(S {}, 1); + } + "#; + assert_no_errors(src); +} + +/// Regression test for https://github.com/noir-lang/noir/issues/11562 +/// M::Key from a parent trait used in an impl method signature and body. +#[test] +fn associated_type_of_generic_in_impl_method() { + let src = r#" + trait HasKey { + type Key; + } + + trait Lookup: HasKey {} + + struct Processor {} + + impl Processor { + fn process(_m: M, key: M::Key) -> M::Key { + key + } + } + + struct Map {} + impl HasKey for Map { type Key = Field; } + impl Lookup for Map {} + + fn main() { + assert(Processor::process(Map {}, 42) == 42); + } + "#; + assert_no_errors(src); +} + +/// Regression test for https://github.com/noir-lang/noir/issues/11562 +/// Ambiguous associated type: M has bounds on two unrelated traits that each define Key. +/// This should produce a clear error. +#[test] +fn associated_type_of_generic_ambiguous_from_multiple_traits() { + let src = r#" + trait Foo { + type Key; + } + + trait Bar { + type Key; + } + + fn ambiguous(_m: M, _k: M::Key) where M: Foo + Bar {} + ^^^^^^^^^ unused function ambiguous + ~~~~~~~~~ unused function + ^^^^^^ Multiple applicable items in scope + ~~~~~~ Multiple traits which provide `Key` are implemented and in scope: `Bar`, `Foo` + fn main() {} + "#; + check_errors(src); +} + // Regression test for https://github.com/noir-lang/noir/issues/11655 #[test] fn self_method_call_on_trait_impl_for_unknown_trait() {