From 9b818cacc64837a571c48d80c244a53253fe69a1 Mon Sep 17 00:00:00 2001 From: Yordan Madzhunkov Date: Tue, 29 Aug 2023 19:50:24 +0300 Subject: [PATCH 01/32] Fix tests by removing trait implemetation checks from def_collector --- .../src/hir/def_collector/dc_mod.rs | 103 ++---------------- 1 file changed, 7 insertions(+), 96 deletions(-) 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..c42867fe91c 100644 --- a/crates/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/crates/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -6,9 +6,8 @@ use crate::{ hir::def_collector::dc_crate::{UnresolvedStruct, UnresolvedTrait}, node_interner::TraitId, parser::SubModule, - FunctionDefinition, FunctionReturnType, Ident, LetStatement, NoirFunction, NoirStruct, - NoirTrait, NoirTypeAlias, ParsedModule, TraitImpl, TraitImplItem, TraitItem, TypeImpl, - UnresolvedType, + FunctionDefinition, Ident, LetStatement, NoirFunction, NoirStruct, NoirTrait, NoirTypeAlias, + ParsedModule, TraitImpl, TraitImplItem, TraitItem, TypeImpl, }; use super::{ @@ -71,87 +70,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, @@ -271,18 +189,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()); } } From 152016052817cd70eff835643c7422fbc0968dbb Mon Sep 17 00:00:00 2001 From: Nikolay Nikolov Date: Wed, 30 Aug 2023 03:19:00 +0300 Subject: [PATCH 02/32] fix(tests): simplify the trait-related compile_failure tests Because compile_failure tests are only checked for producing any compiler error, without checking whether the compiler error is what we expect, it is best if we keep these tests as simple as possible, so that they are only likely to fail for a single reason. --- .../tests/compile_failure/dup_trait_declaration/src/main.nr | 2 -- .../tests/compile_failure/dup_trait_implementation/src/main.nr | 2 -- .../tests/compile_failure/impl_struct_not_trait/src/main.nr | 2 -- .../compile_failure/trait_missing_implementation/src/main.nr | 2 -- .../tests/compile_failure/trait_not_in_scope/src/main.nr | 2 -- .../tests/compile_failure/trait_wrong_method_name/src/main.nr | 2 -- .../compile_failure/trait_wrong_method_return_type/src/main.nr | 2 -- .../tests/compile_failure/trait_wrong_parameter/src/main.nr | 2 -- .../compile_failure/trait_wrong_parameters_count/src/main.nr | 2 -- 9 files changed, 18 deletions(-) diff --git a/crates/nargo_cli/tests/compile_failure/dup_trait_declaration/src/main.nr b/crates/nargo_cli/tests/compile_failure/dup_trait_declaration/src/main.nr index f4c246c786a..052d7762438 100644 --- a/crates/nargo_cli/tests/compile_failure/dup_trait_declaration/src/main.nr +++ b/crates/nargo_cli/tests/compile_failure/dup_trait_declaration/src/main.nr @@ -21,6 +21,4 @@ trait Default { } 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/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/impl_struct_not_trait/src/main.nr b/crates/nargo_cli/tests/compile_failure/impl_struct_not_trait/src/main.nr index e25465378b1..50c142e2f5e 100644 --- a/crates/nargo_cli/tests/compile_failure/impl_struct_not_trait/src/main.nr +++ b/crates/nargo_cli/tests/compile_failure/impl_struct_not_trait/src/main.nr @@ -18,6 +18,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/trait_missing_implementation/src/main.nr b/crates/nargo_cli/tests/compile_failure/trait_missing_implementation/src/main.nr index bc74b328592..1f69d09924b 100644 --- a/crates/nargo_cli/tests/compile_failure/trait_missing_implementation/src/main.nr +++ b/crates/nargo_cli/tests/compile_failure/trait_missing_implementation/src/main.nr @@ -19,6 +19,4 @@ impl Default for Foo { } fn main(x: Field) { - let first = Foo::method2(x); - assert(first == x); } diff --git a/crates/nargo_cli/tests/compile_failure/trait_not_in_scope/src/main.nr b/crates/nargo_cli/tests/compile_failure/trait_not_in_scope/src/main.nr index 9dc57ee395f..2f236e622f0 100644 --- a/crates/nargo_cli/tests/compile_failure/trait_not_in_scope/src/main.nr +++ b/crates/nargo_cli/tests/compile_failure/trait_not_in_scope/src/main.nr @@ -13,6 +13,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/trait_wrong_method_name/src/main.nr b/crates/nargo_cli/tests/compile_failure/trait_wrong_method_name/src/main.nr index 0ba10815efa..dab18516aae 100644 --- a/crates/nargo_cli/tests/compile_failure/trait_wrong_method_name/src/main.nr +++ b/crates/nargo_cli/tests/compile_failure/trait_wrong_method_name/src/main.nr @@ -17,6 +17,4 @@ impl Default for Foo { } fn main(x: Field, y: Field) { - let first = Foo::default_wrong_name(x,y); - assert(first.bar == x); } diff --git a/crates/nargo_cli/tests/compile_failure/trait_wrong_method_return_type/src/main.nr b/crates/nargo_cli/tests/compile_failure/trait_wrong_method_return_type/src/main.nr index acd930a6d49..5dbb165cabe 100644 --- a/crates/nargo_cli/tests/compile_failure/trait_wrong_method_return_type/src/main.nr +++ b/crates/nargo_cli/tests/compile_failure/trait_wrong_method_return_type/src/main.nr @@ -16,6 +16,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/trait_wrong_parameter/src/main.nr b/crates/nargo_cli/tests/compile_failure/trait_wrong_parameter/src/main.nr index 2975aa6b1dd..266f3aaf769 100644 --- a/crates/nargo_cli/tests/compile_failure/trait_wrong_parameter/src/main.nr +++ b/crates/nargo_cli/tests/compile_failure/trait_wrong_parameter/src/main.nr @@ -16,6 +16,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/trait_wrong_parameters_count/src/main.nr b/crates/nargo_cli/tests/compile_failure/trait_wrong_parameters_count/src/main.nr index 92469ae8fdb..4d011ddf737 100644 --- a/crates/nargo_cli/tests/compile_failure/trait_wrong_parameters_count/src/main.nr +++ b/crates/nargo_cli/tests/compile_failure/trait_wrong_parameters_count/src/main.nr @@ -16,6 +16,4 @@ impl Default for Foo { } fn main(x: Field, y: Field) { - let first = Foo::default(x,y); - assert(first.bar == x); } From d8d06cd44cb1e8f91c44e7040b3448ec97e87bc6 Mon Sep 17 00:00:00 2001 From: Nikolay Nikolov Date: Wed, 30 Aug 2023 03:41:03 +0300 Subject: [PATCH 03/32] chore(tests): move tests that fail for a known reason to a subdirectory, where they're not executed Tests that were added too early and fail for a known reason (not yet implemented functionality in the compiler) are temporarily moved to the KNOWN_FAILURES directory, so they're not executed by 'cargo test'. This is due to the fact that the trait type checking was done way too early (in the def collector). After realizing this, we had to remove this code, so we can implement it at a later stage. However, the tests we added now become a chore, because they now fail for a known reason - in the long run, we don't want to remove them, because they are valid tests and should eventually pass. However, they hinder compiler development, because there are several other steps that need to be implemented in the compiler, before we can make them pass again. On the other hand, we want to be able to run the other tests, and the output of 'cargo test' becomes ugly, when there are failing tests. So, basically we want to postpone these tests for later, but not forget about them. That's why we move them temporarily to a KNOWN_FAILURES directory. --- .../compile_failure/trait_wrong_method_return_type/Nargo.toml | 0 .../compile_failure/trait_wrong_method_return_type/Prover.toml | 0 .../compile_failure/trait_wrong_method_return_type/src/main.nr | 0 .../compile_failure/trait_wrong_parameter/Nargo.toml | 0 .../compile_failure/trait_wrong_parameter/Prover.toml | 0 .../compile_failure/trait_wrong_parameter/src/main.nr | 0 .../compile_failure/trait_wrong_parameters_count/Nargo.toml | 0 .../compile_failure/trait_wrong_parameters_count/Prover.toml | 0 .../compile_failure/trait_wrong_parameters_count/src/main.nr | 0 9 files changed, 0 insertions(+), 0 deletions(-) rename crates/nargo_cli/tests/{ => KNOWN_FAILURES}/compile_failure/trait_wrong_method_return_type/Nargo.toml (100%) rename crates/nargo_cli/tests/{ => KNOWN_FAILURES}/compile_failure/trait_wrong_method_return_type/Prover.toml (100%) rename crates/nargo_cli/tests/{ => KNOWN_FAILURES}/compile_failure/trait_wrong_method_return_type/src/main.nr (100%) rename crates/nargo_cli/tests/{ => KNOWN_FAILURES}/compile_failure/trait_wrong_parameter/Nargo.toml (100%) rename crates/nargo_cli/tests/{ => KNOWN_FAILURES}/compile_failure/trait_wrong_parameter/Prover.toml (100%) rename crates/nargo_cli/tests/{ => KNOWN_FAILURES}/compile_failure/trait_wrong_parameter/src/main.nr (100%) rename crates/nargo_cli/tests/{ => KNOWN_FAILURES}/compile_failure/trait_wrong_parameters_count/Nargo.toml (100%) rename crates/nargo_cli/tests/{ => KNOWN_FAILURES}/compile_failure/trait_wrong_parameters_count/Prover.toml (100%) rename crates/nargo_cli/tests/{ => KNOWN_FAILURES}/compile_failure/trait_wrong_parameters_count/src/main.nr (100%) diff --git a/crates/nargo_cli/tests/compile_failure/trait_wrong_method_return_type/Nargo.toml b/crates/nargo_cli/tests/KNOWN_FAILURES/compile_failure/trait_wrong_method_return_type/Nargo.toml similarity index 100% rename from crates/nargo_cli/tests/compile_failure/trait_wrong_method_return_type/Nargo.toml rename to crates/nargo_cli/tests/KNOWN_FAILURES/compile_failure/trait_wrong_method_return_type/Nargo.toml diff --git a/crates/nargo_cli/tests/compile_failure/trait_wrong_method_return_type/Prover.toml b/crates/nargo_cli/tests/KNOWN_FAILURES/compile_failure/trait_wrong_method_return_type/Prover.toml similarity index 100% rename from crates/nargo_cli/tests/compile_failure/trait_wrong_method_return_type/Prover.toml rename to crates/nargo_cli/tests/KNOWN_FAILURES/compile_failure/trait_wrong_method_return_type/Prover.toml diff --git a/crates/nargo_cli/tests/compile_failure/trait_wrong_method_return_type/src/main.nr b/crates/nargo_cli/tests/KNOWN_FAILURES/compile_failure/trait_wrong_method_return_type/src/main.nr similarity index 100% rename from crates/nargo_cli/tests/compile_failure/trait_wrong_method_return_type/src/main.nr rename to crates/nargo_cli/tests/KNOWN_FAILURES/compile_failure/trait_wrong_method_return_type/src/main.nr diff --git a/crates/nargo_cli/tests/compile_failure/trait_wrong_parameter/Nargo.toml b/crates/nargo_cli/tests/KNOWN_FAILURES/compile_failure/trait_wrong_parameter/Nargo.toml similarity index 100% rename from crates/nargo_cli/tests/compile_failure/trait_wrong_parameter/Nargo.toml rename to crates/nargo_cli/tests/KNOWN_FAILURES/compile_failure/trait_wrong_parameter/Nargo.toml diff --git a/crates/nargo_cli/tests/compile_failure/trait_wrong_parameter/Prover.toml b/crates/nargo_cli/tests/KNOWN_FAILURES/compile_failure/trait_wrong_parameter/Prover.toml similarity index 100% rename from crates/nargo_cli/tests/compile_failure/trait_wrong_parameter/Prover.toml rename to crates/nargo_cli/tests/KNOWN_FAILURES/compile_failure/trait_wrong_parameter/Prover.toml diff --git a/crates/nargo_cli/tests/compile_failure/trait_wrong_parameter/src/main.nr b/crates/nargo_cli/tests/KNOWN_FAILURES/compile_failure/trait_wrong_parameter/src/main.nr similarity index 100% rename from crates/nargo_cli/tests/compile_failure/trait_wrong_parameter/src/main.nr rename to crates/nargo_cli/tests/KNOWN_FAILURES/compile_failure/trait_wrong_parameter/src/main.nr diff --git a/crates/nargo_cli/tests/compile_failure/trait_wrong_parameters_count/Nargo.toml b/crates/nargo_cli/tests/KNOWN_FAILURES/compile_failure/trait_wrong_parameters_count/Nargo.toml similarity index 100% rename from crates/nargo_cli/tests/compile_failure/trait_wrong_parameters_count/Nargo.toml rename to crates/nargo_cli/tests/KNOWN_FAILURES/compile_failure/trait_wrong_parameters_count/Nargo.toml diff --git a/crates/nargo_cli/tests/compile_failure/trait_wrong_parameters_count/Prover.toml b/crates/nargo_cli/tests/KNOWN_FAILURES/compile_failure/trait_wrong_parameters_count/Prover.toml similarity index 100% rename from crates/nargo_cli/tests/compile_failure/trait_wrong_parameters_count/Prover.toml rename to crates/nargo_cli/tests/KNOWN_FAILURES/compile_failure/trait_wrong_parameters_count/Prover.toml diff --git a/crates/nargo_cli/tests/compile_failure/trait_wrong_parameters_count/src/main.nr b/crates/nargo_cli/tests/KNOWN_FAILURES/compile_failure/trait_wrong_parameters_count/src/main.nr similarity index 100% rename from crates/nargo_cli/tests/compile_failure/trait_wrong_parameters_count/src/main.nr rename to crates/nargo_cli/tests/KNOWN_FAILURES/compile_failure/trait_wrong_parameters_count/src/main.nr From 0190d68e40474767da91944c6b4bdb5ed7bacc30 Mon Sep 17 00:00:00 2001 From: Nikolay Nikolov Date: Wed, 30 Aug 2023 05:05:00 +0300 Subject: [PATCH 04/32] test(traits): added more tests in KNOWN_FAILURES for the detection of duplicate impl of traits Conclusions from the tests: 1) duplicate checks must happen for the impl itself, not just for the functions inside the trait impl 2) the duplicate check requires type resolution, therefore it must be done later (currently it's done in the def collector) 3) the test runner must be fixed to handle internal compiler errors (panic!) as a test failure for tests in compile_failure (compile_failure tests must produce a proper compiler error, not a compiler crash/internal error/panic!). --- .../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 +++++++++++++++++++ 6 files changed, 55 insertions(+) create mode 100644 crates/nargo_cli/tests/KNOWN_FAILURES/compile_failure/dup_trait_implementation_2/Nargo.toml create mode 100644 crates/nargo_cli/tests/KNOWN_FAILURES/compile_failure/dup_trait_implementation_2/Prover.toml create mode 100644 crates/nargo_cli/tests/KNOWN_FAILURES/compile_failure/dup_trait_implementation_2/src/main.nr create mode 100644 crates/nargo_cli/tests/KNOWN_FAILURES/compile_failure/dup_trait_implementation_3/Nargo.toml create mode 100644 crates/nargo_cli/tests/KNOWN_FAILURES/compile_failure/dup_trait_implementation_3/Prover.toml create mode 100644 crates/nargo_cli/tests/KNOWN_FAILURES/compile_failure/dup_trait_implementation_3/src/main.nr diff --git a/crates/nargo_cli/tests/KNOWN_FAILURES/compile_failure/dup_trait_implementation_2/Nargo.toml b/crates/nargo_cli/tests/KNOWN_FAILURES/compile_failure/dup_trait_implementation_2/Nargo.toml new file mode 100644 index 00000000000..2276db5c741 --- /dev/null +++ b/crates/nargo_cli/tests/KNOWN_FAILURES/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/KNOWN_FAILURES/compile_failure/dup_trait_implementation_2/Prover.toml b/crates/nargo_cli/tests/KNOWN_FAILURES/compile_failure/dup_trait_implementation_2/Prover.toml new file mode 100644 index 00000000000..2c1854573a4 --- /dev/null +++ b/crates/nargo_cli/tests/KNOWN_FAILURES/compile_failure/dup_trait_implementation_2/Prover.toml @@ -0,0 +1,2 @@ +x = 1 +y = 2 diff --git a/crates/nargo_cli/tests/KNOWN_FAILURES/compile_failure/dup_trait_implementation_2/src/main.nr b/crates/nargo_cli/tests/KNOWN_FAILURES/compile_failure/dup_trait_implementation_2/src/main.nr new file mode 100644 index 00000000000..80b544b8e54 --- /dev/null +++ b/crates/nargo_cli/tests/KNOWN_FAILURES/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/KNOWN_FAILURES/compile_failure/dup_trait_implementation_3/Nargo.toml b/crates/nargo_cli/tests/KNOWN_FAILURES/compile_failure/dup_trait_implementation_3/Nargo.toml new file mode 100644 index 00000000000..ac04d9fac4d --- /dev/null +++ b/crates/nargo_cli/tests/KNOWN_FAILURES/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/KNOWN_FAILURES/compile_failure/dup_trait_implementation_3/Prover.toml b/crates/nargo_cli/tests/KNOWN_FAILURES/compile_failure/dup_trait_implementation_3/Prover.toml new file mode 100644 index 00000000000..2c1854573a4 --- /dev/null +++ b/crates/nargo_cli/tests/KNOWN_FAILURES/compile_failure/dup_trait_implementation_3/Prover.toml @@ -0,0 +1,2 @@ +x = 1 +y = 2 diff --git a/crates/nargo_cli/tests/KNOWN_FAILURES/compile_failure/dup_trait_implementation_3/src/main.nr b/crates/nargo_cli/tests/KNOWN_FAILURES/compile_failure/dup_trait_implementation_3/src/main.nr new file mode 100644 index 00000000000..2996b6a00bb --- /dev/null +++ b/crates/nargo_cli/tests/KNOWN_FAILURES/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) { +} From 7ec5c0dbf52ad6b5eca6dc3ed25278c3b3e7cf2b Mon Sep 17 00:00:00 2001 From: Yordan Madzhunkov Date: Wed, 30 Aug 2023 11:46:55 +0300 Subject: [PATCH 05/32] These 2 test should not compile! --- .../Nargo.toml | 7 ++++++ .../Prover.toml | 2 ++ .../src/main.nr | 21 ++++++++++++++++++ .../trait_wrong_parameter2/Nargo.toml | 7 ++++++ .../trait_wrong_parameter2/Prover.toml | 2 ++ .../trait_wrong_parameter2/src/main.nr | 22 +++++++++++++++++++ 6 files changed, 61 insertions(+) create mode 100644 crates/nargo_cli/tests/compile_failure/trait_wrong_method_return_type2/Nargo.toml create mode 100644 crates/nargo_cli/tests/compile_failure/trait_wrong_method_return_type2/Prover.toml create mode 100644 crates/nargo_cli/tests/compile_failure/trait_wrong_method_return_type2/src/main.nr create mode 100644 crates/nargo_cli/tests/compile_failure/trait_wrong_parameter2/Nargo.toml create mode 100644 crates/nargo_cli/tests/compile_failure/trait_wrong_parameter2/Prover.toml create mode 100644 crates/nargo_cli/tests/compile_failure/trait_wrong_parameter2/src/main.nr diff --git a/crates/nargo_cli/tests/compile_failure/trait_wrong_method_return_type2/Nargo.toml b/crates/nargo_cli/tests/compile_failure/trait_wrong_method_return_type2/Nargo.toml new file mode 100644 index 00000000000..95e3e222ca3 --- /dev/null +++ b/crates/nargo_cli/tests/compile_failure/trait_wrong_method_return_type2/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "trait_wrong_method_return_type" +type = "bin" +authors = [""] +compiler_version = "0.9.0" + +[dependencies] \ No newline at end of file diff --git a/crates/nargo_cli/tests/compile_failure/trait_wrong_method_return_type2/Prover.toml b/crates/nargo_cli/tests/compile_failure/trait_wrong_method_return_type2/Prover.toml new file mode 100644 index 00000000000..2c1854573a4 --- /dev/null +++ b/crates/nargo_cli/tests/compile_failure/trait_wrong_method_return_type2/Prover.toml @@ -0,0 +1,2 @@ +x = 1 +y = 2 diff --git a/crates/nargo_cli/tests/compile_failure/trait_wrong_method_return_type2/src/main.nr b/crates/nargo_cli/tests/compile_failure/trait_wrong_method_return_type2/src/main.nr new file mode 100644 index 00000000000..22d65a2d622 --- /dev/null +++ b/crates/nargo_cli/tests/compile_failure/trait_wrong_method_return_type2/src/main.nr @@ -0,0 +1,21 @@ +use dep::std; + +trait Default { + fn default(x: Field, y: Field) -> Self; +} + +struct Foo { + bar: Field, + array: [Field; 2], +} + +impl Default for Foo { + fn default(x: Field, y: Field) -> Field { + x + } +} + +fn main(x: Field, y: Field) { + let first = Foo::default(x,y); + assert(first == x); +} diff --git a/crates/nargo_cli/tests/compile_failure/trait_wrong_parameter2/Nargo.toml b/crates/nargo_cli/tests/compile_failure/trait_wrong_parameter2/Nargo.toml new file mode 100644 index 00000000000..7299ec69e7a --- /dev/null +++ b/crates/nargo_cli/tests/compile_failure/trait_wrong_parameter2/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "trait_wrong_parameter" +type = "bin" +authors = [""] +compiler_version = "0.9.0" + +[dependencies] \ No newline at end of file diff --git a/crates/nargo_cli/tests/compile_failure/trait_wrong_parameter2/Prover.toml b/crates/nargo_cli/tests/compile_failure/trait_wrong_parameter2/Prover.toml new file mode 100644 index 00000000000..2c1854573a4 --- /dev/null +++ b/crates/nargo_cli/tests/compile_failure/trait_wrong_parameter2/Prover.toml @@ -0,0 +1,2 @@ +x = 1 +y = 2 diff --git a/crates/nargo_cli/tests/compile_failure/trait_wrong_parameter2/src/main.nr b/crates/nargo_cli/tests/compile_failure/trait_wrong_parameter2/src/main.nr new file mode 100644 index 00000000000..6cbc59086db --- /dev/null +++ b/crates/nargo_cli/tests/compile_failure/trait_wrong_parameter2/src/main.nr @@ -0,0 +1,22 @@ +use dep::std; + +trait Default { + fn default(x: Field, y: Field) -> Self; +} + +struct Foo { + bar: Field, + array: [Field; 2], +} + +impl Default for Foo { + fn default(x: Field, y: Foo) -> Self { + Self { bar: x, array: [x, y.bar] } + } +} + +fn main(x: Field, y: Field) { + let foo = Foo { bar: x, array: [x, y] }; + let first = Foo::default(x,foo); + assert(first.bar == x); +} From 9d4cbe3c15b7c2bf92260b528da6a35bac07783d Mon Sep 17 00:00:00 2001 From: Yordan Madzhunkov Date: Wed, 30 Aug 2023 12:15:27 +0300 Subject: [PATCH 06/32] Move tests to KNOWN FAILURES --- .../compile_failure/trait_wrong_method_return_type2/Nargo.toml | 0 .../compile_failure/trait_wrong_method_return_type2/Prover.toml | 0 .../compile_failure/trait_wrong_method_return_type2/src/main.nr | 0 .../compile_failure/trait_wrong_parameter2/Nargo.toml | 0 .../compile_failure/trait_wrong_parameter2/Prover.toml | 0 .../compile_failure/trait_wrong_parameter2/src/main.nr | 0 6 files changed, 0 insertions(+), 0 deletions(-) rename crates/nargo_cli/tests/{ => KNOWN_FAILURES}/compile_failure/trait_wrong_method_return_type2/Nargo.toml (100%) rename crates/nargo_cli/tests/{ => KNOWN_FAILURES}/compile_failure/trait_wrong_method_return_type2/Prover.toml (100%) rename crates/nargo_cli/tests/{ => KNOWN_FAILURES}/compile_failure/trait_wrong_method_return_type2/src/main.nr (100%) rename crates/nargo_cli/tests/{ => KNOWN_FAILURES}/compile_failure/trait_wrong_parameter2/Nargo.toml (100%) rename crates/nargo_cli/tests/{ => KNOWN_FAILURES}/compile_failure/trait_wrong_parameter2/Prover.toml (100%) rename crates/nargo_cli/tests/{ => KNOWN_FAILURES}/compile_failure/trait_wrong_parameter2/src/main.nr (100%) diff --git a/crates/nargo_cli/tests/compile_failure/trait_wrong_method_return_type2/Nargo.toml b/crates/nargo_cli/tests/KNOWN_FAILURES/compile_failure/trait_wrong_method_return_type2/Nargo.toml similarity index 100% rename from crates/nargo_cli/tests/compile_failure/trait_wrong_method_return_type2/Nargo.toml rename to crates/nargo_cli/tests/KNOWN_FAILURES/compile_failure/trait_wrong_method_return_type2/Nargo.toml diff --git a/crates/nargo_cli/tests/compile_failure/trait_wrong_method_return_type2/Prover.toml b/crates/nargo_cli/tests/KNOWN_FAILURES/compile_failure/trait_wrong_method_return_type2/Prover.toml similarity index 100% rename from crates/nargo_cli/tests/compile_failure/trait_wrong_method_return_type2/Prover.toml rename to crates/nargo_cli/tests/KNOWN_FAILURES/compile_failure/trait_wrong_method_return_type2/Prover.toml diff --git a/crates/nargo_cli/tests/compile_failure/trait_wrong_method_return_type2/src/main.nr b/crates/nargo_cli/tests/KNOWN_FAILURES/compile_failure/trait_wrong_method_return_type2/src/main.nr similarity index 100% rename from crates/nargo_cli/tests/compile_failure/trait_wrong_method_return_type2/src/main.nr rename to crates/nargo_cli/tests/KNOWN_FAILURES/compile_failure/trait_wrong_method_return_type2/src/main.nr diff --git a/crates/nargo_cli/tests/compile_failure/trait_wrong_parameter2/Nargo.toml b/crates/nargo_cli/tests/KNOWN_FAILURES/compile_failure/trait_wrong_parameter2/Nargo.toml similarity index 100% rename from crates/nargo_cli/tests/compile_failure/trait_wrong_parameter2/Nargo.toml rename to crates/nargo_cli/tests/KNOWN_FAILURES/compile_failure/trait_wrong_parameter2/Nargo.toml diff --git a/crates/nargo_cli/tests/compile_failure/trait_wrong_parameter2/Prover.toml b/crates/nargo_cli/tests/KNOWN_FAILURES/compile_failure/trait_wrong_parameter2/Prover.toml similarity index 100% rename from crates/nargo_cli/tests/compile_failure/trait_wrong_parameter2/Prover.toml rename to crates/nargo_cli/tests/KNOWN_FAILURES/compile_failure/trait_wrong_parameter2/Prover.toml diff --git a/crates/nargo_cli/tests/compile_failure/trait_wrong_parameter2/src/main.nr b/crates/nargo_cli/tests/KNOWN_FAILURES/compile_failure/trait_wrong_parameter2/src/main.nr similarity index 100% rename from crates/nargo_cli/tests/compile_failure/trait_wrong_parameter2/src/main.nr rename to crates/nargo_cli/tests/KNOWN_FAILURES/compile_failure/trait_wrong_parameter2/src/main.nr From 9be47e8a4ea6b30f496a5241430a4c503c9fffff Mon Sep 17 00:00:00 2001 From: Alex Vitkov Date: Thu, 31 Aug 2023 11:55:52 +0300 Subject: [PATCH 07/32] Setup work for checks on Types --- .../src/hir/def_collector/dc_crate.rs | 121 ++++++++++++++++-- .../src/hir/def_collector/dc_mod.rs | 52 +++++--- .../src/hir/resolution/import.rs | 1 + 3 files changed, 147 insertions(+), 27 deletions(-) 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..2fa5aa8298d 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,20 @@ 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, // TODO(vitkov) this should be an ID + pub methods: UnresolvedFunctions, +} + #[derive(Clone)] pub struct UnresolvedTypeAlias { pub file_id: FileId, @@ -74,7 +83,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 +96,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 +108,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 +216,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 +236,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 +312,54 @@ 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(vitkov): 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()); + extend_errors(errors, trait_impl.file_id, resolver.take_errors()); + + // Add the method to the struct's namespace + if let Some(struct_type) = get_struct_type(&typ) { + let struct_type = struct_type.borrow(); + let type_module = struct_type.id.0.local_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 { + todo!(); + } + } + } +} + fn get_struct_type(typ: &Type) -> Option<&Shared> { match typ { Type::Struct(definition, _) => Some(definition), @@ -601,6 +655,53 @@ 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 mut 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 mut path_resolver = StandardPathResolver::new(module_id); + + let mut resolver = + Resolver::new(&mut interner, &mut path_resolver, &context.def_maps, trait_impl.file_id); + + // TODO(vitkov); Handle Type::Error + let self_type = resolver.resolve_type(unresolved_type.clone()); + + let mut impl_methods = resolve_function_set( + &mut interner, + crate_id, + &mut context.def_maps, + trait_impl.methods.clone(), + Some(self_type.clone()), + vec![], // TODO + errors, + ); + + for (_, func_id, _) in &trait_impl.methods.functions { + let method_name = interner.function_name(func_id).to_owned(); + interner.add_method(&self_type, method_name.clone(), *func_id); + } + + //////////////////////////////////////// + // // + // TODO(vitkov): TheChecks™ go here // + // // + //////////////////////////////////////// + + 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 c42867fe91c..2d6d61ef415 100644 --- a/crates/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/crates/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -6,12 +6,15 @@ use crate::{ hir::def_collector::dc_crate::{UnresolvedStruct, UnresolvedTrait}, node_interner::TraitId, parser::SubModule, - FunctionDefinition, Ident, LetStatement, NoirFunction, NoirStruct, NoirTrait, NoirTypeAlias, - ParsedModule, TraitImpl, TraitImplItem, TraitItem, TypeImpl, + FunctionDefinition, Ident, LetStatement, NoirFunction, NoirStruct, NoirTrait, TraitItem, + NoirTypeAlias, ParsedModule, TraitImpl, TraitImplItem, TypeImpl, }; 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}; @@ -135,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 (_, _, noir_function) in &unresolved_functions.functions { + let name = noir_function.name().to_owned(); + let func_id = context.def_interner.push_empty_fn(); + + 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, + }; + + 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(), @@ -170,9 +186,11 @@ impl<'a> ModCollector<'a> { } } - 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, } } diff --git a/crates/noirc_frontend/src/hir/resolution/import.rs b/crates/noirc_frontend/src/hir/resolution/import.rs index 6f3140a65d4..950bf74ed83 100644 --- a/crates/noirc_frontend/src/hir/resolution/import.rs +++ b/crates/noirc_frontend/src/hir/resolution/import.rs @@ -163,6 +163,7 @@ fn resolve_name_in_module( let found_ns = current_mod.find_name(segment); if found_ns.is_none() { + println!("failed to resolv {found_ns:?} {segment:?}"); return Err(PathResolutionError::Unresolved(segment.clone())); } // Check if it is a contract and we're calling from a non-contract context From 7276c4210e6603697ad376feb8f78acb22616b35 Mon Sep 17 00:00:00 2001 From: Alex Vitkov Date: Thu, 31 Aug 2023 12:23:47 +0300 Subject: [PATCH 08/32] remove debug printf --- crates/noirc_frontend/src/hir/resolution/import.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/crates/noirc_frontend/src/hir/resolution/import.rs b/crates/noirc_frontend/src/hir/resolution/import.rs index 950bf74ed83..6f3140a65d4 100644 --- a/crates/noirc_frontend/src/hir/resolution/import.rs +++ b/crates/noirc_frontend/src/hir/resolution/import.rs @@ -163,7 +163,6 @@ fn resolve_name_in_module( let found_ns = current_mod.find_name(segment); if found_ns.is_none() { - println!("failed to resolv {found_ns:?} {segment:?}"); return Err(PathResolutionError::Unresolved(segment.clone())); } // Check if it is a contract and we're calling from a non-contract context From a975c9d2a73694e3a1c45f09de8edc2f4c293c35 Mon Sep 17 00:00:00 2001 From: Yordan Madzhunkov Date: Thu, 31 Aug 2023 12:15:22 +0300 Subject: [PATCH 09/32] Enable tests that implemetns trait twice --- .../compile_failure/dup_trait_implementation_2/Nargo.toml | 0 .../compile_failure/dup_trait_implementation_2/Prover.toml | 0 .../compile_failure/dup_trait_implementation_2/src/main.nr | 0 3 files changed, 0 insertions(+), 0 deletions(-) rename crates/nargo_cli/tests/{KNOWN_FAILURES => }/compile_failure/dup_trait_implementation_2/Nargo.toml (100%) rename crates/nargo_cli/tests/{KNOWN_FAILURES => }/compile_failure/dup_trait_implementation_2/Prover.toml (100%) rename crates/nargo_cli/tests/{KNOWN_FAILURES => }/compile_failure/dup_trait_implementation_2/src/main.nr (100%) diff --git a/crates/nargo_cli/tests/KNOWN_FAILURES/compile_failure/dup_trait_implementation_2/Nargo.toml b/crates/nargo_cli/tests/compile_failure/dup_trait_implementation_2/Nargo.toml similarity index 100% rename from crates/nargo_cli/tests/KNOWN_FAILURES/compile_failure/dup_trait_implementation_2/Nargo.toml rename to crates/nargo_cli/tests/compile_failure/dup_trait_implementation_2/Nargo.toml diff --git a/crates/nargo_cli/tests/KNOWN_FAILURES/compile_failure/dup_trait_implementation_2/Prover.toml b/crates/nargo_cli/tests/compile_failure/dup_trait_implementation_2/Prover.toml similarity index 100% rename from crates/nargo_cli/tests/KNOWN_FAILURES/compile_failure/dup_trait_implementation_2/Prover.toml rename to crates/nargo_cli/tests/compile_failure/dup_trait_implementation_2/Prover.toml diff --git a/crates/nargo_cli/tests/KNOWN_FAILURES/compile_failure/dup_trait_implementation_2/src/main.nr b/crates/nargo_cli/tests/compile_failure/dup_trait_implementation_2/src/main.nr similarity index 100% rename from crates/nargo_cli/tests/KNOWN_FAILURES/compile_failure/dup_trait_implementation_2/src/main.nr rename to crates/nargo_cli/tests/compile_failure/dup_trait_implementation_2/src/main.nr From 64811c8a138a22d4d079a8c4533d195b2c3a99ec Mon Sep 17 00:00:00 2001 From: Yordan Madzhunkov Date: Thu, 31 Aug 2023 15:08:19 +0300 Subject: [PATCH 10/32] Try to build it --- .../src/hir/def_collector/dc_crate.rs | 19 +++++++++--- .../src/hir/def_collector/dc_mod.rs | 5 ++-- .../src/hir/def_collector/errors.rs | 1 + crates/noirc_frontend/src/node_interner.rs | 30 ++++++++++++++++++- 4 files changed, 48 insertions(+), 7 deletions(-) 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 2fa5aa8298d..44b8e5df5e4 100644 --- a/crates/noirc_frontend/src/hir/def_collector/dc_crate.rs +++ b/crates/noirc_frontend/src/hir/def_collector/dc_crate.rs @@ -56,6 +56,7 @@ pub struct UnresolvedTraitImpl { pub module_id: LocalModuleId, pub the_trait: UnresolvedTrait, // TODO(vitkov) this should be an ID pub methods: UnresolvedFunctions, + pub trait_impl_ident: Ident, // for error reporting } #[derive(Clone)] @@ -664,7 +665,7 @@ fn resolve_trait_impls( let mut interner = &mut context.def_interner; let mut methods = Vec::<(FileId, FuncId)>::new(); - for ((unresolved_type, _, _trait_id), trait_impl) in traits { + 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 mut path_resolver = StandardPathResolver::new(module_id); @@ -685,9 +686,19 @@ fn resolve_trait_impls( errors, ); - for (_, func_id, _) in &trait_impl.methods.functions { - let method_name = interner.function_name(func_id).to_owned(); - interner.add_method(&self_type, method_name.clone(), *func_id); + if let Some(first_def) = interner.add_trait_implementaion( + &self_type, + &trait_id, + &trait_impl.trait_impl_ident, + &trait_impl.methods.functions, + ) { + // trait is implemented more then one time ! + let err = DefCollectorErrorKind::Duplicate { + typ: DuplicateType::TraitImplementation, + first_def, + second_def: trait_impl.trait_impl_ident, + }; + errors.push(err.into_file_diagnostic(trait_impl.methods.file_id)); } //////////////////////////////////////// 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 2d6d61ef415..7fe36f910c5 100644 --- a/crates/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/crates/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -6,8 +6,8 @@ use crate::{ hir::def_collector::dc_crate::{UnresolvedStruct, UnresolvedTrait}, node_interner::TraitId, parser::SubModule, - FunctionDefinition, Ident, LetStatement, NoirFunction, NoirStruct, NoirTrait, TraitItem, - NoirTypeAlias, ParsedModule, TraitImpl, TraitImplItem, TypeImpl, + FunctionDefinition, Ident, LetStatement, NoirFunction, NoirStruct, NoirTrait, NoirTypeAlias, + ParsedModule, TraitImpl, TraitImplItem, TraitItem, TypeImpl, }; use super::{ @@ -157,6 +157,7 @@ impl<'a> ModCollector<'a> { module_id: self.module_id, the_trait: collected_trait, methods: unresolved_functions, + trait_impl_ident: trait_impl.clone(), }; let trait_id = match module_def_id { diff --git a/crates/noirc_frontend/src/hir/def_collector/errors.rs b/crates/noirc_frontend/src/hir/def_collector/errors.rs index 2aab7f514d9..ae75d6762d5 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)] diff --git a/crates/noirc_frontend/src/node_interner.rs b/crates/noirc_frontend/src/node_interner.rs index 24015d76a07..d68234b6b0c 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 { } } + // If the self_type did not have implemented the trait_id, None is returned. + // If the self_type did have implemented the trait, the provided methods are updated, and the old implementaion methods are returned. + pub fn add_trait_implementaion( + &mut self, + self_type: &Type, + trait_id: &TraitId, + location: &Ident, + methods: &UnresolvedFunctions, + ) -> Option { + let key = (self_type.clone(), trait_id.clone()); + let func_ids = methods + .functions + .iter() + .flat_map(|(_, func_id, _)| { + self.add_method(&self_type, self.function_name(func_id).to_owned(), *func_id) + }) + .collect::>(); + self.trait_implementaions.insert(key, location.clone()) + } + /// 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() From 379ab660caa8a5f29abe5eea770a9321800e7c0e Mon Sep 17 00:00:00 2001 From: Yordan Madzhunkov Date: Thu, 31 Aug 2023 15:15:32 +0300 Subject: [PATCH 11/32] Fix error reporting when in case of duplicate trait implementation --- .../noirc_frontend/src/hir/def_collector/dc_crate.rs | 12 ++++++------ .../noirc_frontend/src/hir/def_collector/dc_mod.rs | 2 +- .../noirc_frontend/src/hir/def_collector/errors.rs | 1 + crates/noirc_frontend/src/node_interner.rs | 6 +++--- 4 files changed, 11 insertions(+), 10 deletions(-) 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 44b8e5df5e4..f97444a97df 100644 --- a/crates/noirc_frontend/src/hir/def_collector/dc_crate.rs +++ b/crates/noirc_frontend/src/hir/def_collector/dc_crate.rs @@ -662,24 +662,24 @@ fn resolve_trait_impls( crate_id: CrateId, errors: &mut Vec, ) -> Vec<(FileId, FuncId)> { - let mut interner = &mut context.def_interner; + 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 mut path_resolver = StandardPathResolver::new(module_id); + let path_resolver = StandardPathResolver::new(module_id); let mut resolver = - Resolver::new(&mut interner, &mut path_resolver, &context.def_maps, trait_impl.file_id); + Resolver::new(interner, &path_resolver, &context.def_maps, trait_impl.file_id); // TODO(vitkov); Handle Type::Error let self_type = resolver.resolve_type(unresolved_type.clone()); let mut impl_methods = resolve_function_set( - &mut interner, + interner, crate_id, - &mut context.def_maps, + &context.def_maps, trait_impl.methods.clone(), Some(self_type.clone()), vec![], // TODO @@ -690,7 +690,7 @@ fn resolve_trait_impls( &self_type, &trait_id, &trait_impl.trait_impl_ident, - &trait_impl.methods.functions, + &trait_impl.methods, ) { // trait is implemented more then one time ! let err = DefCollectorErrorKind::Duplicate { 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 7fe36f910c5..a179f265aa8 100644 --- a/crates/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/crates/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -157,7 +157,7 @@ impl<'a> ModCollector<'a> { module_id: self.module_id, the_trait: collected_trait, methods: unresolved_functions, - trait_impl_ident: trait_impl.clone(), + trait_impl_ident: trait_impl.trait_name.clone(), }; let trait_id = match module_def_id { diff --git a/crates/noirc_frontend/src/hir/def_collector/errors.rs b/crates/noirc_frontend/src/hir/def_collector/errors.rs index ae75d6762d5..439a82108a4 100644 --- a/crates/noirc_frontend/src/hir/def_collector/errors.rs +++ b/crates/noirc_frontend/src/hir/def_collector/errors.rs @@ -72,6 +72,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"), } } diff --git a/crates/noirc_frontend/src/node_interner.rs b/crates/noirc_frontend/src/node_interner.rs index d68234b6b0c..f8c363ed520 100644 --- a/crates/noirc_frontend/src/node_interner.rs +++ b/crates/noirc_frontend/src/node_interner.rs @@ -720,12 +720,12 @@ impl NodeInterner { location: &Ident, methods: &UnresolvedFunctions, ) -> Option { - let key = (self_type.clone(), trait_id.clone()); - let func_ids = methods + let key = (self_type.clone(), *trait_id); + let _func_ids = methods .functions .iter() .flat_map(|(_, func_id, _)| { - self.add_method(&self_type, self.function_name(func_id).to_owned(), *func_id) + self.add_method(self_type, self.function_name(func_id).to_owned(), *func_id) }) .collect::>(); self.trait_implementaions.insert(key, location.clone()) From 5c3c689b6c79f72c9c798b0520c047c6e1ebfaf8 Mon Sep 17 00:00:00 2001 From: Yordan Madzhunkov Date: Fri, 1 Sep 2023 07:30:36 +0300 Subject: [PATCH 12/32] Enable dup_trait_implementation_3 --- .../compile_failure/dup_trait_implementation_3/Nargo.toml | 0 .../compile_failure/dup_trait_implementation_3/Prover.toml | 0 .../compile_failure/dup_trait_implementation_3/src/main.nr | 0 3 files changed, 0 insertions(+), 0 deletions(-) rename crates/nargo_cli/tests/{KNOWN_FAILURES => }/compile_failure/dup_trait_implementation_3/Nargo.toml (100%) rename crates/nargo_cli/tests/{KNOWN_FAILURES => }/compile_failure/dup_trait_implementation_3/Prover.toml (100%) rename crates/nargo_cli/tests/{KNOWN_FAILURES => }/compile_failure/dup_trait_implementation_3/src/main.nr (100%) diff --git a/crates/nargo_cli/tests/KNOWN_FAILURES/compile_failure/dup_trait_implementation_3/Nargo.toml b/crates/nargo_cli/tests/compile_failure/dup_trait_implementation_3/Nargo.toml similarity index 100% rename from crates/nargo_cli/tests/KNOWN_FAILURES/compile_failure/dup_trait_implementation_3/Nargo.toml rename to crates/nargo_cli/tests/compile_failure/dup_trait_implementation_3/Nargo.toml diff --git a/crates/nargo_cli/tests/KNOWN_FAILURES/compile_failure/dup_trait_implementation_3/Prover.toml b/crates/nargo_cli/tests/compile_failure/dup_trait_implementation_3/Prover.toml similarity index 100% rename from crates/nargo_cli/tests/KNOWN_FAILURES/compile_failure/dup_trait_implementation_3/Prover.toml rename to crates/nargo_cli/tests/compile_failure/dup_trait_implementation_3/Prover.toml diff --git a/crates/nargo_cli/tests/KNOWN_FAILURES/compile_failure/dup_trait_implementation_3/src/main.nr b/crates/nargo_cli/tests/compile_failure/dup_trait_implementation_3/src/main.nr similarity index 100% rename from crates/nargo_cli/tests/KNOWN_FAILURES/compile_failure/dup_trait_implementation_3/src/main.nr rename to crates/nargo_cli/tests/compile_failure/dup_trait_implementation_3/src/main.nr From c6b7582331ed31ae60dc859e9626efeb39dac8e9 Mon Sep 17 00:00:00 2001 From: Yordan Madzhunkov Date: Fri, 1 Sep 2023 10:00:19 +0300 Subject: [PATCH 13/32] Code review comments --- .../src/hir/def_collector/dc_crate.rs | 14 ++++++------ crates/noirc_frontend/src/node_interner.rs | 22 +++++++++---------- 2 files changed, 18 insertions(+), 18 deletions(-) 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 f97444a97df..eaff1079280 100644 --- a/crates/noirc_frontend/src/hir/def_collector/dc_crate.rs +++ b/crates/noirc_frontend/src/hir/def_collector/dc_crate.rs @@ -686,19 +686,19 @@ fn resolve_trait_impls( errors, ); - if let Some(first_def) = interner.add_trait_implementaion( - &self_type, - &trait_id, - &trait_impl.trait_impl_ident, - &trait_impl.methods, - ) { + let trait_definition_ident = &trait_impl.trait_impl_ident; + let key = (self_type, trait_id); + if let Some(first_def) = interner.get_previous_trait_implementation(&key) { // trait is implemented more then one time ! let err = DefCollectorErrorKind::Duplicate { typ: DuplicateType::TraitImplementation, first_def, - second_def: trait_impl.trait_impl_ident, + 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); } //////////////////////////////////////// diff --git a/crates/noirc_frontend/src/node_interner.rs b/crates/noirc_frontend/src/node_interner.rs index f8c363ed520..5e4604d7bdc 100644 --- a/crates/noirc_frontend/src/node_interner.rs +++ b/crates/noirc_frontend/src/node_interner.rs @@ -711,24 +711,24 @@ impl NodeInterner { } } - // If the self_type did not have implemented the trait_id, None is returned. - // If the self_type did have implemented the trait, the provided methods are updated, and the old implementaion methods are returned. + pub fn get_previous_trait_implementation(&self, key: &(Type, TraitId)) -> Option<&Ident> { + self.trait_implementaions.get(key) + } + pub fn add_trait_implementaion( &mut self, - self_type: &Type, - trait_id: &TraitId, - location: &Ident, + key: &(Type, TraitId), + trait_definition_ident: &Ident, methods: &UnresolvedFunctions, - ) -> Option { - let key = (self_type.clone(), *trait_id); - let _func_ids = methods + ) -> Vec { + self.trait_implementaions.insert(key.clone(), trait_definition_ident.clone()); + methods .functions .iter() .flat_map(|(_, func_id, _)| { - self.add_method(self_type, self.function_name(func_id).to_owned(), *func_id) + self.add_method(&key.0, self.function_name(func_id).to_owned(), *func_id) }) - .collect::>(); - self.trait_implementaions.insert(key, location.clone()) + .collect::>() } /// Search by name for a method on the given struct From 562cf9c8b677be19684a98d8d7f8ae4bb108fbd2 Mon Sep 17 00:00:00 2001 From: Yordan Madzhunkov Date: Fri, 1 Sep 2023 10:12:06 +0300 Subject: [PATCH 14/32] Fix build and tests --- crates/noirc_frontend/src/hir/def_collector/dc_crate.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) 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 eaff1079280..783f1747c15 100644 --- a/crates/noirc_frontend/src/hir/def_collector/dc_crate.rs +++ b/crates/noirc_frontend/src/hir/def_collector/dc_crate.rs @@ -688,16 +688,15 @@ fn resolve_trait_impls( let trait_definition_ident = &trait_impl.trait_impl_ident; let key = (self_type, trait_id); - if let Some(first_def) = interner.get_previous_trait_implementation(&key) { - // trait is implemented more then one time ! + if let Some(prev_trait_impl_ident) = interner.get_previous_trait_implementation(&key) { let err = DefCollectorErrorKind::Duplicate { typ: DuplicateType::TraitImplementation, - first_def, + 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 = + let _func_ids = interner.add_trait_implementaion(&key, trait_definition_ident, &trait_impl.methods); } From 6358e358d69fa2596a990f3707cab9b9c0f3d083 Mon Sep 17 00:00:00 2001 From: Alex Vitkov Date: Tue, 5 Sep 2023 07:43:02 +0300 Subject: [PATCH 15/32] fix: public --- crates/noirc_frontend/src/hir/def_collector/dc_crate.rs | 2 +- crates/noirc_frontend/src/node_interner.rs | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) 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 783f1747c15..667cf45f307 100644 --- a/crates/noirc_frontend/src/hir/def_collector/dc_crate.rs +++ b/crates/noirc_frontend/src/hir/def_collector/dc_crate.rs @@ -340,7 +340,7 @@ fn collect_trait_impls( // Add the method to the struct's namespace if let Some(struct_type) = get_struct_type(&typ) { let struct_type = struct_type.borrow(); - let type_module = struct_type.id.0.local_id; + let type_module = struct_type.id.local_module_id(); let module = &mut def_maps.get_mut(&crate_id).unwrap().modules[type_module.0]; diff --git a/crates/noirc_frontend/src/node_interner.rs b/crates/noirc_frontend/src/node_interner.rs index 5e4604d7bdc..ce30b84671e 100644 --- a/crates/noirc_frontend/src/node_interner.rs +++ b/crates/noirc_frontend/src/node_interner.rs @@ -149,6 +149,7 @@ impl FuncId { #[derive(Debug, Eq, PartialEq, Hash, Copy, Clone, PartialOrd, Ord)] pub struct StructId(ModuleId); + impl StructId { //dummy id for error reporting // This can be anything, as the program will ultimately fail From 41b8e48b7c4add68949d249f04cfd3895b28b6bb Mon Sep 17 00:00:00 2001 From: Yordan Madzhunkov Date: Wed, 6 Sep 2023 13:17:15 +0300 Subject: [PATCH 16/32] Enabling back tests that failed --- .../trait_wrong_method_return_type/Nargo.toml | 0 .../Prover.toml | 0 .../src/main.nr | 0 .../Nargo.toml | 0 .../Prover.toml | 0 .../src/main.nr | 0 crates/noirc_frontend/src/ast/expression.rs | 6 +- .../src/hir/def_collector/dc_crate.rs | 87 +++++++++++++++---- .../src/hir/def_collector/dc_mod.rs | 11 ++- .../src/hir/def_collector/errors.rs | 21 +++++ .../src/hir/resolution/resolver.rs | 2 +- crates/noirc_frontend/src/hir_def/types.rs | 2 +- 12 files changed, 104 insertions(+), 25 deletions(-) rename crates/nargo_cli/tests/{KNOWN_FAILURES => }/compile_failure/trait_wrong_method_return_type/Nargo.toml (100%) rename crates/nargo_cli/tests/{KNOWN_FAILURES => }/compile_failure/trait_wrong_method_return_type/Prover.toml (100%) rename crates/nargo_cli/tests/{KNOWN_FAILURES => }/compile_failure/trait_wrong_method_return_type/src/main.nr (100%) rename crates/nargo_cli/tests/{KNOWN_FAILURES => }/compile_failure/trait_wrong_method_return_type2/Nargo.toml (100%) rename crates/nargo_cli/tests/{KNOWN_FAILURES => }/compile_failure/trait_wrong_method_return_type2/Prover.toml (100%) rename crates/nargo_cli/tests/{KNOWN_FAILURES => }/compile_failure/trait_wrong_method_return_type2/src/main.nr (100%) diff --git a/crates/nargo_cli/tests/KNOWN_FAILURES/compile_failure/trait_wrong_method_return_type/Nargo.toml b/crates/nargo_cli/tests/compile_failure/trait_wrong_method_return_type/Nargo.toml similarity index 100% rename from crates/nargo_cli/tests/KNOWN_FAILURES/compile_failure/trait_wrong_method_return_type/Nargo.toml rename to crates/nargo_cli/tests/compile_failure/trait_wrong_method_return_type/Nargo.toml diff --git a/crates/nargo_cli/tests/KNOWN_FAILURES/compile_failure/trait_wrong_method_return_type/Prover.toml b/crates/nargo_cli/tests/compile_failure/trait_wrong_method_return_type/Prover.toml similarity index 100% rename from crates/nargo_cli/tests/KNOWN_FAILURES/compile_failure/trait_wrong_method_return_type/Prover.toml rename to crates/nargo_cli/tests/compile_failure/trait_wrong_method_return_type/Prover.toml diff --git a/crates/nargo_cli/tests/KNOWN_FAILURES/compile_failure/trait_wrong_method_return_type/src/main.nr b/crates/nargo_cli/tests/compile_failure/trait_wrong_method_return_type/src/main.nr similarity index 100% rename from crates/nargo_cli/tests/KNOWN_FAILURES/compile_failure/trait_wrong_method_return_type/src/main.nr rename to crates/nargo_cli/tests/compile_failure/trait_wrong_method_return_type/src/main.nr diff --git a/crates/nargo_cli/tests/KNOWN_FAILURES/compile_failure/trait_wrong_method_return_type2/Nargo.toml b/crates/nargo_cli/tests/compile_failure/trait_wrong_method_return_type2/Nargo.toml similarity index 100% rename from crates/nargo_cli/tests/KNOWN_FAILURES/compile_failure/trait_wrong_method_return_type2/Nargo.toml rename to crates/nargo_cli/tests/compile_failure/trait_wrong_method_return_type2/Nargo.toml diff --git a/crates/nargo_cli/tests/KNOWN_FAILURES/compile_failure/trait_wrong_method_return_type2/Prover.toml b/crates/nargo_cli/tests/compile_failure/trait_wrong_method_return_type2/Prover.toml similarity index 100% rename from crates/nargo_cli/tests/KNOWN_FAILURES/compile_failure/trait_wrong_method_return_type2/Prover.toml rename to crates/nargo_cli/tests/compile_failure/trait_wrong_method_return_type2/Prover.toml diff --git a/crates/nargo_cli/tests/KNOWN_FAILURES/compile_failure/trait_wrong_method_return_type2/src/main.nr b/crates/nargo_cli/tests/compile_failure/trait_wrong_method_return_type2/src/main.nr similarity index 100% rename from crates/nargo_cli/tests/KNOWN_FAILURES/compile_failure/trait_wrong_method_return_type2/src/main.nr rename to crates/nargo_cli/tests/compile_failure/trait_wrong_method_return_type2/src/main.nr diff --git a/crates/noirc_frontend/src/ast/expression.rs b/crates/noirc_frontend/src/ast/expression.rs index 1a829bd0ce7..c64c7ff46ba 100644 --- a/crates/noirc_frontend/src/ast/expression.rs +++ b/crates/noirc_frontend/src/ast/expression.rs @@ -689,10 +689,10 @@ impl Display for FunctionDefinition { } impl FunctionReturnType { - pub fn get_type(&self) -> &UnresolvedTypeData { + pub fn get_type(&self) -> UnresolvedType { match self { - FunctionReturnType::Default(_span) => &UnresolvedTypeData::Unit, - FunctionReturnType::Ty(typ) => &typ.typ, + FunctionReturnType::Default(span) => UnresolvedType { typ: UnresolvedTypeData::Unit, span: Some(span.clone()) }, + FunctionReturnType::Ty(typ) => typ.clone(), } } } 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 667cf45f307..9292a9715a9 100644 --- a/crates/noirc_frontend/src/hir/def_collector/dc_crate.rs +++ b/crates/noirc_frontend/src/hir/def_collector/dc_crate.rs @@ -13,7 +13,7 @@ use crate::hir::type_check::{type_check_func, TypeChecker}; use crate::hir::Context; use crate::node_interner::{FuncId, NodeInterner, StmtId, StructId, TraitId, TypeAliasId}; use crate::{ - ExpressionKind, FunctionReturnType, Generics, Ident, LetStatement, Literal, NoirFunction, + ExpressionKind, Generics, Ident, LetStatement, Literal, NoirFunction, NoirStruct, NoirTrait, NoirTypeAlias, ParsedModule, Shared, StructType, TraitItem, TraitItemType, Type, TypeBinding, UnresolvedGenerics, UnresolvedType, }; @@ -57,6 +57,7 @@ pub struct UnresolvedTraitImpl { pub the_trait: UnresolvedTrait, // TODO(vitkov) this should be an ID pub methods: UnresolvedFunctions, pub trait_impl_ident: Ident, // for error reporting + pub func_id_to_name: HashMap, } #[derive(Clone)] @@ -497,12 +498,8 @@ fn resolve_trait_methods( { let mut resolver = Resolver::new(interner, &path_resolver, def_maps, file); let arguments = vecmap(parameters, |param| resolver.resolve_type(param.1.clone())); - let resolved_return_type = match return_type { - FunctionReturnType::Default(_) => None, - FunctionReturnType::Ty(unresolved_type) => { - Some(resolver.resolve_type(unresolved_type.clone())) - } - }; + let resolved_return_type = resolver.resolve_type(return_type.get_type()); + let name = name.clone(); // TODO let generics: Generics = vec![]; @@ -670,11 +667,13 @@ fn resolve_trait_impls( let module_id = ModuleId { krate: crate_id, local_id: local_mod_id }; let path_resolver = StandardPathResolver::new(module_id); - let mut resolver = - Resolver::new(interner, &path_resolver, &context.def_maps, trait_impl.file_id); - // TODO(vitkov); Handle Type::Error - let self_type = resolver.resolve_type(unresolved_type.clone()); + // TODO(vitkov): Handle Type::Error + 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, @@ -686,6 +685,18 @@ fn resolve_trait_impls( errors, ); + let mut new_resolver = + Resolver::new(interner, &path_resolver, &context.def_maps, trait_impl.file_id); + new_resolver.set_self_type(Some(self_type.clone())); + + check_methods_signatures( + &mut new_resolver, + &impl_methods, + &trait_impl.func_id_to_name, + trait_id, + errors, + ); + let trait_definition_ident = &trait_impl.trait_impl_ident; let key = (self_type, trait_id); if let Some(prev_trait_impl_ident) = interner.get_previous_trait_implementation(&key) { @@ -700,18 +711,60 @@ fn resolve_trait_impls( interner.add_trait_implementaion(&key, trait_definition_ident, &trait_impl.methods); } - //////////////////////////////////////// - // // - // TODO(vitkov): TheChecks™ go here // - // // - //////////////////////////////////////// - methods.append(&mut impl_methods); } methods } + +fn check_methods_signatures( + resolver: &mut Resolver, + impl_methods: &Vec<(FileId, FuncId)>, + func_id_to_name: &HashMap, + trait_id: TraitId, + errors: &mut Vec, +) { + let the_trait = resolver.interner.get_trait(trait_id).clone(); + let the_trait = the_trait.borrow(); + + for (file_id, func_id) in impl_methods { + let func_name = func_id_to_name[func_id].clone(); + let meta = resolver.interner.function_meta(func_id); + + for item in &the_trait.items { + if let TraitItemType::Function { + name, + generics: _, + arguments: _, + return_type, + span: _, + } = item + { + if name.0.contents == func_name { + // TODO(vitkov): Check args + // TODO(vitkov): handle Self + + let resolved_return_type = resolver.resolve_type(meta.return_type.get_type()); + + if resolved_return_type != *return_type { + let err = DefCollectorErrorKind::TraitMethodWrongReturnType { + trait_name: the_trait.name.clone(), + method_name: name.clone(), + span: meta.return_type.get_type().span.unwrap(), + expected_type: return_type.to_string(), + actual_type: resolved_return_type.to_string(), + }; + errors.push(err.into_file_diagnostic(*file_id)); + } + + break; + } + } + } + } +} + 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 a179f265aa8..1cf6cf980a6 100644 --- a/crates/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/crates/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -1,3 +1,5 @@ +use std::collections::HashMap; + use fm::FileId; use noirc_errors::{FileDiagnostic, Location}; @@ -145,11 +147,13 @@ impl<'a> ModCollector<'a> { errors, ); - for (_, _, noir_function) in &unresolved_functions.functions { + let mut func_id_to_name = HashMap::new(); + + for (_, func_id, noir_function) in &unresolved_functions.functions { let name = noir_function.name().to_owned(); - let func_id = context.def_interner.push_empty_fn(); - context.def_interner.push_function_definition(name, func_id); + func_id_to_name.insert(*func_id, name.clone()); + context.def_interner.push_function_definition(name, *func_id); } let unresolved_trait_impl = UnresolvedTraitImpl { @@ -158,6 +162,7 @@ impl<'a> ModCollector<'a> { the_trait: collected_trait, methods: unresolved_functions, trait_impl_ident: trait_impl.trait_name.clone(), + func_id_to_name, }; let trait_id = match module_def_id { diff --git a/crates/noirc_frontend/src/hir/def_collector/errors.rs b/crates/noirc_frontend/src/hir/def_collector/errors.rs index 439a82108a4..1e15b3b34d3 100644 --- a/crates/noirc_frontend/src/hir/def_collector/errors.rs +++ b/crates/noirc_frontend/src/hir/def_collector/errors.rs @@ -56,6 +56,14 @@ pub enum DefCollectorErrorKind { TraitNotFound { trait_name: String, span: Span }, #[error("Missing Trait method implementation")] TraitMissedMethodImplementation { trait_name: Ident, method_name: Ident, trait_impl_span: Span }, + #[error("Trait method wrong return type")] + TraitMethodWrongReturnType { + trait_name: Ident, + method_name: Ident, + span: Span, + expected_type: String, + actual_type: String, + }, } impl DefCollectorErrorKind { @@ -193,6 +201,19 @@ impl From for Diagnostic { span, ) } + DefCollectorErrorKind::TraitMethodWrongReturnType { + trait_name, + method_name, + span, + expected_type, + actual_type, + } => { + Diagnostic::simple_error( + format!("Method `{method_name}` from trait `{trait_name}` must return {expected_type}, not {actual_type}"), + String::new(), + span, + ) + } } } } diff --git a/crates/noirc_frontend/src/hir/resolution/resolver.rs b/crates/noirc_frontend/src/hir/resolution/resolver.rs index 1ab4118099d..004edf75f8b 100644 --- a/crates/noirc_frontend/src/hir/resolution/resolver.rs +++ b/crates/noirc_frontend/src/hir/resolution/resolver.rs @@ -76,7 +76,7 @@ pub struct Resolver<'a> { scopes: ScopeForest, path_resolver: &'a dyn PathResolver, def_maps: &'a BTreeMap, - interner: &'a mut NodeInterner, + pub interner: &'a mut NodeInterner, errors: Vec, file: FileId, diff --git a/crates/noirc_frontend/src/hir_def/types.rs b/crates/noirc_frontend/src/hir_def/types.rs index 8372f7a0355..228e30db884 100644 --- a/crates/noirc_frontend/src/hir_def/types.rs +++ b/crates/noirc_frontend/src/hir_def/types.rs @@ -130,7 +130,7 @@ pub enum TraitItemType { name: Ident, generics: Generics, arguments: Vec, - return_type: Option, + return_type: Type, span: Span, }, From c65884e612446c17ec29266dda0c9bbee9b8c84b Mon Sep 17 00:00:00 2001 From: Yordan Madzhunkov Date: Tue, 5 Sep 2023 09:24:18 +0300 Subject: [PATCH 17/32] Obtain function name from resolver --- crates/noirc_frontend/src/hir/def_collector/dc_crate.rs | 5 +---- crates/noirc_frontend/src/hir/def_collector/dc_mod.rs | 5 ----- 2 files changed, 1 insertion(+), 9 deletions(-) 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 9292a9715a9..28da71ac3db 100644 --- a/crates/noirc_frontend/src/hir/def_collector/dc_crate.rs +++ b/crates/noirc_frontend/src/hir/def_collector/dc_crate.rs @@ -57,7 +57,6 @@ pub struct UnresolvedTraitImpl { pub the_trait: UnresolvedTrait, // TODO(vitkov) this should be an ID pub methods: UnresolvedFunctions, pub trait_impl_ident: Ident, // for error reporting - pub func_id_to_name: HashMap, } #[derive(Clone)] @@ -692,7 +691,6 @@ fn resolve_trait_impls( check_methods_signatures( &mut new_resolver, &impl_methods, - &trait_impl.func_id_to_name, trait_id, errors, ); @@ -721,7 +719,6 @@ fn resolve_trait_impls( fn check_methods_signatures( resolver: &mut Resolver, impl_methods: &Vec<(FileId, FuncId)>, - func_id_to_name: &HashMap, trait_id: TraitId, errors: &mut Vec, ) { @@ -729,8 +726,8 @@ fn check_methods_signatures( let the_trait = the_trait.borrow(); for (file_id, func_id) in impl_methods { - let func_name = func_id_to_name[func_id].clone(); let meta = resolver.interner.function_meta(func_id); + let func_name = resolver.interner.function_name(func_id); for item in &the_trait.items { if let TraitItemType::Function { 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 1cf6cf980a6..f1d8210d57a 100644 --- a/crates/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/crates/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -1,5 +1,3 @@ -use std::collections::HashMap; - use fm::FileId; use noirc_errors::{FileDiagnostic, Location}; @@ -147,12 +145,10 @@ impl<'a> ModCollector<'a> { errors, ); - let mut func_id_to_name = HashMap::new(); for (_, func_id, noir_function) in &unresolved_functions.functions { let name = noir_function.name().to_owned(); - func_id_to_name.insert(*func_id, name.clone()); context.def_interner.push_function_definition(name, *func_id); } @@ -162,7 +158,6 @@ impl<'a> ModCollector<'a> { the_trait: collected_trait, methods: unresolved_functions, trait_impl_ident: trait_impl.trait_name.clone(), - func_id_to_name, }; let trait_id = match module_def_id { From ef46c6da304af836141a9b6b6858104e6f39f150 Mon Sep 17 00:00:00 2001 From: Alex Vitkov Date: Tue, 5 Sep 2023 09:29:10 +0300 Subject: [PATCH 18/32] typevar for Trait::Self --- .../src/hir/def_collector/dc_crate.rs | 15 +++++++++++++-- .../noirc_frontend/src/hir/resolution/resolver.rs | 4 ++++ crates/noirc_frontend/src/hir_def/types.rs | 15 ++++++++++++++- crates/noirc_frontend/src/node_interner.rs | 3 +++ 4 files changed, 34 insertions(+), 3 deletions(-) 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 28da71ac3db..051186d6256 100644 --- a/crates/noirc_frontend/src/hir/def_collector/dc_crate.rs +++ b/crates/noirc_frontend/src/hir/def_collector/dc_crate.rs @@ -15,7 +15,7 @@ use crate::node_interner::{FuncId, NodeInterner, StmtId, StructId, TraitId, Type use crate::{ ExpressionKind, Generics, Ident, LetStatement, Literal, NoirFunction, NoirStruct, NoirTrait, NoirTypeAlias, ParsedModule, Shared, StructType, TraitItem, - TraitItemType, Type, TypeBinding, UnresolvedGenerics, UnresolvedType, + TraitItemType, Type, TypeBinding, UnresolvedGenerics, UnresolvedType, TypeVariableKind, }; use fm::FileId; use iter_extended::vecmap; @@ -470,6 +470,7 @@ fn resolve_trait_constants( fn resolve_trait_methods( context: &mut Context, + trait_id: TraitId, crate_id: CrateId, unresolved_trait: &UnresolvedTrait, errors: &mut Vec, @@ -495,7 +496,12 @@ fn resolve_trait_methods( body: _, } = item { + let the_trait = interner.get_trait(trait_id); + let self_type = Type::TypeVariable(the_trait.borrow().self_type_typevar.clone(), TypeVariableKind::Normal); + let mut resolver = Resolver::new(interner, &path_resolver, def_maps, file); + resolver.set_self_type(Some(self_type)); + let arguments = vecmap(parameters, |param| resolver.resolve_type(param.1.clone())); let resolved_return_type = resolver.resolve_type(return_type.get_type()); @@ -551,7 +557,7 @@ fn resolve_traits( // 2. Trait Constants ( Trait's methods can use trait types & constants, threfore they should be after) items.append(&mut resolve_trait_constants(context, crate_id, &unresolved_trait, errors)); // 3. Trait Methods - items.append(&mut resolve_trait_methods(context, crate_id, &unresolved_trait, errors)); + items.append(&mut resolve_trait_methods(context, trait_id, crate_id, &unresolved_trait, errors)); context.def_interner.update_trait(trait_id, |trait_def| { trait_def.set_items(items); }); @@ -725,6 +731,9 @@ fn check_methods_signatures( let the_trait = resolver.interner.get_trait(trait_id).clone(); let the_trait = the_trait.borrow(); + let self_type = resolver.get_self_type().expect("trait impl must have a Self type"); + the_trait.self_type_typevar.borrow_mut().force_bind_to(self_type); + for (file_id, func_id) in impl_methods { let meta = resolver.interner.function_meta(func_id); let func_name = resolver.interner.function_name(func_id); @@ -742,7 +751,9 @@ fn check_methods_signatures( // TODO(vitkov): Check args // TODO(vitkov): handle Self + println!("{}", meta.return_type.get_type()); let resolved_return_type = resolver.resolve_type(meta.return_type.get_type()); + println!("resolved {}", resolved_return_type); if resolved_return_type != *return_type { let err = DefCollectorErrorKind::TraitMethodWrongReturnType { diff --git a/crates/noirc_frontend/src/hir/resolution/resolver.rs b/crates/noirc_frontend/src/hir/resolution/resolver.rs index 004edf75f8b..9d7dc2cd51a 100644 --- a/crates/noirc_frontend/src/hir/resolution/resolver.rs +++ b/crates/noirc_frontend/src/hir/resolution/resolver.rs @@ -127,6 +127,10 @@ impl<'a> Resolver<'a> { self.self_type = self_type; } + pub fn get_self_type(&mut self) -> Option { + self.self_type.clone() + } + fn push_err(&mut self, err: ResolverError) { self.errors.push(err); } diff --git a/crates/noirc_frontend/src/hir_def/types.rs b/crates/noirc_frontend/src/hir_def/types.rs index 228e30db884..767d3b3cd3a 100644 --- a/crates/noirc_frontend/src/hir_def/types.rs +++ b/crates/noirc_frontend/src/hir_def/types.rs @@ -154,6 +154,12 @@ pub struct Trait { pub name: Ident, pub generics: Generics, pub span: Span, + + /// When resolving the types of Trait elements, all references to `Self` resolve + /// to this TypeVariable. Then when we check if the types of trait impl elements + /// match the definition in the trait, we bind this TypeVariable to whatever + /// the correct Self type is for that particular impl block. + pub self_type_typevar: TypeVariable, } /// Corresponds to generic lists such as `` in the source @@ -193,8 +199,10 @@ impl Trait { span: Span, items: Vec, generics: Generics, + self_type_typevar: TypeVariable, ) -> Trait { - Trait { id, name, span, items, generics } + + Trait { id, name, span, items, generics, self_type_typevar } } pub fn set_items(&mut self, items: Vec) { @@ -462,6 +470,11 @@ impl TypeBinding { } } } + + /// Same as bind_to, but allows rebinding + pub fn force_bind_to(&mut self, typ: Type) { + *self = TypeBinding::Bound(typ); + } } /// A unique ID used to differentiate different type variables diff --git a/crates/noirc_frontend/src/node_interner.rs b/crates/noirc_frontend/src/node_interner.rs index ce30b84671e..6cda7796df1 100644 --- a/crates/noirc_frontend/src/node_interner.rs +++ b/crates/noirc_frontend/src/node_interner.rs @@ -349,6 +349,8 @@ impl NodeInterner { } pub fn push_empty_trait(&mut self, type_id: TraitId, typ: &UnresolvedTrait) { + let self_type_typevar = Shared::new(TypeBinding::Unbound(self.next_type_variable_id())); + self.traits.insert( type_id, Shared::new(Trait::new( @@ -364,6 +366,7 @@ impl NodeInterner { let id = TypeVariableId(0); (id, Shared::new(TypeBinding::Unbound(id))) }), + self_type_typevar, )), ); } From e4f3f27664d55accc3e1c32ee7312aed8d198264 Mon Sep 17 00:00:00 2001 From: Alex Vitkov Date: Tue, 5 Sep 2023 09:45:56 +0300 Subject: [PATCH 19/32] break up Trait::items into 3 arrays --- .../src/hir/def_collector/dc_crate.rs | 66 ++++++++----------- crates/noirc_frontend/src/hir_def/types.rs | 53 +++++++++------ crates/noirc_frontend/src/node_interner.rs | 1 - 3 files changed, 61 insertions(+), 59 deletions(-) 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 051186d6256..0920c6f58b5 100644 --- a/crates/noirc_frontend/src/hir/def_collector/dc_crate.rs +++ b/crates/noirc_frontend/src/hir/def_collector/dc_crate.rs @@ -15,7 +15,7 @@ use crate::node_interner::{FuncId, NodeInterner, StmtId, StructId, TraitId, Type use crate::{ ExpressionKind, Generics, Ident, LetStatement, Literal, NoirFunction, NoirStruct, NoirTrait, NoirTypeAlias, ParsedModule, Shared, StructType, TraitItem, - TraitItemType, Type, TypeBinding, UnresolvedGenerics, UnresolvedType, TypeVariableKind, + Type, TypeBinding, UnresolvedGenerics, UnresolvedType, TypeVariableKind, TraitType, TraitConstant, TraitFunction, }; use fm::FileId; use iter_extended::vecmap; @@ -454,7 +454,7 @@ fn resolve_trait_types( _crate_id: CrateId, _unresolved_trait: &UnresolvedTrait, _errors: &mut [FileDiagnostic], -) -> Vec { +) -> Vec { // TODO vec![] } @@ -463,7 +463,7 @@ fn resolve_trait_constants( _crate_id: CrateId, _unresolved_trait: &UnresolvedTrait, _errors: &mut [FileDiagnostic], -) -> Vec { +) -> Vec { // TODO vec![] } @@ -474,7 +474,7 @@ fn resolve_trait_methods( crate_id: CrateId, unresolved_trait: &UnresolvedTrait, errors: &mut Vec, -) -> Vec { +) -> Vec { let interner = &mut context.def_interner; let def_maps = &mut context.def_maps; @@ -509,7 +509,7 @@ fn resolve_trait_methods( // TODO let generics: Generics = vec![]; let span: Span = name.span(); - let f = TraitItemType::Function { + let f = TraitFunction { name, generics, arguments, @@ -550,16 +550,17 @@ fn resolve_traits( context.def_interner.push_empty_trait(*trait_id, unresolved_trait); } for (trait_id, unresolved_trait) in traits { - let mut items: Vec = vec![]; + // Resolve order // 1. Trait Types ( Trait contants can have a trait type, therefore types before constants) - items.append(&mut resolve_trait_types(context, crate_id, &unresolved_trait, errors)); + let _ = resolve_trait_types(context, crate_id, &unresolved_trait, errors); // 2. Trait Constants ( Trait's methods can use trait types & constants, threfore they should be after) - items.append(&mut resolve_trait_constants(context, crate_id, &unresolved_trait, errors)); + let _ = resolve_trait_constants(context, crate_id, &unresolved_trait, errors); // 3. Trait Methods - items.append(&mut resolve_trait_methods(context, trait_id, crate_id, &unresolved_trait, errors)); + let methods = resolve_trait_methods(context, trait_id, crate_id, &unresolved_trait, errors); + context.def_interner.update_trait(trait_id, |trait_def| { - trait_def.set_items(items); + trait_def.set_methods(methods); }); } } @@ -738,36 +739,23 @@ fn check_methods_signatures( let meta = resolver.interner.function_meta(func_id); let func_name = resolver.interner.function_name(func_id); - for item in &the_trait.items { - if let TraitItemType::Function { - name, - generics: _, - arguments: _, - return_type, - span: _, - } = item - { - if name.0.contents == func_name { - // TODO(vitkov): Check args - // TODO(vitkov): handle Self - - println!("{}", meta.return_type.get_type()); - let resolved_return_type = resolver.resolve_type(meta.return_type.get_type()); - println!("resolved {}", resolved_return_type); - - if resolved_return_type != *return_type { - let err = DefCollectorErrorKind::TraitMethodWrongReturnType { - trait_name: the_trait.name.clone(), - method_name: name.clone(), - span: meta.return_type.get_type().span.unwrap(), - expected_type: return_type.to_string(), - actual_type: resolved_return_type.to_string(), - }; - errors.push(err.into_file_diagnostic(*file_id)); - } - - break; + for method in &the_trait.methods { + if method.name.0.contents == func_name { + // TODO(vitkov): Check args + let resolved_return_type = resolver.resolve_type(meta.return_type.get_type()); + + if resolved_return_type != method.return_type { + let err = DefCollectorErrorKind::TraitMethodWrongReturnType { + trait_name: the_trait.name.clone(), + method_name: method.name.clone(), + span: meta.return_type.get_type().span.unwrap(), + expected_type: method.return_type.to_string(), + actual_type: resolved_return_type.to_string(), + }; + errors.push(err.into_file_diagnostic(*file_id)); } + + break; } } } diff --git a/crates/noirc_frontend/src/hir_def/types.rs b/crates/noirc_frontend/src/hir_def/types.rs index 767d3b3cd3a..58bb7ab30c1 100644 --- a/crates/noirc_frontend/src/hir_def/types.rs +++ b/crates/noirc_frontend/src/hir_def/types.rs @@ -124,22 +124,28 @@ pub struct StructType { } #[derive(Debug, PartialEq, Eq)] -pub enum TraitItemType { - /// A function declaration in a trait. - Function { - name: Ident, - generics: Generics, - arguments: Vec, - return_type: Type, - span: Span, - }, +pub struct TraitFunction { + pub name: Ident, + pub generics: Generics, + pub arguments: Vec, + pub return_type: Type, + pub span: Span, +} - /// A constant declaration in a trait. - Constant { name: Ident, ty: Type, span: Span }, +#[derive(Debug, PartialEq, Eq)] +pub struct TraitConstant { + pub name: Ident, + pub ty: Type, + pub span: Span, +} - /// A type declaration in a trait. - Type { name: Ident, ty: Type, span: Span }, +#[derive(Debug, PartialEq, Eq)] +pub struct TraitType { + pub name: Ident, + pub ty: Type, + pub span: Span, } + /// Represents a trait type in the type system. Each instance of this /// rust struct will be shared across all Type::Trait variants that represent /// the same trait type. @@ -149,7 +155,9 @@ pub struct Trait { /// struct traits are equal. pub id: TraitId, - pub items: Vec, + pub methods: Vec, + pub constants: Vec, + pub types: Vec, pub name: Ident, pub generics: Generics, @@ -197,16 +205,23 @@ impl Trait { id: TraitId, name: Ident, span: Span, - items: Vec, generics: Generics, self_type_typevar: TypeVariable, ) -> Trait { - - Trait { id, name, span, items, generics, self_type_typevar } + Trait { + id, + name, + span, + methods: Vec::new(), + constants: Vec::new(), + types: Vec::new(), + generics, + self_type_typevar, + } } - pub fn set_items(&mut self, items: Vec) { - self.items = items; + pub fn set_methods(&mut self, methods: Vec) { + self.methods = methods; } } diff --git a/crates/noirc_frontend/src/node_interner.rs b/crates/noirc_frontend/src/node_interner.rs index 6cda7796df1..7650cd2f538 100644 --- a/crates/noirc_frontend/src/node_interner.rs +++ b/crates/noirc_frontend/src/node_interner.rs @@ -357,7 +357,6 @@ impl NodeInterner { type_id, typ.trait_def.name.clone(), typ.trait_def.span, - Vec::new(), vecmap(&typ.trait_def.generics, |_| { // Temporary type variable ids before the trait is resolved to its actual ids. // This lets us record how many arguments the type expects so that other types From ba7747dd3dbcbaa53a3770e0da2267ad289ab752 Mon Sep 17 00:00:00 2001 From: Alex Vitkov Date: Tue, 5 Sep 2023 10:25:14 +0300 Subject: [PATCH 20/32] use unify instead of == to compare types --- crates/noirc_frontend/src/ast/expression.rs | 4 +- .../src/hir/def_collector/dc_crate.rs | 62 ++++++++++--------- .../src/hir/def_collector/dc_mod.rs | 1 - 3 files changed, 36 insertions(+), 31 deletions(-) diff --git a/crates/noirc_frontend/src/ast/expression.rs b/crates/noirc_frontend/src/ast/expression.rs index c64c7ff46ba..30b1431e7a2 100644 --- a/crates/noirc_frontend/src/ast/expression.rs +++ b/crates/noirc_frontend/src/ast/expression.rs @@ -691,7 +691,9 @@ impl Display for FunctionDefinition { impl FunctionReturnType { pub fn get_type(&self) -> UnresolvedType { match self { - FunctionReturnType::Default(span) => UnresolvedType { typ: UnresolvedTypeData::Unit, span: Some(span.clone()) }, + FunctionReturnType::Default(span) => { + UnresolvedType { typ: UnresolvedTypeData::Unit, span: Some(span.clone()) } + } FunctionReturnType::Ty(typ) => typ.clone(), } } 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 0920c6f58b5..fc03024bdc5 100644 --- a/crates/noirc_frontend/src/hir/def_collector/dc_crate.rs +++ b/crates/noirc_frontend/src/hir/def_collector/dc_crate.rs @@ -9,13 +9,13 @@ use crate::hir::resolution::{ import::{resolve_imports, ImportDirective}, path_resolver::StandardPathResolver, }; -use crate::hir::type_check::{type_check_func, TypeChecker}; +use crate::hir::type_check::{type_check_func, TypeCheckError, TypeChecker}; use crate::hir::Context; use crate::node_interner::{FuncId, NodeInterner, StmtId, StructId, TraitId, TypeAliasId}; use crate::{ - ExpressionKind, Generics, Ident, LetStatement, Literal, NoirFunction, - NoirStruct, NoirTrait, NoirTypeAlias, ParsedModule, Shared, StructType, TraitItem, - Type, TypeBinding, UnresolvedGenerics, UnresolvedType, TypeVariableKind, TraitType, TraitConstant, TraitFunction, + ExpressionKind, Generics, Ident, LetStatement, Literal, NoirFunction, NoirStruct, NoirTrait, + NoirTypeAlias, ParsedModule, Shared, StructType, TraitConstant, TraitFunction, TraitItem, + TraitType, Type, TypeBinding, TypeVariableKind, UnresolvedGenerics, UnresolvedType, }; use fm::FileId; use iter_extended::vecmap; @@ -244,8 +244,8 @@ impl DefCollector { // Type check all of the functions in the crate type_check_functions(&mut context.def_interner, file_func_ids, errors); - type_check_functions(&mut context.def_interner, file_trait_impls_ids, errors); type_check_functions(&mut context.def_interner, file_method_ids, errors); + type_check_functions(&mut context.def_interner, file_trait_impls_ids, errors); } } @@ -497,7 +497,10 @@ fn resolve_trait_methods( } = item { let the_trait = interner.get_trait(trait_id); - let self_type = Type::TypeVariable(the_trait.borrow().self_type_typevar.clone(), TypeVariableKind::Normal); + let self_type = Type::TypeVariable( + the_trait.borrow().self_type_typevar.clone(), + TypeVariableKind::Normal, + ); let mut resolver = Resolver::new(interner, &path_resolver, def_maps, file); resolver.set_self_type(Some(self_type)); @@ -550,7 +553,6 @@ fn resolve_traits( context.def_interner.push_empty_trait(*trait_id, unresolved_trait); } for (trait_id, unresolved_trait) in traits { - // Resolve order // 1. Trait Types ( Trait contants can have a trait type, therefore types before constants) let _ = resolve_trait_types(context, crate_id, &unresolved_trait, errors); @@ -673,7 +675,6 @@ fn resolve_trait_impls( let module_id = ModuleId { krate: crate_id, local_id: local_mod_id }; let path_resolver = StandardPathResolver::new(module_id); - // TODO(vitkov): Handle Type::Error let self_type = { let mut resolver = @@ -695,15 +696,10 @@ fn resolve_trait_impls( Resolver::new(interner, &path_resolver, &context.def_maps, trait_impl.file_id); new_resolver.set_self_type(Some(self_type.clone())); - check_methods_signatures( - &mut new_resolver, - &impl_methods, - trait_id, - errors, - ); + check_methods_signatures(&mut new_resolver, &impl_methods, trait_id, errors); let trait_definition_ident = &trait_impl.trait_impl_ident; - let key = (self_type, trait_id); + 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, @@ -722,15 +718,14 @@ fn resolve_trait_impls( methods } - fn check_methods_signatures( resolver: &mut Resolver, impl_methods: &Vec<(FileId, FuncId)>, trait_id: TraitId, errors: &mut Vec, ) { - let the_trait = resolver.interner.get_trait(trait_id).clone(); - let the_trait = the_trait.borrow(); + let the_trait_shared = resolver.interner.get_trait(trait_id).clone(); + let the_trait = the_trait_shared.borrow(); let self_type = resolver.get_self_type().expect("trait impl must have a Self type"); the_trait.self_type_typevar.borrow_mut().force_bind_to(self_type); @@ -741,20 +736,29 @@ fn check_methods_signatures( for method in &the_trait.methods { if method.name.0.contents == func_name { - // TODO(vitkov): Check args + // ----------------------------- + // TODO(vitkov): Check args here + // ----------------------------- + + // Check that impl method return type matches trait return type: let resolved_return_type = resolver.resolve_type(meta.return_type.get_type()); - if resolved_return_type != method.return_type { - let err = DefCollectorErrorKind::TraitMethodWrongReturnType { - trait_name: the_trait.name.clone(), - method_name: method.name.clone(), - span: meta.return_type.get_type().span.unwrap(), - expected_type: method.return_type.to_string(), - actual_type: resolved_return_type.to_string(), - }; - errors.push(err.into_file_diagnostic(*file_id)); - } + let mut typecheck_errors = Vec::new(); + method.return_type.unify(&resolved_return_type, &mut typecheck_errors, || { + let ret_type_span = meta + .return_type + .get_type() + .span + .expect("return type must always have a span"); + + TypeCheckError::TypeMismatch { + expected_typ: method.return_type.to_string(), + expr_typ: meta.return_type().to_string(), + expr_span: ret_type_span, + } + }); + extend_errors(errors, *file_id, typecheck_errors); break; } } 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 f1d8210d57a..7b34fbc0a8e 100644 --- a/crates/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/crates/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -145,7 +145,6 @@ impl<'a> ModCollector<'a> { errors, ); - for (_, func_id, noir_function) in &unresolved_functions.functions { let name = noir_function.name().to_owned(); From fa61f445fbaa4e8361e02f30cedffd6cb1c2a417 Mon Sep 17 00:00:00 2001 From: Alex Vitkov Date: Tue, 5 Sep 2023 10:28:25 +0300 Subject: [PATCH 21/32] remove clone --- crates/noirc_frontend/src/hir/def_collector/dc_crate.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 fc03024bdc5..45c73796060 100644 --- a/crates/noirc_frontend/src/hir/def_collector/dc_crate.rs +++ b/crates/noirc_frontend/src/hir/def_collector/dc_crate.rs @@ -724,7 +724,7 @@ fn check_methods_signatures( trait_id: TraitId, errors: &mut Vec, ) { - let the_trait_shared = resolver.interner.get_trait(trait_id).clone(); + let the_trait_shared = resolver.interner.get_trait(trait_id); let the_trait = the_trait_shared.borrow(); let self_type = resolver.get_self_type().expect("trait impl must have a Self type"); From cbf5629e4084794a13ba292e01325af3cebdda39 Mon Sep 17 00:00:00 2001 From: Alex Vitkov Date: Tue, 5 Sep 2023 10:35:22 +0300 Subject: [PATCH 22/32] test: add test --- .../execution_success/trait_self/Nargo.toml | 7 +++++++ .../execution_success/trait_self/src/main.nr | 18 ++++++++++++++++++ 2 files changed, 25 insertions(+) create mode 100644 crates/nargo_cli/tests/execution_success/trait_self/Nargo.toml create mode 100644 crates/nargo_cli/tests/execution_success/trait_self/src/main.nr diff --git a/crates/nargo_cli/tests/execution_success/trait_self/Nargo.toml b/crates/nargo_cli/tests/execution_success/trait_self/Nargo.toml new file mode 100644 index 00000000000..0dfaea44862 --- /dev/null +++ b/crates/nargo_cli/tests/execution_success/trait_self/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "trait_self" +type = "bin" +authors = [""] +compiler_version = "0.10.5" + +[dependencies] \ No newline at end of file diff --git a/crates/nargo_cli/tests/execution_success/trait_self/src/main.nr b/crates/nargo_cli/tests/execution_success/trait_self/src/main.nr new file mode 100644 index 00000000000..f4f73822cc3 --- /dev/null +++ b/crates/nargo_cli/tests/execution_success/trait_self/src/main.nr @@ -0,0 +1,18 @@ +struct Foo { + x: Field +} + +trait Asd { + fn asd() -> Self; +} + +impl Asd for Foo { + // the Self should typecheck properly + fn asd() -> Self { + Foo{x: 100} + } +} + +fn main() { + assert(Foo::asd().x == 100); +} \ No newline at end of file From 3191234af3c13b8b3a5560da5415f8ed6b462850 Mon Sep 17 00:00:00 2001 From: Yordan Madzhunkov Date: Tue, 5 Sep 2023 11:19:29 +0000 Subject: [PATCH 23/32] New error when trait is implemented for non-struct MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit trait Default { fn default(x: Field, y: Field) -> Field; } impl Default for Zoo { fn default(x: Field, y: Field) -> Field { x + y } } error: Only struct types may implement trait `Default` ┌─ /home/yordan/Work/noir/crates/nargo_cli/tests/compile_failure/impl_trait_for_non_struct/src/main.nr:8:6 │ 8 │ impl Default for main { │ ------- Only struct types may implement traits --- .../impl_trait_for_non_struct/Nargo.toml | 7 +++++++ .../impl_trait_for_non_struct/Prover.toml | 2 ++ .../impl_trait_for_non_struct/src/main.nr | 15 +++++++++++++++ .../src/hir/def_collector/dc_crate.rs | 8 ++++++-- .../src/hir/def_collector/dc_mod.rs | 5 +---- .../src/hir/def_collector/errors.rs | 15 +++++++++++---- 6 files changed, 42 insertions(+), 10 deletions(-) create mode 100644 crates/nargo_cli/tests/compile_failure/impl_trait_for_non_struct/Nargo.toml create mode 100644 crates/nargo_cli/tests/compile_failure/impl_trait_for_non_struct/Prover.toml create mode 100644 crates/nargo_cli/tests/compile_failure/impl_trait_for_non_struct/src/main.nr diff --git a/crates/nargo_cli/tests/compile_failure/impl_trait_for_non_struct/Nargo.toml b/crates/nargo_cli/tests/compile_failure/impl_trait_for_non_struct/Nargo.toml new file mode 100644 index 00000000000..21a44bfd36b --- /dev/null +++ b/crates/nargo_cli/tests/compile_failure/impl_trait_for_non_struct/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "impl_trait_for_non_struct" +type = "bin" +authors = [""] +compiler_version = "0.9.0" + +[dependencies] diff --git a/crates/nargo_cli/tests/compile_failure/impl_trait_for_non_struct/Prover.toml b/crates/nargo_cli/tests/compile_failure/impl_trait_for_non_struct/Prover.toml new file mode 100644 index 00000000000..2c1854573a4 --- /dev/null +++ b/crates/nargo_cli/tests/compile_failure/impl_trait_for_non_struct/Prover.toml @@ -0,0 +1,2 @@ +x = 1 +y = 2 diff --git a/crates/nargo_cli/tests/compile_failure/impl_trait_for_non_struct/src/main.nr b/crates/nargo_cli/tests/compile_failure/impl_trait_for_non_struct/src/main.nr new file mode 100644 index 00000000000..8bd6aae8064 --- /dev/null +++ b/crates/nargo_cli/tests/compile_failure/impl_trait_for_non_struct/src/main.nr @@ -0,0 +1,15 @@ +use dep::std; + +trait Default { + fn default(x: Field, y: Field) -> Field; +} + + +impl Default for main { + fn default(x: Field, y: Field) -> Field { + x + y + } +} + +fn main(x: Field, y: Field) { +} 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 45c73796060..7a5c075f551 100644 --- a/crates/noirc_frontend/src/hir/def_collector/dc_crate.rs +++ b/crates/noirc_frontend/src/hir/def_collector/dc_crate.rs @@ -335,10 +335,11 @@ fn collect_trait_impls( 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()); - extend_errors(errors, trait_impl.file_id, resolver.take_errors()); // 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(); @@ -355,7 +356,10 @@ fn collect_trait_impls( errors.push(err.into_file_diagnostic(trait_impl.file_id)); } } else { - todo!(); + 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)); } } } 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 7b34fbc0a8e..2c35d66d78e 100644 --- a/crates/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/crates/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -176,10 +176,7 @@ 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)); } } diff --git a/crates/noirc_frontend/src/hir/def_collector/errors.rs b/crates/noirc_frontend/src/hir/def_collector/errors.rs index 1e15b3b34d3..721e9e2c869 100644 --- a/crates/noirc_frontend/src/hir/def_collector/errors.rs +++ b/crates/noirc_frontend/src/hir/def_collector/errors.rs @@ -30,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")] @@ -53,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 }, #[error("Trait method wrong return type")] @@ -122,15 +124,20 @@ 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, From 5b15d565d88751ade6337b9ff5ecc3702480384d Mon Sep 17 00:00:00 2001 From: Alex Vitkov Date: Tue, 5 Sep 2023 11:12:19 +0300 Subject: [PATCH 24/32] add check for trait method argc --- .../trait_wrong_method_name/src/main.nr | 3 +- .../trait_wrong_parameter/Nargo.toml | 0 .../trait_wrong_parameter/Prover.toml | 0 .../trait_wrong_parameter/src/main.nr | 0 .../trait_wrong_parameter2/Nargo.toml | 0 .../trait_wrong_parameter2/Prover.toml | 0 .../trait_wrong_parameter2/src/main.nr | 0 .../trait_wrong_parameters_count/Nargo.toml | 0 .../trait_wrong_parameters_count/Prover.toml | 0 .../trait_wrong_parameters_count/src/main.nr | 0 .../src/hir/def_collector/dc_crate.rs | 71 ++++++++++++------- .../src/hir/def_collector/errors.rs | 70 ++---------------- .../src/hir/type_check/errors.rs | 15 ++++ crates/noirc_frontend/src/hir_def/stmt.rs | 7 ++ 14 files changed, 76 insertions(+), 90 deletions(-) rename crates/nargo_cli/tests/{KNOWN_FAILURES => }/compile_failure/trait_wrong_parameter/Nargo.toml (100%) rename crates/nargo_cli/tests/{KNOWN_FAILURES => }/compile_failure/trait_wrong_parameter/Prover.toml (100%) rename crates/nargo_cli/tests/{KNOWN_FAILURES => }/compile_failure/trait_wrong_parameter/src/main.nr (100%) rename crates/nargo_cli/tests/{KNOWN_FAILURES => }/compile_failure/trait_wrong_parameter2/Nargo.toml (100%) rename crates/nargo_cli/tests/{KNOWN_FAILURES => }/compile_failure/trait_wrong_parameter2/Prover.toml (100%) rename crates/nargo_cli/tests/{KNOWN_FAILURES => }/compile_failure/trait_wrong_parameter2/src/main.nr (100%) rename crates/nargo_cli/tests/{KNOWN_FAILURES => }/compile_failure/trait_wrong_parameters_count/Nargo.toml (100%) rename crates/nargo_cli/tests/{KNOWN_FAILURES => }/compile_failure/trait_wrong_parameters_count/Prover.toml (100%) rename crates/nargo_cli/tests/{KNOWN_FAILURES => }/compile_failure/trait_wrong_parameters_count/src/main.nr (100%) diff --git a/crates/nargo_cli/tests/compile_failure/trait_wrong_method_name/src/main.nr b/crates/nargo_cli/tests/compile_failure/trait_wrong_method_name/src/main.nr index dab18516aae..470bed9b354 100644 --- a/crates/nargo_cli/tests/compile_failure/trait_wrong_method_name/src/main.nr +++ b/crates/nargo_cli/tests/compile_failure/trait_wrong_method_name/src/main.nr @@ -1,7 +1,6 @@ use dep::std; trait Default { - fn default(x: Field, y: Field) -> Self; } struct Foo { @@ -11,7 +10,7 @@ struct Foo { // wrong trait name method should not compile impl Default for Foo { - fn default_wrong_name(x: Field, y: Field) -> Self { + fn doesnt_exist(x: Field, y: Field) -> Self { Self { bar: x, array: [x,y] } } } diff --git a/crates/nargo_cli/tests/KNOWN_FAILURES/compile_failure/trait_wrong_parameter/Nargo.toml b/crates/nargo_cli/tests/compile_failure/trait_wrong_parameter/Nargo.toml similarity index 100% rename from crates/nargo_cli/tests/KNOWN_FAILURES/compile_failure/trait_wrong_parameter/Nargo.toml rename to crates/nargo_cli/tests/compile_failure/trait_wrong_parameter/Nargo.toml diff --git a/crates/nargo_cli/tests/KNOWN_FAILURES/compile_failure/trait_wrong_parameter/Prover.toml b/crates/nargo_cli/tests/compile_failure/trait_wrong_parameter/Prover.toml similarity index 100% rename from crates/nargo_cli/tests/KNOWN_FAILURES/compile_failure/trait_wrong_parameter/Prover.toml rename to crates/nargo_cli/tests/compile_failure/trait_wrong_parameter/Prover.toml diff --git a/crates/nargo_cli/tests/KNOWN_FAILURES/compile_failure/trait_wrong_parameter/src/main.nr b/crates/nargo_cli/tests/compile_failure/trait_wrong_parameter/src/main.nr similarity index 100% rename from crates/nargo_cli/tests/KNOWN_FAILURES/compile_failure/trait_wrong_parameter/src/main.nr rename to crates/nargo_cli/tests/compile_failure/trait_wrong_parameter/src/main.nr diff --git a/crates/nargo_cli/tests/KNOWN_FAILURES/compile_failure/trait_wrong_parameter2/Nargo.toml b/crates/nargo_cli/tests/compile_failure/trait_wrong_parameter2/Nargo.toml similarity index 100% rename from crates/nargo_cli/tests/KNOWN_FAILURES/compile_failure/trait_wrong_parameter2/Nargo.toml rename to crates/nargo_cli/tests/compile_failure/trait_wrong_parameter2/Nargo.toml diff --git a/crates/nargo_cli/tests/KNOWN_FAILURES/compile_failure/trait_wrong_parameter2/Prover.toml b/crates/nargo_cli/tests/compile_failure/trait_wrong_parameter2/Prover.toml similarity index 100% rename from crates/nargo_cli/tests/KNOWN_FAILURES/compile_failure/trait_wrong_parameter2/Prover.toml rename to crates/nargo_cli/tests/compile_failure/trait_wrong_parameter2/Prover.toml diff --git a/crates/nargo_cli/tests/KNOWN_FAILURES/compile_failure/trait_wrong_parameter2/src/main.nr b/crates/nargo_cli/tests/compile_failure/trait_wrong_parameter2/src/main.nr similarity index 100% rename from crates/nargo_cli/tests/KNOWN_FAILURES/compile_failure/trait_wrong_parameter2/src/main.nr rename to crates/nargo_cli/tests/compile_failure/trait_wrong_parameter2/src/main.nr diff --git a/crates/nargo_cli/tests/KNOWN_FAILURES/compile_failure/trait_wrong_parameters_count/Nargo.toml b/crates/nargo_cli/tests/compile_failure/trait_wrong_parameters_count/Nargo.toml similarity index 100% rename from crates/nargo_cli/tests/KNOWN_FAILURES/compile_failure/trait_wrong_parameters_count/Nargo.toml rename to crates/nargo_cli/tests/compile_failure/trait_wrong_parameters_count/Nargo.toml diff --git a/crates/nargo_cli/tests/KNOWN_FAILURES/compile_failure/trait_wrong_parameters_count/Prover.toml b/crates/nargo_cli/tests/compile_failure/trait_wrong_parameters_count/Prover.toml similarity index 100% rename from crates/nargo_cli/tests/KNOWN_FAILURES/compile_failure/trait_wrong_parameters_count/Prover.toml rename to crates/nargo_cli/tests/compile_failure/trait_wrong_parameters_count/Prover.toml diff --git a/crates/nargo_cli/tests/KNOWN_FAILURES/compile_failure/trait_wrong_parameters_count/src/main.nr b/crates/nargo_cli/tests/compile_failure/trait_wrong_parameters_count/src/main.nr similarity index 100% rename from crates/nargo_cli/tests/KNOWN_FAILURES/compile_failure/trait_wrong_parameters_count/src/main.nr rename to crates/nargo_cli/tests/compile_failure/trait_wrong_parameters_count/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 7a5c075f551..7a7b358f9f5 100644 --- a/crates/noirc_frontend/src/hir/def_collector/dc_crate.rs +++ b/crates/noirc_frontend/src/hir/def_collector/dc_crate.rs @@ -738,33 +738,54 @@ fn check_methods_signatures( let meta = resolver.interner.function_meta(func_id); let func_name = resolver.interner.function_name(func_id); - for method in &the_trait.methods { - if method.name.0.contents == func_name { - // ----------------------------- - // TODO(vitkov): Check args here - // ----------------------------- - - // Check that impl method return type matches trait return type: - let resolved_return_type = resolver.resolve_type(meta.return_type.get_type()); - - let mut typecheck_errors = Vec::new(); - method.return_type.unify(&resolved_return_type, &mut typecheck_errors, || { - let ret_type_span = meta - .return_type - .get_type() - .span - .expect("return type must always have a span"); - - TypeCheckError::TypeMismatch { - expected_typ: method.return_type.to_string(), - expr_typ: meta.return_type().to_string(), - expr_span: ret_type_span, - } - }); + let mut typecheck_errors = Vec::new(); - extend_errors(errors, *file_id, typecheck_errors); - break; + if let Some(method) = + the_trait.methods.iter().find(|method| method.name.0.contents == func_name) + { + if method.arguments.len() == meta.parameters.0.len() { + // Check the parameters of the impl method against the parameters of the trait method + for (parameter_index, (expected, (hir_pattern, actual, _))) in + method.arguments.iter().zip(&meta.parameters.0).enumerate() + { + expected.unify(&actual, &mut typecheck_errors, || { + TypeCheckError::TraitMethodArgMismatch { + method_name: func_name.to_string(), + expected_typ: expected.to_string(), + actual_typ: actual.to_string(), + expr_span: hir_pattern.span(), + parameter_index: parameter_index + 1, + } + }); + } + } else { + errors.push( + DefCollectorErrorKind::MismatchTraitImplementationNumParameters { + actual_num_parameters: meta.parameters.0.len(), + expected_num_parameters: method.arguments.len(), + trait_name: the_trait.name.to_string(), + method_name: func_name.to_string(), + span: meta.location.span, + } + .into_file_diagnostic(*file_id), + ) } + + // Check that impl method return type matches trait return type: + let resolved_return_type = resolver.resolve_type(meta.return_type.get_type()); + + method.return_type.unify(&resolved_return_type, &mut typecheck_errors, || { + let ret_type_span = + meta.return_type.get_type().span.expect("return type must always have a span"); + + TypeCheckError::TypeMismatch { + expected_typ: method.return_type.to_string(), + expr_typ: meta.return_type().to_string(), + expr_span: ret_type_span, + } + }); + + extend_errors(errors, *file_id, typecheck_errors); } } } diff --git a/crates/noirc_frontend/src/hir/def_collector/errors.rs b/crates/noirc_frontend/src/hir/def_collector/errors.rs index 721e9e2c869..2be2766d227 100644 --- a/crates/noirc_frontend/src/hir/def_collector/errors.rs +++ b/crates/noirc_frontend/src/hir/def_collector/errors.rs @@ -1,6 +1,5 @@ use crate::hir::resolution::import::PathResolutionError; use crate::Ident; -use crate::UnresolvedType; use noirc_errors::CustomDiagnostic as Diagnostic; use noirc_errors::FileDiagnostic; @@ -34,21 +33,13 @@ pub enum DefCollectorErrorKind { 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")] - MismatchTraitImlementationParameter { - trait_name: String, - impl_method: String, - parameter: Ident, - expected_type: UnresolvedType, - }, - #[error("Mismatch return type of trait implementation")] - MismatchTraitImplementationReturnType { trait_name: String, impl_ident: Ident }, #[error("Mismatch number of parameters in of trait implementation")] MismatchTraitImplementationNumParameters { actual_num_parameters: usize, expected_num_parameters: usize, trait_name: String, - impl_ident: Ident, + method_name: String, + span: Span, }, #[error("Method is not defined in trait")] MethodNotInTrait { trait_name: Ident, impl_method: Ident }, @@ -58,14 +49,6 @@ pub enum DefCollectorErrorKind { TraitNotFound { trait_ident: Ident }, #[error("Missing Trait method implementation")] TraitMissedMethodImplementation { trait_name: Ident, method_name: Ident, trait_impl_span: Span }, - #[error("Trait method wrong return type")] - TraitMethodWrongReturnType { - trait_name: Ident, - method_name: Ident, - span: Span, - expected_type: String, - actual_type: String, - }, } impl DefCollectorErrorKind { @@ -139,48 +122,22 @@ impl From for Diagnostic { "".to_string(), trait_ident.span(), ), - DefCollectorErrorKind::MismatchTraitImplementationReturnType { - trait_name, - impl_ident, - } => { - let span = impl_ident.span(); - let method_name = impl_ident.0.contents; - Diagnostic::simple_error( - format!("Mismatch return type of method with name {method_name} that implements trait {trait_name}"), - "".to_string(), - span, - ) - } DefCollectorErrorKind::MismatchTraitImplementationNumParameters { expected_num_parameters, actual_num_parameters, trait_name, - impl_ident, - } => { - let method_name = impl_ident.0.contents.clone(); - let primary_message = format!( - "Mismatch - expected {expected_num_parameters} arguments, but got {actual_num_parameters} of trait `{trait_name}` implementation `{method_name}`"); - Diagnostic::simple_error(primary_message, "".to_string(), impl_ident.span()) - } - DefCollectorErrorKind::MismatchTraitImlementationParameter { - trait_name, - impl_method, - parameter, - expected_type, + method_name, + span, } => { let primary_message = format!( - "Mismatch signature of method {impl_method} that implements trait {trait_name}" - ); - let secondary_message = - format!("`{}: {}` expected", parameter.0.contents, expected_type,); - let span = parameter.span(); - Diagnostic::simple_error(primary_message, secondary_message, span) + "method `{method_name}` of trait `{trait_name}` needs {expected_num_parameters} parameters, but has {actual_num_parameters}"); + Diagnostic::simple_error(primary_message, "".to_string(), span) } DefCollectorErrorKind::MethodNotInTrait { trait_name, impl_method } => { let trait_name = trait_name.0.contents; let impl_method_span = impl_method.span(); let impl_method_name = impl_method.0.contents; - let primary_message = format!("method with name {impl_method_name} is not part of trait {trait_name}, therefore it can't be implemented"); + let primary_message = format!("Method with name {impl_method_name} is not part of trait {trait_name}, therefore it can't be implemented"); Diagnostic::simple_error(primary_message, "".to_owned(), impl_method_span) } DefCollectorErrorKind::TraitMissedMethodImplementation { @@ -208,19 +165,6 @@ impl From for Diagnostic { span, ) } - DefCollectorErrorKind::TraitMethodWrongReturnType { - trait_name, - method_name, - span, - expected_type, - actual_type, - } => { - Diagnostic::simple_error( - format!("Method `{method_name}` from trait `{trait_name}` must return {expected_type}, not {actual_type}"), - String::new(), - span, - ) - } } } } diff --git a/crates/noirc_frontend/src/hir/type_check/errors.rs b/crates/noirc_frontend/src/hir/type_check/errors.rs index 3190c7a24a2..6e1ad8a8417 100644 --- a/crates/noirc_frontend/src/hir/type_check/errors.rs +++ b/crates/noirc_frontend/src/hir/type_check/errors.rs @@ -100,6 +100,14 @@ pub enum TypeCheckError { ResolverError(ResolverError), #[error("Unused expression result of type {expr_type}")] UnusedResultError { expr_type: Type, expr_span: Span }, + #[error("Expected type {expected_typ:?} is not the same as {actual_typ:?}")] + TraitMethodArgMismatch { + method_name: String, + expected_typ: String, + actual_typ: String, + expr_span: Span, + parameter_index: usize, + }, } impl TypeCheckError { @@ -133,6 +141,13 @@ impl From for Diagnostic { expr_span, ) } + TypeCheckError::TraitMethodArgMismatch { method_name, expected_typ, actual_typ, parameter_index, expr_span } => { + Diagnostic::simple_error( + format!("Parameter #{parameter_index} of method `{method_name}` must be of type {expected_typ}, not {actual_typ}"), + String::new(), + expr_span, + ) + } TypeCheckError::NonHomogeneousArray { first_span, first_type, diff --git a/crates/noirc_frontend/src/hir_def/stmt.rs b/crates/noirc_frontend/src/hir_def/stmt.rs index 0dcb7192be2..036b3d5b403 100644 --- a/crates/noirc_frontend/src/hir_def/stmt.rs +++ b/crates/noirc_frontend/src/hir_def/stmt.rs @@ -79,6 +79,13 @@ impl HirPattern { other => panic!("Tried to iterate over the fields of '{other:?}', which has none"), } } + + pub fn span(&self) -> Span { + match self { + HirPattern::Identifier(ident) => ident.location.span, + HirPattern::Mutable(_, span) | HirPattern::Tuple(_, span) | HirPattern::Struct(_, _, span) => *span, + } + } } /// Represents an Ast form that can be assigned to. These From 580c1d77d914197d98b603c087b748b48617310f Mon Sep 17 00:00:00 2001 From: Alex Vitkov Date: Tue, 5 Sep 2023 11:52:41 +0300 Subject: [PATCH 25/32] consistent capitalization --- crates/noirc_frontend/src/hir/def_collector/errors.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/noirc_frontend/src/hir/def_collector/errors.rs b/crates/noirc_frontend/src/hir/def_collector/errors.rs index 2be2766d227..344bc56aac6 100644 --- a/crates/noirc_frontend/src/hir/def_collector/errors.rs +++ b/crates/noirc_frontend/src/hir/def_collector/errors.rs @@ -130,7 +130,7 @@ impl From for Diagnostic { span, } => { let primary_message = format!( - "method `{method_name}` of trait `{trait_name}` needs {expected_num_parameters} parameters, but has {actual_num_parameters}"); + "Method `{method_name}` of trait `{trait_name}` needs {expected_num_parameters} parameters, but has {actual_num_parameters}"); Diagnostic::simple_error(primary_message, "".to_string(), span) } DefCollectorErrorKind::MethodNotInTrait { trait_name, impl_method } => { @@ -148,7 +148,7 @@ impl From for Diagnostic { let trait_name = trait_name.0.contents; let impl_method_name = method_name.0.contents; let primary_message = format!( - "method `{impl_method_name}` from trait `{trait_name}` is not implemented" + "Method `{impl_method_name}` from trait `{trait_name}` is not implemented" ); Diagnostic::simple_error( primary_message, From ea7bbc98beb29935db1c1d0780f5c350e7de56ac Mon Sep 17 00:00:00 2001 From: Alex Vitkov Date: Tue, 5 Sep 2023 11:54:17 +0300 Subject: [PATCH 26/32] better error struct field names --- crates/noirc_frontend/src/hir/def_collector/dc_crate.rs | 4 ++-- crates/noirc_frontend/src/hir/type_check/errors.rs | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) 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 7a7b358f9f5..217703fe32c 100644 --- a/crates/noirc_frontend/src/hir/def_collector/dc_crate.rs +++ b/crates/noirc_frontend/src/hir/def_collector/dc_crate.rs @@ -749,11 +749,11 @@ fn check_methods_signatures( method.arguments.iter().zip(&meta.parameters.0).enumerate() { expected.unify(&actual, &mut typecheck_errors, || { - TypeCheckError::TraitMethodArgMismatch { + TypeCheckError::TraitMethodParameterTypeMismatch { method_name: func_name.to_string(), expected_typ: expected.to_string(), actual_typ: actual.to_string(), - expr_span: hir_pattern.span(), + parameter_span: hir_pattern.span(), parameter_index: parameter_index + 1, } }); diff --git a/crates/noirc_frontend/src/hir/type_check/errors.rs b/crates/noirc_frontend/src/hir/type_check/errors.rs index 6e1ad8a8417..ece3a4c61ef 100644 --- a/crates/noirc_frontend/src/hir/type_check/errors.rs +++ b/crates/noirc_frontend/src/hir/type_check/errors.rs @@ -101,11 +101,11 @@ pub enum TypeCheckError { #[error("Unused expression result of type {expr_type}")] UnusedResultError { expr_type: Type, expr_span: Span }, #[error("Expected type {expected_typ:?} is not the same as {actual_typ:?}")] - TraitMethodArgMismatch { + TraitMethodParameterTypeMismatch { method_name: String, expected_typ: String, actual_typ: String, - expr_span: Span, + parameter_span: Span, parameter_index: usize, }, } @@ -141,11 +141,11 @@ impl From for Diagnostic { expr_span, ) } - TypeCheckError::TraitMethodArgMismatch { method_name, expected_typ, actual_typ, parameter_index, expr_span } => { + TypeCheckError::TraitMethodParameterTypeMismatch { method_name, expected_typ, actual_typ, parameter_index, parameter_span } => { Diagnostic::simple_error( format!("Parameter #{parameter_index} of method `{method_name}` must be of type {expected_typ}, not {actual_typ}"), String::new(), - expr_span, + parameter_span, ) } TypeCheckError::NonHomogeneousArray { From dac1fe28e5f1828d158fed5c51d10fcf81210a82 Mon Sep 17 00:00:00 2001 From: Alex Vitkov Date: Tue, 5 Sep 2023 14:26:38 +0300 Subject: [PATCH 27/32] emit MethodNotInTrait when needed --- .../src/hir/def_collector/dc_crate.rs | 1 + .../src/hir/def_collector/dc_mod.rs | 28 +++++++++++++++++-- .../src/hir/def_collector/errors.rs | 2 +- 3 files changed, 27 insertions(+), 4 deletions(-) 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 217703fe32c..344c96e7af2 100644 --- a/crates/noirc_frontend/src/hir/def_collector/dc_crate.rs +++ b/crates/noirc_frontend/src/hir/def_collector/dc_crate.rs @@ -740,6 +740,7 @@ fn check_methods_signatures( let mut typecheck_errors = Vec::new(); + // `method` is none in the case where the impl block has a method that's not part of the Trait if let Some(method) = the_trait.methods.iter().find(|method| method.name.0.contents == func_name) { 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 2c35d66d78e..319ccda4838 100644 --- a/crates/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/crates/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -1,3 +1,5 @@ +use std::collections::HashSet; + use fm::FileId; use noirc_errors::{FileDiagnostic, Location}; @@ -212,6 +214,9 @@ impl<'a> ModCollector<'a> { } } + // set of function ids that have a corresponding method in the trait + let mut func_ids_in_trait = HashSet::new(); + for item in &trait_def.items { if let TraitItem::Function { name, @@ -222,11 +227,14 @@ impl<'a> ModCollector<'a> { body, } = item { - let is_implemented = unresolved_functions + let implementing_fn = unresolved_functions .functions .iter() - .any(|(_, _, func_impl)| func_impl.name() == name.0.contents); - if !is_implemented { + .find(|(_, _, func_impl)| func_impl.name() == name.0.contents); + + if let Some((_, func_id, _)) = implementing_fn { + func_ids_in_trait.insert(*func_id); + } else { match body { Some(body) => { let method_name = name.0.contents.clone(); @@ -240,6 +248,7 @@ impl<'a> ModCollector<'a> { where_clause, return_type, )); + func_ids_in_trait.insert(func_id); unresolved_functions.push_fn(self.module_id, func_id, impl_method); } None => { @@ -254,6 +263,19 @@ impl<'a> ModCollector<'a> { } } } + + // Emit MethodNotInTrait error for methods in the impl block that + // don't have a corresponding method signature defined in the trait + for (_, func_id, func) in &unresolved_functions.functions { + if !func_ids_in_trait.contains(func_id) { + let error = DefCollectorErrorKind::MethodNotInTrait { + trait_name: trait_def.name.clone(), + impl_method: func.name_ident().clone(), + }; + errors.push(error.into_file_diagnostic(self.file_id)); + } + } + unresolved_functions } diff --git a/crates/noirc_frontend/src/hir/def_collector/errors.rs b/crates/noirc_frontend/src/hir/def_collector/errors.rs index 344bc56aac6..ef2e9b00090 100644 --- a/crates/noirc_frontend/src/hir/def_collector/errors.rs +++ b/crates/noirc_frontend/src/hir/def_collector/errors.rs @@ -137,7 +137,7 @@ impl From for Diagnostic { let trait_name = trait_name.0.contents; let impl_method_span = impl_method.span(); let impl_method_name = impl_method.0.contents; - let primary_message = format!("Method with name {impl_method_name} is not part of trait {trait_name}, therefore it can't be implemented"); + let primary_message = format!("Method with name `{impl_method_name}` is not part of trait `{trait_name}`, therefore it can't be implemented"); Diagnostic::simple_error(primary_message, "".to_owned(), impl_method_span) } DefCollectorErrorKind::TraitMissedMethodImplementation { From e33b8b43f19a4fe424d3a0267f4f86774e9ccaa8 Mon Sep 17 00:00:00 2001 From: Alex Vitkov Date: Tue, 5 Sep 2023 14:29:33 +0300 Subject: [PATCH 28/32] remove outdated todos --- crates/noirc_frontend/src/hir/def_collector/dc_crate.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) 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 344c96e7af2..8f631c4c251 100644 --- a/crates/noirc_frontend/src/hir/def_collector/dc_crate.rs +++ b/crates/noirc_frontend/src/hir/def_collector/dc_crate.rs @@ -54,7 +54,7 @@ pub struct UnresolvedTrait { pub struct UnresolvedTraitImpl { pub file_id: FileId, pub module_id: LocalModuleId, - pub the_trait: UnresolvedTrait, // TODO(vitkov) this should be an ID + pub the_trait: UnresolvedTrait, pub methods: UnresolvedFunctions, pub trait_impl_ident: Ident, // for error reporting } @@ -322,7 +322,7 @@ fn collect_trait_impls( let interner = &mut context.def_interner; let def_maps = &mut context.def_maps; - // TODO(vitkov): To follow the semantics of Rust, we must allow the impl if either + // 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 { @@ -679,7 +679,6 @@ fn resolve_trait_impls( let module_id = ModuleId { krate: crate_id, local_id: local_mod_id }; let path_resolver = StandardPathResolver::new(module_id); - // TODO(vitkov): Handle Type::Error let self_type = { let mut resolver = Resolver::new(interner, &path_resolver, &context.def_maps, trait_impl.file_id); From 77180f4555da91668e8b6143da29a5545ffeadbb Mon Sep 17 00:00:00 2001 From: Yordan Madzhunkov Date: Tue, 5 Sep 2023 15:07:57 +0300 Subject: [PATCH 29/32] Fix clippy --- crates/noirc_frontend/src/ast/expression.rs | 2 +- .../src/hir/def_collector/dc_crate.rs | 4 ++-- .../noirc_frontend/src/hir/def_collector/errors.rs | 14 ++++++++------ crates/noirc_frontend/src/hir_def/stmt.rs | 4 +++- 4 files changed, 14 insertions(+), 10 deletions(-) diff --git a/crates/noirc_frontend/src/ast/expression.rs b/crates/noirc_frontend/src/ast/expression.rs index 30b1431e7a2..399b4d867a3 100644 --- a/crates/noirc_frontend/src/ast/expression.rs +++ b/crates/noirc_frontend/src/ast/expression.rs @@ -692,7 +692,7 @@ impl FunctionReturnType { pub fn get_type(&self) -> UnresolvedType { match self { FunctionReturnType::Default(span) => { - UnresolvedType { typ: UnresolvedTypeData::Unit, span: Some(span.clone()) } + UnresolvedType { typ: UnresolvedTypeData::Unit, span: Some(*span) } } FunctionReturnType::Ty(typ) => typ.clone(), } 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 8f631c4c251..9172050e2a3 100644 --- a/crates/noirc_frontend/src/hir/def_collector/dc_crate.rs +++ b/crates/noirc_frontend/src/hir/def_collector/dc_crate.rs @@ -748,7 +748,7 @@ fn check_methods_signatures( for (parameter_index, (expected, (hir_pattern, actual, _))) in method.arguments.iter().zip(&meta.parameters.0).enumerate() { - expected.unify(&actual, &mut typecheck_errors, || { + expected.unify(actual, &mut typecheck_errors, || { TypeCheckError::TraitMethodParameterTypeMismatch { method_name: func_name.to_string(), expected_typ: expected.to_string(), @@ -768,7 +768,7 @@ fn check_methods_signatures( span: meta.location.span, } .into_file_diagnostic(*file_id), - ) + ); } // Check that impl method return type matches trait return type: diff --git a/crates/noirc_frontend/src/hir/def_collector/errors.rs b/crates/noirc_frontend/src/hir/def_collector/errors.rs index ef2e9b00090..32f3a7fe718 100644 --- a/crates/noirc_frontend/src/hir/def_collector/errors.rs +++ b/crates/noirc_frontend/src/hir/def_collector/errors.rs @@ -107,17 +107,19 @@ 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::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_ident} => Diagnostic::simple_error( + DefCollectorErrorKind::TraitNotFound { trait_ident } => Diagnostic::simple_error( format!("Trait {} not found", trait_ident), "".to_string(), trait_ident.span(), diff --git a/crates/noirc_frontend/src/hir_def/stmt.rs b/crates/noirc_frontend/src/hir_def/stmt.rs index 036b3d5b403..d7f0d2e466f 100644 --- a/crates/noirc_frontend/src/hir_def/stmt.rs +++ b/crates/noirc_frontend/src/hir_def/stmt.rs @@ -83,7 +83,9 @@ impl HirPattern { pub fn span(&self) -> Span { match self { HirPattern::Identifier(ident) => ident.location.span, - HirPattern::Mutable(_, span) | HirPattern::Tuple(_, span) | HirPattern::Struct(_, _, span) => *span, + HirPattern::Mutable(_, span) + | HirPattern::Tuple(_, span) + | HirPattern::Struct(_, _, span) => *span, } } } From 000a8db5e33215089008c101ab0c6a1dd50fd500 Mon Sep 17 00:00:00 2001 From: Yordan Madzhunkov Date: Tue, 5 Sep 2023 17:58:43 +0300 Subject: [PATCH 30/32] Rename test impl_trait_for_non_struct -> impl_trait_for_non_type --- .../Nargo.toml | 2 +- .../Prover.toml | 0 .../src/main.nr | 0 3 files changed, 1 insertion(+), 1 deletion(-) rename crates/nargo_cli/tests/compile_failure/{impl_trait_for_non_struct => impl_trait_for_non_type}/Nargo.toml (69%) rename crates/nargo_cli/tests/compile_failure/{impl_trait_for_non_struct => impl_trait_for_non_type}/Prover.toml (100%) rename crates/nargo_cli/tests/compile_failure/{impl_trait_for_non_struct => impl_trait_for_non_type}/src/main.nr (100%) diff --git a/crates/nargo_cli/tests/compile_failure/impl_trait_for_non_struct/Nargo.toml b/crates/nargo_cli/tests/compile_failure/impl_trait_for_non_type/Nargo.toml similarity index 69% rename from crates/nargo_cli/tests/compile_failure/impl_trait_for_non_struct/Nargo.toml rename to crates/nargo_cli/tests/compile_failure/impl_trait_for_non_type/Nargo.toml index 21a44bfd36b..35f174bf546 100644 --- a/crates/nargo_cli/tests/compile_failure/impl_trait_for_non_struct/Nargo.toml +++ b/crates/nargo_cli/tests/compile_failure/impl_trait_for_non_type/Nargo.toml @@ -1,5 +1,5 @@ [package] -name = "impl_trait_for_non_struct" +name = "impl_trait_for_non_type" type = "bin" authors = [""] compiler_version = "0.9.0" diff --git a/crates/nargo_cli/tests/compile_failure/impl_trait_for_non_struct/Prover.toml b/crates/nargo_cli/tests/compile_failure/impl_trait_for_non_type/Prover.toml similarity index 100% rename from crates/nargo_cli/tests/compile_failure/impl_trait_for_non_struct/Prover.toml rename to crates/nargo_cli/tests/compile_failure/impl_trait_for_non_type/Prover.toml diff --git a/crates/nargo_cli/tests/compile_failure/impl_trait_for_non_struct/src/main.nr b/crates/nargo_cli/tests/compile_failure/impl_trait_for_non_type/src/main.nr similarity index 100% rename from crates/nargo_cli/tests/compile_failure/impl_trait_for_non_struct/src/main.nr rename to crates/nargo_cli/tests/compile_failure/impl_trait_for_non_type/src/main.nr From 30a19ff688c15e7014755b67055f10ce68262ea0 Mon Sep 17 00:00:00 2001 From: Yordan Madzhunkov Date: Wed, 6 Sep 2023 14:09:04 +0300 Subject: [PATCH 31/32] Fix cargo fmt --- crates/noirc_frontend/src/node_interner.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/crates/noirc_frontend/src/node_interner.rs b/crates/noirc_frontend/src/node_interner.rs index 7650cd2f538..207250896bc 100644 --- a/crates/noirc_frontend/src/node_interner.rs +++ b/crates/noirc_frontend/src/node_interner.rs @@ -149,7 +149,6 @@ impl FuncId { #[derive(Debug, Eq, PartialEq, Hash, Copy, Clone, PartialOrd, Ord)] pub struct StructId(ModuleId); - impl StructId { //dummy id for error reporting // This can be anything, as the program will ultimately fail From 7e30706a3c2371a82644bbc656fa3528281df47c Mon Sep 17 00:00:00 2001 From: Yordan Madzhunkov Date: Wed, 6 Sep 2023 14:17:24 +0300 Subject: [PATCH 32/32] These tests are expected to execute successfully --- .../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 6 files changed, 0 insertions(+), 0 deletions(-) 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_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