From 4a317d86f066b849b69d4e05dc0e3476ab55211c Mon Sep 17 00:00:00 2001 From: Nikolay Nikolov Date: Thu, 5 Oct 2023 21:16:41 +0300 Subject: [PATCH] feat(traits): added checks for duplicated trait associated items (types, consts, functions) (#2927) --- .../src/hir/def_collector/dc_crate.rs | 10 +-- .../src/hir/def_collector/dc_mod.rs | 86 +++++++++++++++---- .../src/hir/def_collector/errors.rs | 6 ++ .../dup_trait_items_1/Nargo.toml | 7 ++ .../dup_trait_items_1/Prover.toml | 0 .../dup_trait_items_1/src/main.nr | 7 ++ .../dup_trait_items_2/Nargo.toml | 7 ++ .../dup_trait_items_2/Prover.toml | 0 .../dup_trait_items_2/src/main.nr | 7 ++ .../dup_trait_items_3/Nargo.toml | 7 ++ .../dup_trait_items_3/Prover.toml | 0 .../dup_trait_items_3/src/main.nr | 7 ++ .../dup_trait_items_4/Nargo.toml | 7 ++ .../dup_trait_items_4/Prover.toml | 0 .../dup_trait_items_4/src/main.nr | 7 ++ .../dup_trait_items_5/Nargo.toml | 7 ++ .../dup_trait_items_5/Prover.toml | 0 .../dup_trait_items_5/src/main.nr | 7 ++ .../dup_trait_items_6/Nargo.toml | 7 ++ .../dup_trait_items_6/Prover.toml | 0 .../dup_trait_items_6/src/main.nr | 15 ++++ .../Nargo.toml | 7 ++ .../Prover.toml | 0 .../src/main.nr | 26 ++++++ 24 files changed, 202 insertions(+), 25 deletions(-) create mode 100644 tooling/nargo_cli/tests/compile_failure/dup_trait_items_1/Nargo.toml create mode 100644 tooling/nargo_cli/tests/compile_failure/dup_trait_items_1/Prover.toml create mode 100644 tooling/nargo_cli/tests/compile_failure/dup_trait_items_1/src/main.nr create mode 100644 tooling/nargo_cli/tests/compile_failure/dup_trait_items_2/Nargo.toml create mode 100644 tooling/nargo_cli/tests/compile_failure/dup_trait_items_2/Prover.toml create mode 100644 tooling/nargo_cli/tests/compile_failure/dup_trait_items_2/src/main.nr create mode 100644 tooling/nargo_cli/tests/compile_failure/dup_trait_items_3/Nargo.toml create mode 100644 tooling/nargo_cli/tests/compile_failure/dup_trait_items_3/Prover.toml create mode 100644 tooling/nargo_cli/tests/compile_failure/dup_trait_items_3/src/main.nr create mode 100644 tooling/nargo_cli/tests/compile_failure/dup_trait_items_4/Nargo.toml create mode 100644 tooling/nargo_cli/tests/compile_failure/dup_trait_items_4/Prover.toml create mode 100644 tooling/nargo_cli/tests/compile_failure/dup_trait_items_4/src/main.nr create mode 100644 tooling/nargo_cli/tests/compile_failure/dup_trait_items_5/Nargo.toml create mode 100644 tooling/nargo_cli/tests/compile_failure/dup_trait_items_5/Prover.toml create mode 100644 tooling/nargo_cli/tests/compile_failure/dup_trait_items_5/src/main.nr create mode 100644 tooling/nargo_cli/tests/compile_failure/dup_trait_items_6/Nargo.toml create mode 100644 tooling/nargo_cli/tests/compile_failure/dup_trait_items_6/Prover.toml create mode 100644 tooling/nargo_cli/tests/compile_failure/dup_trait_items_6/src/main.nr create mode 100644 tooling/nargo_cli/tests/execution_success/trait_allowed_item_name_matches/Nargo.toml create mode 100644 tooling/nargo_cli/tests/execution_success/trait_allowed_item_name_matches/Prover.toml create mode 100644 tooling/nargo_cli/tests/execution_success/trait_allowed_item_name_matches/src/main.nr 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 fb1ef0adc5d..9c870240371 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs @@ -468,7 +468,7 @@ fn collect_trait_impl_methods( if overrides.len() > 1 { let error = DefCollectorErrorKind::Duplicate { - typ: DuplicateType::Function, + typ: DuplicateType::TraitAssociatedFunction, first_def: overrides[0].2.name_ident().clone(), second_def: overrides[1].2.name_ident().clone(), }; @@ -542,7 +542,7 @@ fn collect_trait_impl( if let Some(struct_type) = get_struct_type(&typ) { errors.extend(take_errors(trait_impl.file_id, resolver)); - let current_def_map = def_maps.get_mut(&crate_id).unwrap(); + let current_def_map = def_maps.get_mut(&struct_type.borrow().id.krate()).unwrap(); match add_method_to_struct_namespace( current_def_map, struct_type, @@ -801,11 +801,7 @@ fn resolve_trait_methods( .iter() .filter(|(_, _, q)| q.name() == name.0.contents) .collect(); - let default_impl = if !default_impl_list.is_empty() { - if default_impl_list.len() > 1 { - // TODO(nickysn): Add check for method duplicates in the trait and emit proper error messages. This is planned in a future PR. - panic!("Too many functions with the same name!"); - } + let default_impl = if default_impl_list.len() == 1 { Some(Box::new(default_impl_list[0].2.clone())) } else { None diff --git a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs index 92c2e0fcebc..8386101520f 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -6,7 +6,7 @@ use noirc_errors::Location; use crate::{ graph::CrateId, hir::def_collector::dc_crate::{UnresolvedStruct, UnresolvedTrait}, - node_interner::TraitId, + node_interner::{TraitId, TypeAliasId}, parser::{SortedModule, SubModule}, FunctionDefinition, Ident, LetStatement, NoirFunction, NoirStruct, NoirTrait, NoirTraitImpl, NoirTypeAlias, TraitImplItem, TraitItem, TypeImpl, @@ -360,27 +360,77 @@ impl<'a> ModCollector<'a> { trait_id: None, }; for trait_item in &trait_definition.items { - // TODO(Maddiaa): Investigate trait implementations with attributes see: https://github.com/noir-lang/noir/issues/2629 - if let TraitItem::Function { - name, - generics, - parameters, - return_type, - where_clause, - body: Some(body), - } = trait_item - { - let func_id = context.def_interner.push_empty_fn(); - - let impl_method = NoirFunction::normal(FunctionDefinition::normal( + match trait_item { + TraitItem::Function { name, generics, parameters, - body, - where_clause, return_type, - )); - unresolved_functions.push_fn(self.module_id, func_id, impl_method); + where_clause, + body, + } => { + let func_id = context.def_interner.push_empty_fn(); + match self.def_collector.def_map.modules[id.0.local_id.0] + .declare_function(name.clone(), func_id) + { + Ok(()) => { + // TODO(Maddiaa): Investigate trait implementations with attributes see: https://github.com/noir-lang/noir/issues/2629 + if let Some(body) = body { + let impl_method = + NoirFunction::normal(FunctionDefinition::normal( + name, + generics, + parameters, + body, + where_clause, + return_type, + )); + unresolved_functions.push_fn( + self.module_id, + func_id, + impl_method, + ); + } + } + Err((first_def, second_def)) => { + let error = DefCollectorErrorKind::Duplicate { + typ: DuplicateType::TraitAssociatedFunction, + first_def, + second_def, + }; + errors.push((error.into(), self.file_id)); + } + } + } + TraitItem::Constant { name, .. } => { + let stmt_id = context.def_interner.push_empty_global(); + + if let Err((first_def, second_def)) = self.def_collector.def_map.modules + [id.0.local_id.0] + .declare_global(name.clone(), stmt_id) + { + let error = DefCollectorErrorKind::Duplicate { + typ: DuplicateType::TraitAssociatedConst, + first_def, + second_def, + }; + errors.push((error.into(), self.file_id)); + } + } + TraitItem::Type { name } => { + // TODO(nickysn or alexvitkov): implement context.def_interner.push_empty_type_alias and get an id, instead of using TypeAliasId::dummy_id() + if let Err((first_def, second_def)) = self.def_collector.def_map.modules + [id.0.local_id.0] + .declare_type_alias(name.clone(), TypeAliasId::dummy_id()) + { + let error = DefCollectorErrorKind::Duplicate { + typ: DuplicateType::TraitAssociatedType, + first_def, + second_def, + }; + errors.push((error.into(), self.file_id)); + } + } } } diff --git a/compiler/noirc_frontend/src/hir/def_collector/errors.rs b/compiler/noirc_frontend/src/hir/def_collector/errors.rs index f959cdec598..cda9fea6d08 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/errors.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/errors.rs @@ -18,6 +18,9 @@ pub enum DuplicateType { Import, Trait, TraitImplementation, + TraitAssociatedType, + TraitAssociatedConst, + TraitAssociatedFunction, } #[derive(Error, Debug, Clone)] @@ -79,6 +82,9 @@ impl fmt::Display for DuplicateType { DuplicateType::Trait => write!(f, "trait definition"), DuplicateType::TraitImplementation => write!(f, "trait implementation"), DuplicateType::Import => write!(f, "import"), + DuplicateType::TraitAssociatedType => write!(f, "trait associated type"), + DuplicateType::TraitAssociatedConst => write!(f, "trait associated constant"), + DuplicateType::TraitAssociatedFunction => write!(f, "trait associated function"), } } } diff --git a/tooling/nargo_cli/tests/compile_failure/dup_trait_items_1/Nargo.toml b/tooling/nargo_cli/tests/compile_failure/dup_trait_items_1/Nargo.toml new file mode 100644 index 00000000000..d8b44af47c2 --- /dev/null +++ b/tooling/nargo_cli/tests/compile_failure/dup_trait_items_1/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "dup_trait_items_1" +type = "bin" +authors = [""] +compiler_version = "0.15.0" + +[dependencies] \ No newline at end of file diff --git a/tooling/nargo_cli/tests/compile_failure/dup_trait_items_1/Prover.toml b/tooling/nargo_cli/tests/compile_failure/dup_trait_items_1/Prover.toml new file mode 100644 index 00000000000..e69de29bb2d diff --git a/tooling/nargo_cli/tests/compile_failure/dup_trait_items_1/src/main.nr b/tooling/nargo_cli/tests/compile_failure/dup_trait_items_1/src/main.nr new file mode 100644 index 00000000000..9055d6e3998 --- /dev/null +++ b/tooling/nargo_cli/tests/compile_failure/dup_trait_items_1/src/main.nr @@ -0,0 +1,7 @@ +trait MyTrait { + fn SomeFunc(); + fn SomeFunc(); +} + +fn main() { +} diff --git a/tooling/nargo_cli/tests/compile_failure/dup_trait_items_2/Nargo.toml b/tooling/nargo_cli/tests/compile_failure/dup_trait_items_2/Nargo.toml new file mode 100644 index 00000000000..b37256a1292 --- /dev/null +++ b/tooling/nargo_cli/tests/compile_failure/dup_trait_items_2/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "dup_trait_items_2" +type = "bin" +authors = [""] +compiler_version = "0.15.0" + +[dependencies] \ No newline at end of file diff --git a/tooling/nargo_cli/tests/compile_failure/dup_trait_items_2/Prover.toml b/tooling/nargo_cli/tests/compile_failure/dup_trait_items_2/Prover.toml new file mode 100644 index 00000000000..e69de29bb2d diff --git a/tooling/nargo_cli/tests/compile_failure/dup_trait_items_2/src/main.nr b/tooling/nargo_cli/tests/compile_failure/dup_trait_items_2/src/main.nr new file mode 100644 index 00000000000..312b1e5a9d4 --- /dev/null +++ b/tooling/nargo_cli/tests/compile_failure/dup_trait_items_2/src/main.nr @@ -0,0 +1,7 @@ +trait MyTrait { + let SomeConst: u32; + let SomeConst: Field; +} + +fn main() { +} diff --git a/tooling/nargo_cli/tests/compile_failure/dup_trait_items_3/Nargo.toml b/tooling/nargo_cli/tests/compile_failure/dup_trait_items_3/Nargo.toml new file mode 100644 index 00000000000..c9a0de11174 --- /dev/null +++ b/tooling/nargo_cli/tests/compile_failure/dup_trait_items_3/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "dup_trait_items_3" +type = "bin" +authors = [""] +compiler_version = "0.15.0" + +[dependencies] \ No newline at end of file diff --git a/tooling/nargo_cli/tests/compile_failure/dup_trait_items_3/Prover.toml b/tooling/nargo_cli/tests/compile_failure/dup_trait_items_3/Prover.toml new file mode 100644 index 00000000000..e69de29bb2d diff --git a/tooling/nargo_cli/tests/compile_failure/dup_trait_items_3/src/main.nr b/tooling/nargo_cli/tests/compile_failure/dup_trait_items_3/src/main.nr new file mode 100644 index 00000000000..ca97a9a143d --- /dev/null +++ b/tooling/nargo_cli/tests/compile_failure/dup_trait_items_3/src/main.nr @@ -0,0 +1,7 @@ +trait MyTrait { + type SomeType; + type SomeType; +} + +fn main() { +} diff --git a/tooling/nargo_cli/tests/compile_failure/dup_trait_items_4/Nargo.toml b/tooling/nargo_cli/tests/compile_failure/dup_trait_items_4/Nargo.toml new file mode 100644 index 00000000000..5e4af3a29ae --- /dev/null +++ b/tooling/nargo_cli/tests/compile_failure/dup_trait_items_4/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "dup_trait_items_4" +type = "bin" +authors = [""] +compiler_version = "0.15.0" + +[dependencies] \ No newline at end of file diff --git a/tooling/nargo_cli/tests/compile_failure/dup_trait_items_4/Prover.toml b/tooling/nargo_cli/tests/compile_failure/dup_trait_items_4/Prover.toml new file mode 100644 index 00000000000..e69de29bb2d diff --git a/tooling/nargo_cli/tests/compile_failure/dup_trait_items_4/src/main.nr b/tooling/nargo_cli/tests/compile_failure/dup_trait_items_4/src/main.nr new file mode 100644 index 00000000000..da0532e39c1 --- /dev/null +++ b/tooling/nargo_cli/tests/compile_failure/dup_trait_items_4/src/main.nr @@ -0,0 +1,7 @@ +trait MyTrait { + let MyItem: u32; + fn MyItem(); +} + +fn main() { +} diff --git a/tooling/nargo_cli/tests/compile_failure/dup_trait_items_5/Nargo.toml b/tooling/nargo_cli/tests/compile_failure/dup_trait_items_5/Nargo.toml new file mode 100644 index 00000000000..2d8c9aad7ec --- /dev/null +++ b/tooling/nargo_cli/tests/compile_failure/dup_trait_items_5/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "dup_trait_items_5" +type = "bin" +authors = [""] +compiler_version = "0.15.0" + +[dependencies] \ No newline at end of file diff --git a/tooling/nargo_cli/tests/compile_failure/dup_trait_items_5/Prover.toml b/tooling/nargo_cli/tests/compile_failure/dup_trait_items_5/Prover.toml new file mode 100644 index 00000000000..e69de29bb2d diff --git a/tooling/nargo_cli/tests/compile_failure/dup_trait_items_5/src/main.nr b/tooling/nargo_cli/tests/compile_failure/dup_trait_items_5/src/main.nr new file mode 100644 index 00000000000..4881a338a84 --- /dev/null +++ b/tooling/nargo_cli/tests/compile_failure/dup_trait_items_5/src/main.nr @@ -0,0 +1,7 @@ +trait MyTrait { + fn MyItem(); + let MyItem: u32; +} + +fn main() { +} diff --git a/tooling/nargo_cli/tests/compile_failure/dup_trait_items_6/Nargo.toml b/tooling/nargo_cli/tests/compile_failure/dup_trait_items_6/Nargo.toml new file mode 100644 index 00000000000..5107c10b41d --- /dev/null +++ b/tooling/nargo_cli/tests/compile_failure/dup_trait_items_6/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "dup_trait_items_6" +type = "bin" +authors = [""] +compiler_version = "0.15.0" + +[dependencies] \ No newline at end of file diff --git a/tooling/nargo_cli/tests/compile_failure/dup_trait_items_6/Prover.toml b/tooling/nargo_cli/tests/compile_failure/dup_trait_items_6/Prover.toml new file mode 100644 index 00000000000..e69de29bb2d diff --git a/tooling/nargo_cli/tests/compile_failure/dup_trait_items_6/src/main.nr b/tooling/nargo_cli/tests/compile_failure/dup_trait_items_6/src/main.nr new file mode 100644 index 00000000000..a1a731d943b --- /dev/null +++ b/tooling/nargo_cli/tests/compile_failure/dup_trait_items_6/src/main.nr @@ -0,0 +1,15 @@ +trait MyTrait { + fn SomeFunc() { }; + fn SomeFunc() { }; +} + +struct MyStruct { +} + +impl MyTrait for MyStruct { + fn SomeFunc() { + } +} + +fn main() { +} diff --git a/tooling/nargo_cli/tests/execution_success/trait_allowed_item_name_matches/Nargo.toml b/tooling/nargo_cli/tests/execution_success/trait_allowed_item_name_matches/Nargo.toml new file mode 100644 index 00000000000..459cf96bab7 --- /dev/null +++ b/tooling/nargo_cli/tests/execution_success/trait_allowed_item_name_matches/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "trait_allowed_item_name_matches" +type = "bin" +authors = [""] +compiler_version = "0.15.0" + +[dependencies] \ No newline at end of file diff --git a/tooling/nargo_cli/tests/execution_success/trait_allowed_item_name_matches/Prover.toml b/tooling/nargo_cli/tests/execution_success/trait_allowed_item_name_matches/Prover.toml new file mode 100644 index 00000000000..e69de29bb2d diff --git a/tooling/nargo_cli/tests/execution_success/trait_allowed_item_name_matches/src/main.nr b/tooling/nargo_cli/tests/execution_success/trait_allowed_item_name_matches/src/main.nr new file mode 100644 index 00000000000..7db61e854fc --- /dev/null +++ b/tooling/nargo_cli/tests/execution_success/trait_allowed_item_name_matches/src/main.nr @@ -0,0 +1,26 @@ +trait Trait1 { + // types and consts with the same name are allowed + type Tralala; + let Tralala: u32; +} + +trait Trait2 { + // consts and types with the same name are allowed + let Tralala: u32; + type Tralala; +} + +trait Trait3 { + // types and functions with the same name are allowed + type Tralala; + fn Tralala(); +} + +trait Trait4 { + // functions and types with the same name are allowed + fn Tralala(); + type Tralala; +} + +fn main() { +}