From 8d6667773db13392a97a610cb5b996bffdf3e405 Mon Sep 17 00:00:00 2001 From: jfecher Date: Mon, 11 Mar 2024 05:30:26 -0500 Subject: [PATCH] chore: Move `check_method_signatures` to type checking phase (#4516) # Description ## Problem\* Resolves https://github.com/noir-lang/noir/issues/3583 ## Summary\* Resolves an old TODO by moving this function to the type checking phase. I've rewritten the function since it was binding certain type variables then unbound them afterward. I've changed that to substitute new variables and only use `try_unify` to avoid accidentally binding anything. ## Additional Context ## Documentation\* Check one: - [x] No documentation needed. - [ ] Documentation included in this PR. - [ ] **[Exceptional Case]** Documentation to be submitted in a separate PR. # PR Checklist\* - [x] I have tested the changes locally. - [x] I have formatted the changes with [Prettier](https://prettier.io/) and/or `cargo fmt` on default settings. --- .../src/hir/def_collector/dc_crate.rs | 176 +++--------------- .../src/hir/def_collector/errors.rs | 20 -- .../src/hir/resolution/resolver.rs | 10 +- .../src/hir/resolution/traits.rs | 17 +- .../src/hir/type_check/errors.rs | 32 +++- .../noirc_frontend/src/hir/type_check/expr.rs | 4 +- .../noirc_frontend/src/hir/type_check/mod.rs | 159 +++++++++++++++- .../noirc_frontend/src/hir_def/function.rs | 10 +- compiler/noirc_frontend/src/tests.rs | 16 +- 9 files changed, 239 insertions(+), 205 deletions(-) diff --git a/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs index 27b1d376f11..62febc89899 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs @@ -5,13 +5,14 @@ use crate::hir::def_map::{CrateDefMap, LocalModuleId, ModuleId}; use crate::hir::resolution::errors::ResolverError; use crate::hir::resolution::import::{resolve_import, ImportDirective}; -use crate::hir::resolution::resolver::Resolver; use crate::hir::resolution::{ collect_impls, collect_trait_impls, path_resolver, resolve_free_functions, resolve_globals, resolve_impls, resolve_structs, resolve_trait_by_path, resolve_trait_impls, resolve_traits, resolve_type_aliases, }; -use crate::hir::type_check::{type_check_func, TypeCheckError, TypeChecker}; +use crate::hir::type_check::{ + check_trait_impl_method_matches_declaration, type_check_func, TypeCheckError, TypeChecker, +}; use crate::hir::Context; use crate::macros_api::{MacroError, MacroProcessor}; @@ -20,8 +21,7 @@ use crate::node_interner::{FuncId, GlobalId, NodeInterner, StructId, TraitId, Ty use crate::parser::{ParserError, SortedModule}; use crate::{ ExpressionKind, Ident, LetStatement, Literal, NoirFunction, NoirStruct, NoirTrait, - NoirTypeAlias, Path, PathKind, Type, TypeBindings, UnresolvedGenerics, - UnresolvedTraitConstraint, UnresolvedType, + NoirTypeAlias, Path, PathKind, UnresolvedGenerics, UnresolvedTraitConstraint, UnresolvedType, }; use fm::FileId; use iter_extended::vecmap; @@ -368,12 +368,12 @@ impl DefCollector { &mut errors, )); - functions.extend(resolve_trait_impls( + let impl_functions = resolve_trait_impls( context, def_collector.collected_traits_impls, crate_id, &mut errors, - )); + ); for macro_processor in macro_processors { macro_processor.process_typed_ast(&crate_id, context).unwrap_or_else( @@ -387,6 +387,8 @@ impl DefCollector { errors.extend(type_check_globals(&mut context.def_interner, resolved_globals.globals)); errors.extend(type_check_functions(&mut context.def_interner, functions)); + errors.extend(type_check_trait_impl_signatures(&mut context.def_interner, &impl_functions)); + errors.extend(type_check_functions(&mut context.def_interner, impl_functions)); errors } } @@ -468,158 +470,24 @@ fn type_check_functions( .into_iter() .flat_map(|(file, func)| { type_check_func(interner, func) - .iter() - .cloned() + .into_iter() .map(|e| (e.into(), file)) .collect::>() }) .collect() } -// TODO(vitkov): Move this out of here and into type_check -#[allow(clippy::too_many_arguments)] -pub(crate) fn check_methods_signatures( - resolver: &mut Resolver, - impl_methods: &[(FileId, FuncId)], - trait_id: TraitId, - trait_name_span: Span, - // These are the generics on the trait itself from the impl. - // E.g. in `impl Foo for Bar`, this is `vec![A, B]`. - trait_generics: Vec, - trait_impl_generic_count: usize, - file_id: FileId, - errors: &mut Vec<(CompilationError, FileId)>, -) { - let self_type = resolver.get_self_type().expect("trait impl must have a Self type").clone(); - let trait_generics = vecmap(trait_generics, |typ| resolver.resolve_type(typ)); - - // Temporarily bind the trait's Self type to self_type so we can type check - let the_trait = resolver.interner.get_trait_mut(trait_id); - the_trait.self_type_typevar.bind(self_type); - - if trait_generics.len() != the_trait.generics.len() { - let error = DefCollectorErrorKind::MismatchGenericCount { - actual_generic_count: trait_generics.len(), - expected_generic_count: the_trait.generics.len(), - // Preferring to use 'here' over a more precise term like 'this reference' - // to try to make the error easier to understand for newer users. - location: "here it", - origin: the_trait.name.to_string(), - span: trait_name_span, - }; - errors.push((error.into(), file_id)); - } - - // We also need to bind the traits generics to the trait's generics on the impl - for (generic, binding) in the_trait.generics.iter().zip(trait_generics) { - generic.bind(binding); - } - - // Temporarily take the trait's methods so we can use both them and a mutable reference - // to the interner within the loop. - let trait_methods = std::mem::take(&mut the_trait.methods); - - for (file_id, func_id) in impl_methods { - let func_name = resolver.interner.function_name(func_id).to_owned(); - - // This is None in the case where the impl block has a method that's not part of the trait. - // If that's the case, a `MethodNotInTrait` error has already been thrown, and we can ignore - // the impl method, since there's nothing in the trait to match its signature against. - if let Some(trait_method) = - trait_methods.iter().find(|method| method.name.0.contents == func_name) - { - let impl_method = resolver.interner.function_meta(func_id); - - let impl_method_generic_count = - impl_method.typ.generic_count() - trait_impl_generic_count; - - // We subtract 1 here to account for the implicit generic `Self` type that is on all - // traits (and thus trait methods) but is not required (or allowed) for users to specify. - let the_trait = resolver.interner.get_trait(trait_id); - let trait_method_generic_count = - trait_method.generics().len() - 1 - the_trait.generics.len(); - - if impl_method_generic_count != trait_method_generic_count { - let trait_name = resolver.interner.get_trait(trait_id).name.clone(); - - let error = DefCollectorErrorKind::MismatchGenericCount { - actual_generic_count: impl_method_generic_count, - expected_generic_count: trait_method_generic_count, - origin: format!("{}::{}", trait_name, func_name), - location: "this method", - span: impl_method.location.span, - }; - errors.push((error.into(), *file_id)); - } - - // This instantiation is technically not needed. We could bind each generic in the - // trait function to the impl's corresponding generic but to do so we'd have to rely - // on the trait function's generics being first in the generic list, since the same - // list also contains the generic `Self` variable, and any generics on the trait itself. - // - // Instantiating the impl method's generics here instead is a bit less precise but - // doesn't rely on any orderings that may be changed. - let impl_function_type = impl_method.typ.instantiate(resolver.interner).0; - - let mut bindings = TypeBindings::new(); - let mut typecheck_errors = Vec::new(); - - if let Type::Function(impl_params, impl_return, _) = impl_function_type.as_monotype() { - if trait_method.arguments().len() != impl_params.len() { - let error = DefCollectorErrorKind::MismatchTraitImplementationNumParameters { - actual_num_parameters: impl_method.parameters.0.len(), - expected_num_parameters: trait_method.arguments().len(), - trait_name: resolver.interner.get_trait(trait_id).name.to_string(), - method_name: func_name.to_string(), - span: impl_method.location.span, - }; - errors.push((error.into(), *file_id)); - } - - // Check the parameters of the impl method against the parameters of the trait method - let args = trait_method.arguments().iter(); - let args_and_params = args.zip(impl_params).zip(&impl_method.parameters.0); - - for (parameter_index, ((expected, actual), (hir_pattern, _, _))) in - args_and_params.enumerate() - { - if expected.try_unify(actual, &mut bindings).is_err() { - typecheck_errors.push(TypeCheckError::TraitMethodParameterTypeMismatch { - method_name: func_name.to_string(), - expected_typ: expected.to_string(), - actual_typ: actual.to_string(), - parameter_span: hir_pattern.span(), - parameter_index: parameter_index + 1, - }); - } - } - - if trait_method.return_type().try_unify(impl_return, &mut bindings).is_err() { - let impl_method = resolver.interner.function_meta(func_id); - let ret_type_span = impl_method.return_type.get_type().span; - let expr_span = ret_type_span.expect("return type must always have a span"); - - let expected_typ = trait_method.return_type().to_string(); - let expr_typ = impl_method.return_type().to_string(); - let error = TypeCheckError::TypeMismatch { expr_typ, expected_typ, expr_span }; - typecheck_errors.push(error); - } - } else { - unreachable!( - "impl_function_type is not a function type, it is: {impl_function_type}" - ); - } - - errors.extend(typecheck_errors.iter().cloned().map(|e| (e.into(), *file_id))); - } - } - - // Now unbind `Self` and the trait's generics - let the_trait = resolver.interner.get_trait_mut(trait_id); - the_trait.set_methods(trait_methods); - the_trait.self_type_typevar.unbind(the_trait.self_type_typevar_id); - - for generic in &the_trait.generics { - generic.unbind(generic.id()); - } +fn type_check_trait_impl_signatures( + interner: &mut NodeInterner, + file_func_ids: &[(FileId, FuncId)], +) -> Vec<(CompilationError, fm::FileId)> { + file_func_ids + .iter() + .flat_map(|(file, func)| { + check_trait_impl_method_matches_declaration(interner, *func) + .into_iter() + .map(|e| (e.into(), *file)) + .collect::>() + }) + .collect() } diff --git a/compiler/noirc_frontend/src/hir/def_collector/errors.rs b/compiler/noirc_frontend/src/hir/def_collector/errors.rs index de45be48c4e..29daf5d6369 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/errors.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/errors.rs @@ -41,14 +41,6 @@ pub enum DefCollectorErrorKind { OverlappingImplNote { span: Span }, #[error("Cannot `impl` a type defined outside the current crate")] ForeignImpl { span: Span, type_name: String }, - #[error("Mismatched number of parameters in trait implementation")] - MismatchTraitImplementationNumParameters { - actual_num_parameters: usize, - expected_num_parameters: usize, - trait_name: String, - method_name: String, - span: Span, - }, #[error("Mismatched number of generics in {location}")] MismatchGenericCount { actual_generic_count: usize, @@ -176,18 +168,6 @@ impl From for Diagnostic { "".to_string(), trait_path.span(), ), - DefCollectorErrorKind::MismatchTraitImplementationNumParameters { - expected_num_parameters, - actual_num_parameters, - trait_name, - method_name, - span, - } => { - let plural = if expected_num_parameters == 1 { "" } else { "s" }; - let primary_message = format!( - "`{trait_name}::{method_name}` expects {expected_num_parameters} parameter{plural}, but this method has {actual_num_parameters}"); - Diagnostic::simple_error(primary_message, "".to_string(), span) - } DefCollectorErrorKind::MismatchGenericCount { actual_generic_count, expected_generic_count, diff --git a/compiler/noirc_frontend/src/hir/resolution/resolver.rs b/compiler/noirc_frontend/src/hir/resolution/resolver.rs index 322891f0ae9..875d0ceb85e 100644 --- a/compiler/noirc_frontend/src/hir/resolution/resolver.rs +++ b/compiler/noirc_frontend/src/hir/resolution/resolver.rs @@ -217,6 +217,7 @@ impl<'a> Resolver<'a> { pub fn resolve_trait_function( &mut self, name: &Ident, + generics: &UnresolvedGenerics, parameters: &[(Ident, UnresolvedType)], return_type: &FunctionReturnType, where_clause: &[UnresolvedTraitConstraint], @@ -237,7 +238,7 @@ impl<'a> Resolver<'a> { is_internal: false, is_unconstrained: false, visibility: FunctionVisibility::Public, // Trait functions are always public - generics: Vec::new(), // self.generics should already be set + generics: generics.clone(), parameters: vecmap(parameters, |(name, typ)| Param { visibility: Visibility::Private, pattern: Pattern::Identifier(name.clone()), @@ -975,11 +976,18 @@ impl<'a> Resolver<'a> { self.handle_function_type(&func_id); self.handle_is_function_internal(&func_id); + let direct_generics = func.def.generics.iter(); + let direct_generics = direct_generics + .filter_map(|generic| self.find_generic(&generic.0.contents)) + .map(|(name, typevar, _span)| (name.clone(), typevar.clone())) + .collect(); + FuncMeta { name: name_ident, kind: func.kind, location, typ, + direct_generics, trait_impl: self.current_trait_impl, parameters: parameters.into(), return_type: func.def.return_type.clone(), diff --git a/compiler/noirc_frontend/src/hir/resolution/traits.rs b/compiler/noirc_frontend/src/hir/resolution/traits.rs index 8f966be312b..5d546954f0d 100644 --- a/compiler/noirc_frontend/src/hir/resolution/traits.rs +++ b/compiler/noirc_frontend/src/hir/resolution/traits.rs @@ -8,9 +8,7 @@ use crate::{ graph::CrateId, hir::{ def_collector::{ - dc_crate::{ - check_methods_signatures, CompilationError, UnresolvedTrait, UnresolvedTraitImpl, - }, + dc_crate::{CompilationError, UnresolvedTrait, UnresolvedTraitImpl}, errors::{DefCollectorErrorKind, DuplicateType}, }, def_map::{CrateDefMap, ModuleDefId, ModuleId}, @@ -131,6 +129,7 @@ fn resolve_trait_methods( let func_id = unresolved_trait.method_ids[&name.0.contents]; let (_, func_meta) = resolver.resolve_trait_function( name, + generics, parameters, return_type, where_clause, @@ -365,6 +364,7 @@ pub(crate) fn resolve_trait_by_path( Err(_) => Err(DefCollectorErrorKind::TraitNotFound { trait_path: path }), } } + pub(crate) fn resolve_trait_impls( context: &mut Context, traits: Vec, @@ -424,17 +424,6 @@ pub(crate) fn resolve_trait_impls( new_resolver.set_self_type(Some(self_type.clone())); if let Some(trait_id) = maybe_trait_id { - check_methods_signatures( - &mut new_resolver, - &impl_methods, - trait_id, - trait_impl.trait_path.span(), - trait_impl.trait_generics, - trait_impl.generics.len(), - trait_impl.file_id, - errors, - ); - let where_clause = trait_impl .where_clause .into_iter() diff --git a/compiler/noirc_frontend/src/hir/type_check/errors.rs b/compiler/noirc_frontend/src/hir/type_check/errors.rs index cba2400441f..7eacc8eb2d1 100644 --- a/compiler/noirc_frontend/src/hir/type_check/errors.rs +++ b/compiler/noirc_frontend/src/hir/type_check/errors.rs @@ -44,7 +44,7 @@ pub enum TypeCheckError { #[error("Expected type {expected} is not the same as {actual}")] TypeMismatchWithSource { expected: Type, actual: Type, span: Span, source: Source }, #[error("Expected {expected:?} found {found:?}")] - ArityMisMatch { expected: u16, found: u16, span: Span }, + ArityMisMatch { expected: usize, found: usize, span: Span }, #[error("Return type in a function cannot be public")] PublicReturnType { typ: Type, span: Span }, #[error("Cannot cast type {from}, 'as' is only for primitive field or integer types")] @@ -53,8 +53,10 @@ pub enum TypeCheckError { ExpectedFunction { found: Type, span: Span }, #[error("Type {lhs_type} has no member named {field_name}")] AccessUnknownMember { lhs_type: Type, field_name: String, span: Span }, - #[error("Function expects {expected} parameters but {found} given")] + #[error("Function expects {expected} parameters but {found} were given")] ParameterCountMismatch { expected: usize, found: usize, span: Span }, + #[error("{item} expects {expected} generics but {found} were given")] + GenericCountMismatch { item: String, expected: usize, found: usize, span: Span }, #[error("Only integer and Field types may be casted to")] UnsupportedCast { span: Span }, #[error("Index {index} is out of bounds for this tuple {lhs_type} of length {length}")] @@ -124,6 +126,14 @@ pub enum TypeCheckError { UnconstrainedSliceReturnToConstrained { span: Span }, #[error("Only sized types may be used in the entry point to a program")] InvalidTypeForEntryPoint { span: Span }, + #[error("Mismatched number of parameters in trait implementation")] + MismatchTraitImplNumParameters { + actual_num_parameters: usize, + expected_num_parameters: usize, + trait_name: String, + method_name: String, + span: Span, + }, } impl TypeCheckError { @@ -193,6 +203,12 @@ impl From for Diagnostic { let msg = format!("Function expects {expected} parameter{empty_or_s} but {found} {was_or_were} given"); Diagnostic::simple_error(msg, String::new(), span) } + TypeCheckError::GenericCountMismatch { item, expected, found, span } => { + let empty_or_s = if expected == 1 { "" } else { "s" }; + let was_or_were = if found == 1 { "was" } else { "were" }; + let msg = format!("{item} expects {expected} generic{empty_or_s} but {found} {was_or_were} given"); + Diagnostic::simple_error(msg, String::new(), span) + } TypeCheckError::InvalidCast { span, .. } | TypeCheckError::ExpectedFunction { span, .. } | TypeCheckError::AccessUnknownMember { span, .. } @@ -289,6 +305,18 @@ impl From for Diagnostic { TypeCheckError::InvalidTypeForEntryPoint { span } => Diagnostic::simple_error( "Only sized types may be used in the entry point to a program".to_string(), "Slices, references, or any type containing them may not be used in main or a contract function".to_string(), span), + TypeCheckError::MismatchTraitImplNumParameters { + expected_num_parameters, + actual_num_parameters, + trait_name, + method_name, + span, + } => { + let plural = if expected_num_parameters == 1 { "" } else { "s" }; + let primary_message = format!( + "`{trait_name}::{method_name}` expects {expected_num_parameters} parameter{plural}, but this method has {actual_num_parameters}"); + Diagnostic::simple_error(primary_message, "".to_string(), span) + } } } } diff --git a/compiler/noirc_frontend/src/hir/type_check/expr.rs b/compiler/noirc_frontend/src/hir/type_check/expr.rs index c5287d35caf..7219f4d09c6 100644 --- a/compiler/noirc_frontend/src/hir/type_check/expr.rs +++ b/compiler/noirc_frontend/src/hir/type_check/expr.rs @@ -1040,9 +1040,9 @@ impl<'interner> TypeChecker<'interner> { } ret } + // ignoring env for subtype on purpose Type::Function(parameters, ret, _env) => { - // ignoring env for subtype on purpose - self.bind_function_type_impl(parameters.as_ref(), ret.as_ref(), args.as_ref(), span) + self.bind_function_type_impl(¶meters, &ret, &args, span) } Type::Error => Type::Error, found => { diff --git a/compiler/noirc_frontend/src/hir/type_check/mod.rs b/compiler/noirc_frontend/src/hir/type_check/mod.rs index aab793ec867..ab759f454e5 100644 --- a/compiler/noirc_frontend/src/hir/type_check/mod.rs +++ b/compiler/noirc_frontend/src/hir/type_check/mod.rs @@ -12,11 +12,17 @@ mod expr; mod stmt; pub use errors::TypeCheckError; +use noirc_errors::Span; use crate::{ - hir_def::{expr::HirExpression, function::Param, stmt::HirStatement, traits::TraitConstraint}, + hir_def::{ + expr::HirExpression, + function::{Param, Parameters}, + stmt::HirStatement, + traits::TraitConstraint, + }, node_interner::{ExprId, FuncId, GlobalId, NodeInterner}, - Type, + Type, TypeBindings, }; use self::errors::Source; @@ -179,6 +185,154 @@ fn function_info(interner: &NodeInterner, function_body_id: &ExprId) -> (noirc_e (expr_span, empty_function) } +/// Checks that the type of a function in a trait impl matches the type +/// of the corresponding function declaration in the trait itself. +/// +/// To do this, given a trait such as: +/// `trait Foo { fn foo(...); }` +/// +/// And an impl such as: +/// `impl Foo for Bar { fn foo(...); } ` +/// +/// We have to substitute: +/// - Self for Bar +/// - A for D +/// - B for F +/// +/// Before we can type check. Finally, we must also check that the unification +/// result does not introduce any new bindings. This can happen if the impl +/// function's type is more general than that of the trait function. E.g. +/// `fn baz(a: A, b: B)` when the impl required `fn baz(a: A, b: A)`. +/// +/// This does not type check the body of the impl function. +pub(crate) fn check_trait_impl_method_matches_declaration( + interner: &mut NodeInterner, + function: FuncId, +) -> Vec { + let meta = interner.function_meta(&function); + let method_name = interner.function_name(&function); + let mut errors = Vec::new(); + + let definition_type = meta.typ.as_monotype(); + + let impl_ = + meta.trait_impl.expect("Trait impl function should have a corresponding trait impl"); + let impl_ = interner.get_trait_implementation(impl_); + let impl_ = impl_.borrow(); + let trait_info = interner.get_trait(impl_.trait_id); + + let mut bindings = TypeBindings::new(); + bindings.insert( + trait_info.self_type_typevar_id, + (trait_info.self_type_typevar.clone(), impl_.typ.clone()), + ); + + if trait_info.generics.len() != impl_.trait_generics.len() { + let expected = trait_info.generics.len(); + let found = impl_.trait_generics.len(); + let span = impl_.ident.span(); + let item = trait_info.name.to_string(); + errors.push(TypeCheckError::GenericCountMismatch { item, expected, found, span }); + } + + // Substitute each generic on the trait with the corresponding generic on the impl + for (generic, arg) in trait_info.generics.iter().zip(&impl_.trait_generics) { + bindings.insert(generic.id(), (generic.clone(), arg.clone())); + } + + // If this is None, the trait does not have the corresponding function. + // This error should have been caught in name resolution already so we don't + // issue an error for it here. + if let Some(trait_fn_id) = trait_info.method_ids.get(method_name) { + let trait_fn_meta = interner.function_meta(trait_fn_id); + + if trait_fn_meta.direct_generics.len() != meta.direct_generics.len() { + let expected = trait_fn_meta.direct_generics.len(); + let found = meta.direct_generics.len(); + let span = meta.name.location.span; + let item = method_name.to_string(); + errors.push(TypeCheckError::GenericCountMismatch { item, expected, found, span }); + } + + // Substitute each generic on the trait function with the corresponding generic on the impl function + for ((_, trait_fn_generic), (name, impl_fn_generic)) in + trait_fn_meta.direct_generics.iter().zip(&meta.direct_generics) + { + let arg = Type::NamedGeneric(impl_fn_generic.clone(), name.clone()); + bindings.insert(trait_fn_generic.id(), (trait_fn_generic.clone(), arg)); + } + + let (declaration_type, _) = trait_fn_meta.typ.instantiate_with_bindings(bindings, interner); + + check_function_type_matches_expected_type( + &declaration_type, + definition_type, + method_name, + &meta.parameters, + meta.name.location.span, + &trait_info.name.0.contents, + &mut errors, + ); + } + + errors +} + +fn check_function_type_matches_expected_type( + expected: &Type, + actual: &Type, + method_name: &str, + actual_parameters: &Parameters, + span: Span, + trait_name: &str, + errors: &mut Vec, +) { + let mut bindings = TypeBindings::new(); + // Shouldn't need to unify envs, they should always be equal since they're both free functions + if let (Type::Function(params_a, ret_a, _env_a), Type::Function(params_b, ret_b, _env_b)) = + (expected, actual) + { + if params_a.len() == params_b.len() { + for (i, (a, b)) in params_a.iter().zip(params_b.iter()).enumerate() { + if a.try_unify(b, &mut bindings).is_err() { + errors.push(TypeCheckError::TraitMethodParameterTypeMismatch { + method_name: method_name.to_string(), + expected_typ: a.to_string(), + actual_typ: b.to_string(), + parameter_span: actual_parameters.0[i].0.span(), + parameter_index: i + 1, + }); + } + } + + if ret_b.try_unify(ret_a, &mut bindings).is_err() { + errors.push(TypeCheckError::TypeMismatch { + expected_typ: ret_a.to_string(), + expr_typ: ret_b.to_string(), + expr_span: span, + }); + } + } else { + errors.push(TypeCheckError::MismatchTraitImplNumParameters { + actual_num_parameters: params_b.len(), + expected_num_parameters: params_a.len(), + trait_name: trait_name.to_string(), + method_name: method_name.to_string(), + span, + }); + } + } + + // If result bindings is not empty, a type variable was bound which means the two + // signatures were not a perfect match. Note that this relies on us already binding + // all the expected generics to each other prior to this check. + if !bindings.is_empty() { + let expected_typ = expected.to_string(); + let expr_typ = actual.to_string(); + errors.push(TypeCheckError::TypeMismatch { expected_typ, expr_typ, expr_span: span }); + } +} + impl<'interner> TypeChecker<'interner> { fn new(interner: &'interner mut NodeInterner) -> Self { Self { interner, errors: Vec::new(), trait_constraints: Vec::new(), current_function: None } @@ -346,6 +500,7 @@ mod test { trait_impl: None, return_type: FunctionReturnType::Default(Span::default()), trait_constraints: Vec::new(), + direct_generics: Vec::new(), is_entry_point: true, }; interner.push_fn_meta(func_meta, func_id); diff --git a/compiler/noirc_frontend/src/hir_def/function.rs b/compiler/noirc_frontend/src/hir_def/function.rs index 82bbe1aa5b6..56543e8185c 100644 --- a/compiler/noirc_frontend/src/hir_def/function.rs +++ b/compiler/noirc_frontend/src/hir_def/function.rs @@ -1,12 +1,14 @@ use iter_extended::vecmap; use noirc_errors::{Location, Span}; +use std::rc::Rc; + use super::expr::{HirBlockExpression, HirExpression, HirIdent}; use super::stmt::HirPattern; use super::traits::TraitConstraint; use crate::node_interner::{ExprId, NodeInterner, TraitImplId}; use crate::FunctionKind; -use crate::{Distinctness, FunctionReturnType, Type, Visibility}; +use crate::{Distinctness, FunctionReturnType, Type, TypeVariable, Visibility}; /// A Hir function is a block expression /// with a list of statements @@ -103,6 +105,12 @@ pub struct FuncMeta { /// or a Type::Forall for generic functions. pub typ: Type, + /// The set of generics that are declared directly on this function in the source code. + /// This does not include generics from an outer scope, like those introduced by + /// an `impl` block. This also does not include implicit generics added by the compiler + /// such as a trait's `Self` type variable. + pub direct_generics: Vec<(Rc, TypeVariable)>, + pub location: Location, // This flag is needed for the attribute check pass diff --git a/compiler/noirc_frontend/src/tests.rs b/compiler/noirc_frontend/src/tests.rs index 9be6252b10a..3f78bd43ba9 100644 --- a/compiler/noirc_frontend/src/tests.rs +++ b/compiler/noirc_frontend/src/tests.rs @@ -535,15 +535,13 @@ mod test { assert!(errors.len() == 1, "Expected 1 error, got: {:?}", errors); for (err, _file_id) in errors { match &err { - CompilationError::DefinitionError( - DefCollectorErrorKind::MismatchTraitImplementationNumParameters { - actual_num_parameters, - expected_num_parameters, - trait_name, - method_name, - .. - }, - ) => { + CompilationError::TypeError(TypeCheckError::MismatchTraitImplNumParameters { + actual_num_parameters, + expected_num_parameters, + trait_name, + method_name, + .. + }) => { assert_eq!(actual_num_parameters, &1_usize); assert_eq!(expected_num_parameters, &2_usize); assert_eq!(method_name, "default");