From 7b3ae7e60af2886bd06e702fe44c68ec8ab7b5ac Mon Sep 17 00:00:00 2001 From: Yordan Madzhunkov Date: Thu, 7 Sep 2023 08:48:32 +0300 Subject: [PATCH] feat(traits): Add default and override of methods This commit removes some checks on UnresolvedTypes. It adds the following validity checks and tests for them: Duplicate trait blocks should not compile Duplicate trait impl blocks should not compile Duplicate method definitions in impl blocks should not compile Impl blocks invalid types should not compile --- .../dup_trait_implementation/src/main.nr | 2 - .../dup_trait_implementation_2/Nargo.toml | 7 + .../dup_trait_implementation_2/Prover.toml | 2 + .../dup_trait_implementation_2/src/main.nr | 18 ++ .../dup_trait_implementation_3/Nargo.toml | 7 + .../dup_trait_implementation_3/Prover.toml | 2 + .../dup_trait_implementation_3/src/main.nr | 19 +++ .../trait_default_implementation/Nargo.toml | 0 .../trait_default_implementation/Prover.toml | 0 .../trait_default_implementation/src/main.nr | 0 .../trait_override_implementation/Nargo.toml | 0 .../trait_override_implementation/Prover.toml | 0 .../trait_override_implementation/src/main.nr | 0 .../src/hir/def_collector/dc_crate.rs | 128 +++++++++++++-- .../src/hir/def_collector/dc_mod.rs | 154 +++++------------- .../src/hir/def_collector/errors.rs | 19 ++- crates/noirc_frontend/src/node_interner.rs | 30 +++- 17 files changed, 257 insertions(+), 131 deletions(-) create mode 100644 crates/nargo_cli/tests/compile_failure/dup_trait_implementation_2/Nargo.toml create mode 100644 crates/nargo_cli/tests/compile_failure/dup_trait_implementation_2/Prover.toml create mode 100644 crates/nargo_cli/tests/compile_failure/dup_trait_implementation_2/src/main.nr create mode 100644 crates/nargo_cli/tests/compile_failure/dup_trait_implementation_3/Nargo.toml create mode 100644 crates/nargo_cli/tests/compile_failure/dup_trait_implementation_3/Prover.toml create mode 100644 crates/nargo_cli/tests/compile_failure/dup_trait_implementation_3/src/main.nr rename crates/nargo_cli/tests/{compile_success_empty => execution_success}/trait_default_implementation/Nargo.toml (100%) rename crates/nargo_cli/tests/{compile_success_empty => execution_success}/trait_default_implementation/Prover.toml (100%) rename crates/nargo_cli/tests/{compile_success_empty => execution_success}/trait_default_implementation/src/main.nr (100%) rename crates/nargo_cli/tests/{compile_success_empty => execution_success}/trait_override_implementation/Nargo.toml (100%) rename crates/nargo_cli/tests/{compile_success_empty => execution_success}/trait_override_implementation/Prover.toml (100%) rename crates/nargo_cli/tests/{compile_success_empty => execution_success}/trait_override_implementation/src/main.nr (100%) diff --git a/crates/nargo_cli/tests/compile_failure/dup_trait_implementation/src/main.nr b/crates/nargo_cli/tests/compile_failure/dup_trait_implementation/src/main.nr index 6bb6cedefb5..cfc098a6ff7 100644 --- a/crates/nargo_cli/tests/compile_failure/dup_trait_implementation/src/main.nr +++ b/crates/nargo_cli/tests/compile_failure/dup_trait_implementation/src/main.nr @@ -25,6 +25,4 @@ impl Default for Foo { fn main(x: Field, y: Field) { - let first = Foo::default(x,y); - assert(first.bar == x); } diff --git a/crates/nargo_cli/tests/compile_failure/dup_trait_implementation_2/Nargo.toml b/crates/nargo_cli/tests/compile_failure/dup_trait_implementation_2/Nargo.toml new file mode 100644 index 00000000000..2276db5c741 --- /dev/null +++ b/crates/nargo_cli/tests/compile_failure/dup_trait_implementation_2/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "dup_trait_implementation_2" +type = "bin" +authors = [""] +compiler_version = "0.9.0" + +[dependencies] \ No newline at end of file diff --git a/crates/nargo_cli/tests/compile_failure/dup_trait_implementation_2/Prover.toml b/crates/nargo_cli/tests/compile_failure/dup_trait_implementation_2/Prover.toml new file mode 100644 index 00000000000..2c1854573a4 --- /dev/null +++ b/crates/nargo_cli/tests/compile_failure/dup_trait_implementation_2/Prover.toml @@ -0,0 +1,2 @@ +x = 1 +y = 2 diff --git a/crates/nargo_cli/tests/compile_failure/dup_trait_implementation_2/src/main.nr b/crates/nargo_cli/tests/compile_failure/dup_trait_implementation_2/src/main.nr new file mode 100644 index 00000000000..80b544b8e54 --- /dev/null +++ b/crates/nargo_cli/tests/compile_failure/dup_trait_implementation_2/src/main.nr @@ -0,0 +1,18 @@ +trait Default { +} + +struct Foo { + bar: Field, +} + +// Duplicate trait implementations should not compile +impl Default for Foo { +} + +// Duplicate trait implementations should not compile +impl Default for Foo { +} + + +fn main(x: Field, y: Field) { +} diff --git a/crates/nargo_cli/tests/compile_failure/dup_trait_implementation_3/Nargo.toml b/crates/nargo_cli/tests/compile_failure/dup_trait_implementation_3/Nargo.toml new file mode 100644 index 00000000000..ac04d9fac4d --- /dev/null +++ b/crates/nargo_cli/tests/compile_failure/dup_trait_implementation_3/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "dup_trait_implementation_3" +type = "bin" +authors = [""] +compiler_version = "0.9.0" + +[dependencies] \ No newline at end of file diff --git a/crates/nargo_cli/tests/compile_failure/dup_trait_implementation_3/Prover.toml b/crates/nargo_cli/tests/compile_failure/dup_trait_implementation_3/Prover.toml new file mode 100644 index 00000000000..2c1854573a4 --- /dev/null +++ b/crates/nargo_cli/tests/compile_failure/dup_trait_implementation_3/Prover.toml @@ -0,0 +1,2 @@ +x = 1 +y = 2 diff --git a/crates/nargo_cli/tests/compile_failure/dup_trait_implementation_3/src/main.nr b/crates/nargo_cli/tests/compile_failure/dup_trait_implementation_3/src/main.nr new file mode 100644 index 00000000000..2996b6a00bb --- /dev/null +++ b/crates/nargo_cli/tests/compile_failure/dup_trait_implementation_3/src/main.nr @@ -0,0 +1,19 @@ +trait Default { +} + +struct MyStruct { +} + +type MyType = MyStruct; + +// Duplicate trait implementations should not compile +impl Default for MyStruct { +} + +// Duplicate trait implementations should not compile +impl Default for MyType { +} + + +fn main(x: Field, y: Field) { +} diff --git a/crates/nargo_cli/tests/compile_success_empty/trait_default_implementation/Nargo.toml b/crates/nargo_cli/tests/execution_success/trait_default_implementation/Nargo.toml similarity index 100% rename from crates/nargo_cli/tests/compile_success_empty/trait_default_implementation/Nargo.toml rename to crates/nargo_cli/tests/execution_success/trait_default_implementation/Nargo.toml diff --git a/crates/nargo_cli/tests/compile_success_empty/trait_default_implementation/Prover.toml b/crates/nargo_cli/tests/execution_success/trait_default_implementation/Prover.toml similarity index 100% rename from crates/nargo_cli/tests/compile_success_empty/trait_default_implementation/Prover.toml rename to crates/nargo_cli/tests/execution_success/trait_default_implementation/Prover.toml diff --git a/crates/nargo_cli/tests/compile_success_empty/trait_default_implementation/src/main.nr b/crates/nargo_cli/tests/execution_success/trait_default_implementation/src/main.nr similarity index 100% rename from crates/nargo_cli/tests/compile_success_empty/trait_default_implementation/src/main.nr rename to crates/nargo_cli/tests/execution_success/trait_default_implementation/src/main.nr diff --git a/crates/nargo_cli/tests/compile_success_empty/trait_override_implementation/Nargo.toml b/crates/nargo_cli/tests/execution_success/trait_override_implementation/Nargo.toml similarity index 100% rename from crates/nargo_cli/tests/compile_success_empty/trait_override_implementation/Nargo.toml rename to crates/nargo_cli/tests/execution_success/trait_override_implementation/Nargo.toml diff --git a/crates/nargo_cli/tests/compile_success_empty/trait_override_implementation/Prover.toml b/crates/nargo_cli/tests/execution_success/trait_override_implementation/Prover.toml similarity index 100% rename from crates/nargo_cli/tests/compile_success_empty/trait_override_implementation/Prover.toml rename to crates/nargo_cli/tests/execution_success/trait_override_implementation/Prover.toml diff --git a/crates/nargo_cli/tests/compile_success_empty/trait_override_implementation/src/main.nr b/crates/nargo_cli/tests/execution_success/trait_override_implementation/src/main.nr similarity index 100% rename from crates/nargo_cli/tests/compile_success_empty/trait_override_implementation/src/main.nr rename to crates/nargo_cli/tests/execution_success/trait_override_implementation/src/main.nr diff --git a/crates/noirc_frontend/src/hir/def_collector/dc_crate.rs b/crates/noirc_frontend/src/hir/def_collector/dc_crate.rs index 57e5fb8197c..c55335e4443 100644 --- a/crates/noirc_frontend/src/hir/def_collector/dc_crate.rs +++ b/crates/noirc_frontend/src/hir/def_collector/dc_crate.rs @@ -26,6 +26,7 @@ use std::rc::Rc; use std::vec; /// Stores all of the unresolved functions in a particular file/mod +#[derive(Clone)] pub struct UnresolvedFunctions { pub file_id: FileId, pub functions: Vec<(LocalModuleId, FuncId, NoirFunction)>, @@ -43,12 +44,21 @@ pub struct UnresolvedStruct { pub struct_def: NoirStruct, } +#[derive(Clone)] pub struct UnresolvedTrait { pub file_id: FileId, pub module_id: LocalModuleId, pub trait_def: NoirTrait, } +pub struct UnresolvedTraitImpl { + pub file_id: FileId, + pub module_id: LocalModuleId, + pub the_trait: UnresolvedTrait, + pub methods: UnresolvedFunctions, + pub trait_impl_ident: Ident, // for error reporting +} + #[derive(Clone)] pub struct UnresolvedTypeAlias { pub file_id: FileId, @@ -74,7 +84,7 @@ pub struct DefCollector { pub(crate) collected_traits: BTreeMap, pub(crate) collected_globals: Vec, pub(crate) collected_impls: ImplMap, - pub(crate) collected_traits_impls: ImplMap, + pub(crate) collected_traits_impls: TraitImplMap, } /// Maps the type and the module id in which the impl is defined to the functions contained in that @@ -87,6 +97,8 @@ pub struct DefCollector { type ImplMap = HashMap<(UnresolvedType, LocalModuleId), Vec<(UnresolvedGenerics, Span, UnresolvedFunctions)>>; +type TraitImplMap = HashMap<(UnresolvedType, LocalModuleId, TraitId), UnresolvedTraitImpl>; + impl DefCollector { fn new(def_map: CrateDefMap) -> DefCollector { DefCollector { @@ -97,8 +109,8 @@ impl DefCollector { collected_type_aliases: BTreeMap::new(), collected_traits: BTreeMap::new(), collected_impls: HashMap::new(), - collected_traits_impls: HashMap::new(), collected_globals: vec![], + collected_traits_impls: HashMap::new(), } } @@ -205,7 +217,7 @@ impl DefCollector { // impl since that determines the module we should collect into. collect_impls(context, crate_id, &def_collector.collected_impls, errors); - collect_impls(context, crate_id, &def_collector.collected_traits_impls, errors); + collect_trait_impls(context, crate_id, &def_collector.collected_traits_impls, errors); // Lower each function in the crate. This is now possible since imports have been resolved let file_func_ids = resolve_free_functions( @@ -225,13 +237,8 @@ impl DefCollector { errors, ); - let file_trait_impls_ids = resolve_impls( - &mut context.def_interner, - crate_id, - &context.def_maps, - def_collector.collected_traits_impls, - errors, - ); + let file_trait_impls_ids = + resolve_trait_impls(context, def_collector.collected_traits_impls, crate_id, errors); type_check_globals(&mut context.def_interner, file_global_ids, errors); @@ -306,6 +313,58 @@ fn collect_impls( } } +fn collect_trait_impls( + context: &mut Context, + crate_id: CrateId, + collected_impls: &TraitImplMap, + errors: &mut Vec, +) { + let interner = &mut context.def_interner; + let def_maps = &mut context.def_maps; + + // TODO: To follow the semantics of Rust, we must allow the impl if either + // 1. The type is a struct and it's defined in the current crate + // 2. The trait is defined in the current crate + for ((unresolved_type, module_id, _), trait_impl) in collected_impls { + let path_resolver = + StandardPathResolver::new(ModuleId { local_id: *module_id, krate: crate_id }); + + for (_, func_id, ast) in &trait_impl.methods.functions { + let file = def_maps[&crate_id].file_id(*module_id); + + let mut resolver = Resolver::new(interner, &path_resolver, def_maps, file); + resolver.add_generics(&ast.def.generics); + let typ = resolver.resolve_type(unresolved_type.clone()); + + // Add the method to the struct's namespace + if let Some(struct_type) = get_struct_type(&typ) { + extend_errors(errors, trait_impl.file_id, resolver.take_errors()); + + let struct_type = struct_type.borrow(); + let type_module = struct_type.id.local_module_id(); + + let module = &mut def_maps.get_mut(&crate_id).unwrap().modules[type_module.0]; + + let result = module.declare_function(ast.name_ident().clone(), *func_id); + + if let Err((first_def, second_def)) = result { + let err = DefCollectorErrorKind::Duplicate { + typ: DuplicateType::Function, + first_def, + second_def, + }; + errors.push(err.into_file_diagnostic(trait_impl.file_id)); + } + } else { + let span = trait_impl.trait_impl_ident.span(); + let trait_ident = trait_impl.the_trait.trait_def.name.clone(); + let error = DefCollectorErrorKind::NonStructTraitImpl { trait_ident, span }; + errors.push(error.into_file_diagnostic(trait_impl.file_id)); + } + } + } +} + fn get_struct_type(typ: &Type) -> Option<&Shared> { match typ { Type::Struct(definition, _) => Some(definition), @@ -601,6 +660,55 @@ fn resolve_impls( file_method_ids } +fn resolve_trait_impls( + context: &mut Context, + traits: TraitImplMap, + crate_id: CrateId, + errors: &mut Vec, +) -> Vec<(FileId, FuncId)> { + let interner = &mut context.def_interner; + let mut methods = Vec::<(FileId, FuncId)>::new(); + + for ((unresolved_type, _, trait_id), trait_impl) in traits { + let local_mod_id = trait_impl.module_id; + let module_id = ModuleId { krate: crate_id, local_id: local_mod_id }; + let path_resolver = StandardPathResolver::new(module_id); + + let self_type = { + let mut resolver = + Resolver::new(interner, &path_resolver, &context.def_maps, trait_impl.file_id); + resolver.resolve_type(unresolved_type.clone()) + }; + + let mut impl_methods = resolve_function_set( + interner, + crate_id, + &context.def_maps, + trait_impl.methods.clone(), + Some(self_type.clone()), + vec![], // TODO + errors, + ); + + let trait_definition_ident = &trait_impl.trait_impl_ident; + let key = (self_type.clone(), trait_id); + if let Some(prev_trait_impl_ident) = interner.get_previous_trait_implementation(&key) { + let err = DefCollectorErrorKind::Duplicate { + typ: DuplicateType::TraitImplementation, + first_def: prev_trait_impl_ident.clone(), + second_def: trait_definition_ident.clone(), + }; + errors.push(err.into_file_diagnostic(trait_impl.methods.file_id)); + } else { + let _func_ids = + interner.add_trait_implementaion(&key, trait_definition_ident, &trait_impl.methods); + } + + methods.append(&mut impl_methods); + } + + methods +} fn resolve_free_functions( interner: &mut NodeInterner, crate_id: CrateId, diff --git a/crates/noirc_frontend/src/hir/def_collector/dc_mod.rs b/crates/noirc_frontend/src/hir/def_collector/dc_mod.rs index 809623623ee..0784fb467f8 100644 --- a/crates/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/crates/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -6,13 +6,15 @@ use crate::{ hir::def_collector::dc_crate::{UnresolvedStruct, UnresolvedTrait}, node_interner::TraitId, parser::SubModule, - FunctionDefinition, FunctionReturnType, Ident, LetStatement, NoirFunction, NoirStruct, + FunctionDefinition, Ident, LetStatement, NoirFunction, NoirStruct, NoirTrait, NoirTypeAlias, ParsedModule, TraitImpl, TraitImplItem, TraitItem, TypeImpl, - UnresolvedType, }; use super::{ - dc_crate::{DefCollector, UnresolvedFunctions, UnresolvedGlobal, UnresolvedTypeAlias}, + dc_crate::{ + DefCollector, UnresolvedFunctions, UnresolvedGlobal, UnresolvedTraitImpl, + UnresolvedTypeAlias, + }, errors::{DefCollectorErrorKind, DuplicateType}, }; use crate::hir::def_map::{parse_file, LocalModuleId, ModuleData, ModuleDefId, ModuleId}; @@ -71,87 +73,6 @@ pub fn collect_defs( collector.collect_impls(context, ast.impls); } -fn check_trait_method_implementation_parameters( - expected_parameters: &Vec<(Ident, UnresolvedType)>, - impl_method: &NoirFunction, - trait_name: &str, -) -> Result<(), DefCollectorErrorKind> { - let expected_num_parameters = expected_parameters.len(); - let actual_num_parameters = impl_method.def.parameters.len(); - if actual_num_parameters != expected_num_parameters { - return Err(DefCollectorErrorKind::MismatchTraitImplementationNumParameters { - actual_num_parameters, - expected_num_parameters, - trait_name: trait_name.to_owned(), - impl_ident: impl_method.name_ident().clone(), - }); - } - for (count, (parameter, typ, _abi_vis)) in impl_method.def.parameters.iter().enumerate() { - let (_expected_name, expected_type) = &expected_parameters[count]; - if typ.typ != expected_type.typ { - return Err(DefCollectorErrorKind::MismatchTraitImlementationParameter { - trait_name: trait_name.to_owned(), - expected_type: expected_type.clone(), - impl_method: impl_method.name().to_string(), - parameter: parameter.name_ident().clone(), - }); - } - } - Ok(()) -} - -fn check_trait_method_implementation_return_type( - expected_return_type: &FunctionReturnType, - impl_method: &NoirFunction, - trait_name: &str, -) -> Result<(), DefCollectorErrorKind> { - if expected_return_type.get_type() == impl_method.def.return_type.get_type() { - Ok(()) - } else { - Err(DefCollectorErrorKind::MismatchTraitImplementationReturnType { - trait_name: trait_name.to_owned(), - impl_ident: impl_method.name_ident().clone(), - }) - } -} - -fn check_trait_method_implementation( - r#trait: &NoirTrait, - impl_method: &NoirFunction, -) -> Result<(), DefCollectorErrorKind> { - for item in &r#trait.items { - if let TraitItem::Function { - name, - generics: _, - parameters, - return_type, - where_clause: _, - body: _, - } = item - { - if name.0.contents == impl_method.def.name.0.contents { - // name matches, check for parameters - count and type, return type - check_trait_method_implementation_parameters( - parameters, - impl_method, - &r#trait.name.0.contents, - )?; - check_trait_method_implementation_return_type( - return_type, - impl_method, - &r#trait.name.0.contents, - )?; - return Ok(()); - } - } - } - - Err(DefCollectorErrorKind::MethodNotInTrait { - trait_name: r#trait.name.clone(), - impl_method: impl_method.def.name.clone(), - }) -} - impl<'a> ModCollector<'a> { fn collect_globals( &mut self, @@ -217,23 +138,36 @@ impl<'a> ModCollector<'a> { match module.find_name(&trait_name).types { Some((module_def_id, _visibility)) => { if let Some(collected_trait) = self.get_unresolved_trait(module_def_id) { - let trait_def = collected_trait.trait_def.clone(); - let collected_implementations = self.collect_trait_implementations( + let unresolved_functions = self.collect_trait_implementations( context, &trait_impl, - &trait_def, + &collected_trait.trait_def, errors, ); - let impl_type_span = trait_impl.object_type_span; - let impl_generics = trait_impl.impl_generics.clone(); - let impl_object_type = trait_impl.object_type.clone(); - let key = (impl_object_type, self.module_id); - self.def_collector.collected_traits_impls.entry(key).or_default().push(( - impl_generics, - impl_type_span, - collected_implementations, - )); + for (_, func_id, noir_function) in &unresolved_functions.functions { + let name = noir_function.name().to_owned(); + + context.def_interner.push_function_definition(name, *func_id); + } + + let unresolved_trait_impl = UnresolvedTraitImpl { + file_id: self.file_id, + module_id: self.module_id, + the_trait: collected_trait, + methods: unresolved_functions, + trait_impl_ident: trait_impl.trait_name.clone(), + }; + + let trait_id = match module_def_id { + ModuleDefId::TraitId(trait_id) => trait_id, + _ => unreachable!(), + }; + + let key = (trait_impl.object_type, self.module_id, trait_id); + self.def_collector + .collected_traits_impls + .insert(key, unresolved_trait_impl); } else { let error = DefCollectorErrorKind::NotATrait { not_a_trait_name: trait_name.clone(), @@ -242,19 +176,18 @@ impl<'a> ModCollector<'a> { } } None => { - let error = DefCollectorErrorKind::TraitNotFound { - trait_name: trait_name.to_string(), - span: trait_name.span(), - }; + let error = DefCollectorErrorKind::TraitNotFound { trait_ident: trait_name }; errors.push(error.into_file_diagnostic(self.file_id)); } } } } - fn get_unresolved_trait(&self, module_def_id: ModuleDefId) -> Option<&UnresolvedTrait> { + fn get_unresolved_trait(&self, module_def_id: ModuleDefId) -> Option { match module_def_id { - ModuleDefId::TraitId(trait_id) => self.def_collector.collected_traits.get(&trait_id), + ModuleDefId::TraitId(trait_id) => { + self.def_collector.collected_traits.get(&trait_id).cloned() + } _ => None, } } @@ -271,18 +204,11 @@ impl<'a> ModCollector<'a> { for item in &trait_impl.items { if let TraitImplItem::Function(impl_method) = item { - match check_trait_method_implementation(trait_def, impl_method) { - Ok(()) => { - let func_id = context.def_interner.push_empty_fn(); - context - .def_interner - .push_function_definition(impl_method.name().to_owned(), func_id); - unresolved_functions.push_fn(self.module_id, func_id, impl_method.clone()); - } - Err(error) => { - errors.push(error.into_file_diagnostic(self.file_id)); - } - } + let func_id = context.def_interner.push_empty_fn(); + context + .def_interner + .push_function_definition(impl_method.name().to_owned(), func_id); + unresolved_functions.push_fn(self.module_id, func_id, impl_method.clone()); } } diff --git a/crates/noirc_frontend/src/hir/def_collector/errors.rs b/crates/noirc_frontend/src/hir/def_collector/errors.rs index 2aab7f514d9..9be63533bb5 100644 --- a/crates/noirc_frontend/src/hir/def_collector/errors.rs +++ b/crates/noirc_frontend/src/hir/def_collector/errors.rs @@ -17,6 +17,7 @@ pub enum DuplicateType { TypeDefinition, Import, Trait, + TraitImplementation, } #[derive(Error, Debug)] @@ -29,6 +30,8 @@ pub enum DefCollectorErrorKind { PathResolutionError(PathResolutionError), #[error("Non-struct type used in impl")] NonStructTypeInImpl { span: Span }, + #[error("Non-struct type used in trait impl")] + NonStructTraitImpl { trait_ident: Ident, span: Span }, #[error("Cannot `impl` a type defined outside the current crate")] ForeignImpl { span: Span, type_name: String }, #[error("Mismatch signature of trait")] @@ -52,7 +55,7 @@ pub enum DefCollectorErrorKind { #[error("Only traits can be implemented")] NotATrait { not_a_trait_name: Ident }, #[error("Trait not found")] - TraitNotFound { trait_name: String, span: Span }, + TraitNotFound { trait_ident: Ident }, #[error("Missing Trait method implementation")] TraitMissedMethodImplementation { trait_name: Ident, method_name: Ident, trait_impl_span: Span }, } @@ -71,6 +74,7 @@ impl fmt::Display for DuplicateType { DuplicateType::Global => write!(f, "global"), DuplicateType::TypeDefinition => write!(f, "type definition"), DuplicateType::Trait => write!(f, "trait definition"), + DuplicateType::TraitImplementation => write!(f, "trait implementation"), DuplicateType::Import => write!(f, "import"), } } @@ -112,15 +116,22 @@ impl From for Diagnostic { "Only struct types may have implementation methods".into(), span, ), + DefCollectorErrorKind::NonStructTraitImpl { trait_ident, span } => { + Diagnostic::simple_error( + format!("Only struct types may implement trait `{}`", trait_ident), + "Only struct types may implement traits".into(), + span, + ) + } DefCollectorErrorKind::ForeignImpl { span, type_name } => Diagnostic::simple_error( "Cannot `impl` a type that was defined outside the current crate".into(), format!("{type_name} was defined outside the current crate"), span, ), - DefCollectorErrorKind::TraitNotFound { trait_name, span } => Diagnostic::simple_error( - format!("Trait {} not found", trait_name), + DefCollectorErrorKind::TraitNotFound { trait_ident } => Diagnostic::simple_error( + format!("Trait {} not found", trait_ident), "".to_string(), - span, + trait_ident.span(), ), DefCollectorErrorKind::MismatchTraitImplementationReturnType { trait_name, diff --git a/crates/noirc_frontend/src/node_interner.rs b/crates/noirc_frontend/src/node_interner.rs index 24015d76a07..5e4604d7bdc 100644 --- a/crates/noirc_frontend/src/node_interner.rs +++ b/crates/noirc_frontend/src/node_interner.rs @@ -7,7 +7,9 @@ use noirc_errors::{Location, Span, Spanned}; use crate::ast::Ident; use crate::graph::CrateId; -use crate::hir::def_collector::dc_crate::{UnresolvedStruct, UnresolvedTrait, UnresolvedTypeAlias}; +use crate::hir::def_collector::dc_crate::{ + UnresolvedFunctions, UnresolvedStruct, UnresolvedTrait, UnresolvedTypeAlias, +}; use crate::hir::def_map::{LocalModuleId, ModuleId}; use crate::hir::StorageSlot; use crate::hir_def::stmt::HirLetStatement; @@ -71,6 +73,11 @@ pub struct NodeInterner { // We'd just lookup their methods as needed through the NodeInterner. traits: HashMap>, + // Trait implementation map + // For each type that implements a given Trait ( corresponding TraitId), there should be an entry here + // The purpose for this hashmap is to detect duplication of trait implementations ( if any ) + trait_implementaions: HashMap<(Type, TraitId), Ident>, + /// Map from ExprId (referring to a Function/Method call) to its corresponding TypeBindings, /// filled out during type checking from instantiated variables. Used during monomorphization /// to map call site types back onto function parameter types, and undo this binding as needed. @@ -297,6 +304,7 @@ impl Default for NodeInterner { structs: HashMap::new(), type_aliases: Vec::new(), traits: HashMap::new(), + trait_implementaions: HashMap::new(), instantiation_bindings: HashMap::new(), field_indices: HashMap::new(), next_type_variable_id: 0, @@ -703,6 +711,26 @@ impl NodeInterner { } } + pub fn get_previous_trait_implementation(&self, key: &(Type, TraitId)) -> Option<&Ident> { + self.trait_implementaions.get(key) + } + + pub fn add_trait_implementaion( + &mut self, + key: &(Type, TraitId), + trait_definition_ident: &Ident, + methods: &UnresolvedFunctions, + ) -> Vec { + self.trait_implementaions.insert(key.clone(), trait_definition_ident.clone()); + methods + .functions + .iter() + .flat_map(|(_, func_id, _)| { + self.add_method(&key.0, self.function_name(func_id).to_owned(), *func_id) + }) + .collect::>() + } + /// Search by name for a method on the given struct pub fn lookup_method(&self, id: StructId, method_name: &str) -> Option { self.struct_methods.get(&(id, method_name.to_owned())).copied()