From 5300ec321fb99ddaad32e83f751aed28e175736f Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Thu, 9 Jan 2025 18:16:57 -0300 Subject: [PATCH] fix: require generic trait impls to be in scope to call them (#6913) Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com> --- .../noirc_frontend/src/elaborator/types.rs | 80 +++++++++++-------- compiler/noirc_frontend/src/node_interner.rs | 45 +++-------- compiler/noirc_frontend/src/tests/traits.rs | 70 ++++++++++++++++ 3 files changed, 126 insertions(+), 69 deletions(-) diff --git a/compiler/noirc_frontend/src/elaborator/types.rs b/compiler/noirc_frontend/src/elaborator/types.rs index 9b3df4631b..72e46d36c2 100644 --- a/compiler/noirc_frontend/src/elaborator/types.rs +++ b/compiler/noirc_frontend/src/elaborator/types.rs @@ -1361,7 +1361,7 @@ impl<'context> Elaborator<'context> { span: Span, has_self_arg: bool, ) -> Option { - // First search in the struct methods + // First search in the type methods. If there is one, that's the one. if let Some(method_id) = self.interner.lookup_direct_method(object_type, method_name, has_self_arg) { @@ -1372,43 +1372,55 @@ impl<'context> Elaborator<'context> { let trait_methods = self.interner.lookup_trait_methods(object_type, method_name, has_self_arg); - if trait_methods.is_empty() { - // If we couldn't find any trait methods, search in - // impls for all types `T`, e.g. `impl Foo for T` - if let Some(func_id) = - self.interner.lookup_generic_method(object_type, method_name, has_self_arg) - { - return Some(HirMethodReference::FuncId(func_id)); - } + // If there's at least one matching trait method we need to see if only one is in scope. + if !trait_methods.is_empty() { + return self.return_trait_method_in_scope(&trait_methods, method_name, span); + } - if let Type::Struct(struct_type, _) = object_type { - let has_field_with_function_type = - struct_type.borrow().get_fields_as_written().into_iter().any(|field| { - field.name.0.contents == method_name && field.typ.is_function() - }); - if has_field_with_function_type { - self.push_err(TypeCheckError::CannotInvokeStructFieldFunctionType { - method_name: method_name.to_string(), - object_type: object_type.clone(), - span, - }); - } else { - self.push_err(TypeCheckError::UnresolvedMethodCall { - method_name: method_name.to_string(), - object_type: object_type.clone(), - span, - }); - } - return None; + // If we couldn't find any trait methods, search in + // impls for all types `T`, e.g. `impl Foo for T` + let generic_methods = + self.interner.lookup_generic_methods(object_type, method_name, has_self_arg); + if !generic_methods.is_empty() { + return self.return_trait_method_in_scope(&generic_methods, method_name, span); + } + + if let Type::Struct(struct_type, _) = object_type { + let has_field_with_function_type = struct_type + .borrow() + .get_fields_as_written() + .into_iter() + .any(|field| field.name.0.contents == method_name && field.typ.is_function()); + if has_field_with_function_type { + self.push_err(TypeCheckError::CannotInvokeStructFieldFunctionType { + method_name: method_name.to_string(), + object_type: object_type.clone(), + span, + }); } else { - // It could be that this type is a composite type that is bound to a trait, - // for example `x: (T, U) ... where (T, U): SomeTrait` - // (so this case is a generalization of the NamedGeneric case) - return self.lookup_method_in_trait_constraints(object_type, method_name, span); + self.push_err(TypeCheckError::UnresolvedMethodCall { + method_name: method_name.to_string(), + object_type: object_type.clone(), + span, + }); } + None + } else { + // It could be that this type is a composite type that is bound to a trait, + // for example `x: (T, U) ... where (T, U): SomeTrait` + // (so this case is a generalization of the NamedGeneric case) + self.lookup_method_in_trait_constraints(object_type, method_name, span) } + } - // We found some trait methods... but is only one of them currently in scope? + /// Given a list of functions and the trait they belong to, returns the one function + /// that is in scope. + fn return_trait_method_in_scope( + &mut self, + trait_methods: &[(FuncId, TraitId)], + method_name: &str, + span: Span, + ) -> Option { let module_id = self.module_id(); let module_data = self.get_module(module_id); @@ -1490,7 +1502,7 @@ impl<'context> Elaborator<'context> { fn trait_hir_method_reference( &self, trait_id: TraitId, - trait_methods: Vec<(FuncId, TraitId)>, + trait_methods: &[(FuncId, TraitId)], method_name: &str, span: Span, ) -> HirMethodReference { diff --git a/compiler/noirc_frontend/src/node_interner.rs b/compiler/noirc_frontend/src/node_interner.rs index c1a405535f..a2b1d7fc8f 100644 --- a/compiler/noirc_frontend/src/node_interner.rs +++ b/compiler/noirc_frontend/src/node_interner.rs @@ -1746,7 +1746,7 @@ impl NodeInterner { Ok(()) } - /// Looks up a method that's directly defined in the given struct. + /// Looks up a method that's directly defined in the given type. pub fn lookup_direct_method( &self, typ: &Type, @@ -1761,7 +1761,7 @@ impl NodeInterner { .and_then(|methods| methods.find_direct_method(typ, has_self_arg, self)) } - /// Looks up a methods that apply to the given struct but are defined in traits. + /// Looks up a methods that apply to the given type but are defined in traits. pub fn lookup_trait_methods( &self, typ: &Type, @@ -1780,43 +1780,18 @@ impl NodeInterner { } } - /// Select the 1 matching method with an object type matching `typ` - fn find_matching_method( - &self, - typ: &Type, - methods: Option<&Methods>, - method_name: &str, - has_self_arg: bool, - ) -> Option { - if let Some(method) = methods.and_then(|m| m.find_matching_method(typ, has_self_arg, self)) - { - Some(method) - } else { - self.lookup_generic_method(typ, method_name, has_self_arg) - } - } - - /// Looks up a method at impls for all types `T`, e.g. `impl Foo for T` - pub fn lookup_generic_method( + /// Looks up methods at impls for all types `T`, e.g. `impl Foo for T` + pub fn lookup_generic_methods( &self, typ: &Type, method_name: &str, has_self_arg: bool, - ) -> Option { - let global_methods = self.methods.get(&TypeMethodKey::Generic)?.get(method_name)?; - global_methods.find_matching_method(typ, has_self_arg, self) - } - - /// Looks up a given method name on the given primitive type. - pub fn lookup_primitive_method( - &self, - typ: &Type, - method_name: &str, - has_self_arg: bool, - ) -> Option { - let key = get_type_method_key(typ)?; - let methods = self.methods.get(&key)?.get(method_name)?; - self.find_matching_method(typ, Some(methods), method_name, has_self_arg) + ) -> Vec<(FuncId, TraitId)> { + self.methods + .get(&TypeMethodKey::Generic) + .and_then(|h| h.get(method_name)) + .map(|methods| methods.find_trait_methods(typ, has_self_arg, self)) + .unwrap_or_default() } /// Returns what the next trait impl id is expected to be. diff --git a/compiler/noirc_frontend/src/tests/traits.rs b/compiler/noirc_frontend/src/tests/traits.rs index f573045131..3a55fc2b67 100644 --- a/compiler/noirc_frontend/src/tests/traits.rs +++ b/compiler/noirc_frontend/src/tests/traits.rs @@ -879,6 +879,43 @@ fn errors_if_multiple_trait_methods_are_in_scope_for_function_call() { assert_eq!(traits, vec!["private_mod::Foo", "private_mod::Foo2"]); } +#[test] +fn warns_if_trait_is_not_in_scope_for_method_call_and_there_is_only_one_trait_method() { + let src = r#" + fn main() { + let bar = Bar { x: 42 }; + let _ = bar.foo(); + } + + pub struct Bar { + x: i32, + } + + mod private_mod { + pub trait Foo { + fn foo(self) -> i32; + } + + impl Foo for super::Bar { + fn foo(self) -> i32 { + self.x + } + } + } + "#; + let errors = get_program_errors(src); + assert_eq!(errors.len(), 1); + + let CompilationError::ResolverError(ResolverError::PathResolutionError( + PathResolutionError::TraitMethodNotInScope { ident, trait_name }, + )) = &errors[0].0 + else { + panic!("Expected a 'trait method not in scope' error"); + }; + assert_eq!(ident.to_string(), "foo"); + assert_eq!(trait_name, "private_mod::Foo"); +} + #[test] fn calls_trait_method_if_it_is_in_scope() { let src = r#" @@ -1166,3 +1203,36 @@ fn warns_if_trait_is_not_in_scope_for_primitive_method_call_and_there_is_only_on assert_eq!(ident.to_string(), "foo"); assert_eq!(trait_name, "private_mod::Foo"); } + +#[test] +fn warns_if_trait_is_not_in_scope_for_generic_function_call_and_there_is_only_one_trait_method() { + let src = r#" + fn main() { + let x: i32 = 1; + let _ = x.foo(); + } + + mod private_mod { + pub trait Foo { + fn foo(self) -> i32; + } + + impl Foo for T { + fn foo(self) -> i32 { + 42 + } + } + } + "#; + let errors = get_program_errors(src); + assert_eq!(errors.len(), 1); + + let CompilationError::ResolverError(ResolverError::PathResolutionError( + PathResolutionError::TraitMethodNotInScope { ident, trait_name }, + )) = &errors[0].0 + else { + panic!("Expected a 'trait method not in scope' error"); + }; + assert_eq!(ident.to_string(), "foo"); + assert_eq!(trait_name, "private_mod::Foo"); +}