From 49984f748629fccbcac3b840d1fa72ae77ea98b7 Mon Sep 17 00:00:00 2001 From: Yordan Madzhunkov Date: Thu, 17 Aug 2023 15:31:28 +0300 Subject: [PATCH 01/31] Maybe it's better this way --- crates/noirc_frontend/src/hir/resolution/resolver.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/noirc_frontend/src/hir/resolution/resolver.rs b/crates/noirc_frontend/src/hir/resolution/resolver.rs index 7cde9ae231d..f73a3b85166 100644 --- a/crates/noirc_frontend/src/hir/resolution/resolver.rs +++ b/crates/noirc_frontend/src/hir/resolution/resolver.rs @@ -1537,7 +1537,7 @@ mod test { src: &str, ) -> (ParsedModule, NodeInterner, HashMap, FileId, TestPathResolver) { let (program, errors) = parse_program(src); - if !errors.is_empty() { + if errors.iter().any(|e| e.is_error()) { panic!("Unexpected parse errors in test code: {:?}", errors); } From 4a007aabdb2b4f575c74af763cddab39799b0ce1 Mon Sep 17 00:00:00 2001 From: Yordan Madzhunkov Date: Wed, 9 Aug 2023 15:19:15 +0300 Subject: [PATCH 02/31] Collect traits from AST to def_map --- .../src/hir/def_collector/dc_crate.rs | 12 ++++- .../src/hir/def_collector/dc_mod.rs | 49 ++++++++++++++++++- .../src/hir/def_collector/errors.rs | 7 ++- 3 files changed, 62 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 ac83e81ea71..2ae71d070c1 100644 --- a/crates/noirc_frontend/src/hir/def_collector/dc_crate.rs +++ b/crates/noirc_frontend/src/hir/def_collector/dc_crate.rs @@ -10,9 +10,9 @@ use crate::hir::resolution::{ }; use crate::hir::type_check::{type_check_func, TypeChecker}; use crate::hir::Context; -use crate::node_interner::{FuncId, NodeInterner, StmtId, StructId, TypeAliasId}; +use crate::node_interner::{FuncId, NodeInterner, StmtId, StructId, TraitId, TypeAliasId}; use crate::{ - ExpressionKind, Generics, Ident, LetStatement, Literal, NoirFunction, NoirStruct, + ExpressionKind, Generics, Ident, LetStatement, Literal, NoirFunction, NoirStruct, NoirTrait, NoirTypeAlias, ParsedModule, Shared, StructType, Type, TypeBinding, UnresolvedGenerics, UnresolvedType, }; @@ -41,6 +41,12 @@ pub struct UnresolvedStruct { pub struct_def: NoirStruct, } +pub struct UnresolvedTrait { + pub file_id: FileId, + pub module_id: LocalModuleId, + pub trait_def: NoirTrait, +} + #[derive(Clone)] pub struct UnresolvedTypeAlias { pub file_id: FileId, @@ -63,6 +69,7 @@ pub struct DefCollector { pub(crate) collected_functions: Vec, pub(crate) collected_types: HashMap, pub(crate) collected_type_aliases: HashMap, + pub(crate) collected_traits: HashMap, pub(crate) collected_globals: Vec, pub(crate) collected_impls: ImplMap, } @@ -81,6 +88,7 @@ impl DefCollector { collected_functions: vec![], collected_types: HashMap::new(), collected_type_aliases: HashMap::new(), + collected_traits: HashMap::new(), collected_impls: HashMap::new(), collected_globals: vec![], } 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 53ed397e647..a40653d0dbe 100644 --- a/crates/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/crates/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -2,8 +2,11 @@ use fm::FileId; use noirc_errors::{FileDiagnostic, Location}; use crate::{ - graph::CrateId, hir::def_collector::dc_crate::UnresolvedStruct, node_interner::StructId, - parser::SubModule, Ident, LetStatement, NoirFunction, NoirStruct, NoirTypeAlias, ParsedModule, + graph::CrateId, + hir::def_collector::dc_crate::{UnresolvedStruct, UnresolvedTrait}, + node_interner::{StructId, TraitId}, + parser::SubModule, + Ident, LetStatement, NoirFunction, NoirStruct, NoirTrait, NoirTypeAlias, ParsedModule, TypeImpl, }; @@ -54,6 +57,8 @@ pub fn collect_defs( collector.collect_globals(context, ast.globals, errors); + collector.collect_traits(ast.traits, crate_id, errors); + collector.collect_structs(ast.types, crate_id, errors); collector.collect_type_aliases(context, ast.type_aliases, errors); @@ -235,6 +240,46 @@ impl<'a> ModCollector<'a> { } } + /// Collect any traits definitions declared within the ast. + /// Returns a vector of errors if any traits were already defined. + fn collect_traits( + &mut self, + traits: Vec, + krate: CrateId, + errors: &mut Vec, + ) { + for trait_definition in traits { + let name = trait_definition.name.clone(); + + // Create the corresponding module for the trait namespace + let id = match self.push_child_module(&name, self.file_id, false, false, errors) { + Some(local_id) => TraitId(ModuleId { krate, local_id }), + None => continue, + }; + + // Add the trait to scope so its path can be looked up later + let result = + self.def_collector.def_map.modules[self.module_id.0].declare_trait(name, id); + + if let Err((first_def, second_def)) = result { + let err = DefCollectorErrorKind::Duplicate { + typ: DuplicateType::Trait, + first_def, + second_def, + }; + errors.push(err.into_file_diagnostic(self.file_id)); + } + + // And store the TraitId -> TraitType mapping somewhere it is reachable + let unresolved = UnresolvedTrait { + file_id: self.file_id, + module_id: self.module_id, + trait_def: trait_definition, + }; + self.def_collector.collected_traits.insert(id, unresolved); + } + } + fn collect_submodules( &mut self, context: &mut Context, diff --git a/crates/noirc_frontend/src/hir/def_collector/errors.rs b/crates/noirc_frontend/src/hir/def_collector/errors.rs index d47d985e12e..8e6f4ec39f3 100644 --- a/crates/noirc_frontend/src/hir/def_collector/errors.rs +++ b/crates/noirc_frontend/src/hir/def_collector/errors.rs @@ -15,6 +15,7 @@ pub enum DuplicateType { Global, TypeDefinition, Import, + Trait, } #[derive(Error, Debug)] @@ -44,7 +45,9 @@ impl fmt::Display for DuplicateType { DuplicateType::Module => write!(f, "module"), DuplicateType::Global => write!(f, "global"), DuplicateType::TypeDefinition => write!(f, "type definition"), + DuplicateType::Trait => write!(f, "trait definition"), DuplicateType::Import => write!(f, "import"), + DuplicateType::Trait => write!(f, "trait"), } } } @@ -62,10 +65,10 @@ impl From for Diagnostic { let second_span = second_def.0.span(); let mut diag = Diagnostic::simple_error( primary_message, - format!("first {:?} found here", &typ), + format!("first {} found here", &typ), first_span, ); - diag.add_secondary(format!("second {:?} found here", &typ), second_span); + diag.add_secondary(format!("second {} found here", &typ), second_span); diag } } From 29094f0992fadeb6792dc4a19e40673e7eb0e3e7 Mon Sep 17 00:00:00 2001 From: Yordan Madzhunkov Date: Wed, 9 Aug 2023 15:47:57 +0300 Subject: [PATCH 03/31] Collect trait implementations in def collector --- .../fail/dup_trait_declaration.nr | 26 ++ .../fail/dup_trait_implementation.nr | 30 +++ .../fail/impl_struct_not_trait.nr | 23 ++ .../trait_implementation_wrong_parameter.nr | 21 ++ .../fail/trait_not_exists.nr | 18 ++ .../fail/trait_wrong_method_name.nr | 22 ++ .../fail/trait_wrong_method_return_type.nr | 21 ++ .../fail/trait_wrong_parameter.nr | 21 ++ .../fail/trait_wrong_parameters_count.nr | 21 ++ .../src/hir/def_collector/dc_mod.rs | 236 +++++++++++++++++- .../src/hir/def_collector/errors.rs | 36 ++- 11 files changed, 471 insertions(+), 4 deletions(-) create mode 100644 crates/nargo_cli/tests/compile_tests_data/fail/dup_trait_declaration.nr create mode 100644 crates/nargo_cli/tests/compile_tests_data/fail/dup_trait_implementation.nr create mode 100644 crates/nargo_cli/tests/compile_tests_data/fail/impl_struct_not_trait.nr create mode 100644 crates/nargo_cli/tests/compile_tests_data/fail/trait_implementation_wrong_parameter.nr create mode 100644 crates/nargo_cli/tests/compile_tests_data/fail/trait_not_exists.nr create mode 100644 crates/nargo_cli/tests/compile_tests_data/fail/trait_wrong_method_name.nr create mode 100644 crates/nargo_cli/tests/compile_tests_data/fail/trait_wrong_method_return_type.nr create mode 100644 crates/nargo_cli/tests/compile_tests_data/fail/trait_wrong_parameter.nr create mode 100644 crates/nargo_cli/tests/compile_tests_data/fail/trait_wrong_parameters_count.nr diff --git a/crates/nargo_cli/tests/compile_tests_data/fail/dup_trait_declaration.nr b/crates/nargo_cli/tests/compile_tests_data/fail/dup_trait_declaration.nr new file mode 100644 index 00000000000..f4c246c786a --- /dev/null +++ b/crates/nargo_cli/tests/compile_tests_data/fail/dup_trait_declaration.nr @@ -0,0 +1,26 @@ +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) -> Self { + Self { bar: x, array: [x,y] } + } +} + +// Duplicate trait declarations should not compile +trait Default { + fn default(x: Field) -> Self; +} + +fn main(x: Field, y: Field) { + let first = Foo::default(x,y); + assert(first.bar == x); +} diff --git a/crates/nargo_cli/tests/compile_tests_data/fail/dup_trait_implementation.nr b/crates/nargo_cli/tests/compile_tests_data/fail/dup_trait_implementation.nr new file mode 100644 index 00000000000..6bb6cedefb5 --- /dev/null +++ b/crates/nargo_cli/tests/compile_tests_data/fail/dup_trait_implementation.nr @@ -0,0 +1,30 @@ +use dep::std; + +trait Default { + fn default(x: Field, y: Field) -> Self; +} + +struct Foo { + bar: Field, + array: [Field; 2], +} + +// Duplicate trait implementations should not compile +impl Default for Foo { + fn default(x: Field,y: Field) -> Self { + Self { bar: x, array: [x,y] } + } +} + +// Duplicate trait implementations should not compile +impl Default for Foo { + fn default(x: Field, y: Field) -> Self { + Self { bar: y, array: [y,x] } + } +} + + +fn main(x: Field, y: Field) { + let first = Foo::default(x,y); + assert(first.bar == x); +} diff --git a/crates/nargo_cli/tests/compile_tests_data/fail/impl_struct_not_trait.nr b/crates/nargo_cli/tests/compile_tests_data/fail/impl_struct_not_trait.nr new file mode 100644 index 00000000000..e25465378b1 --- /dev/null +++ b/crates/nargo_cli/tests/compile_tests_data/fail/impl_struct_not_trait.nr @@ -0,0 +1,23 @@ +use dep::std; + +struct Foo { + bar: Field, + array: [Field; 2], +} + +struct Default { + x: Field, + z: Field, +} + +// Default is struct not a trait +impl Default for Foo { + fn default(x: Field, y: Field) -> Self { + Self { bar: x, array: [x,y] } + } +} + +fn main(x: Field, y: Field) { + let first = Foo::default(x,y); + assert(first.bar == x); +} diff --git a/crates/nargo_cli/tests/compile_tests_data/fail/trait_implementation_wrong_parameter.nr b/crates/nargo_cli/tests/compile_tests_data/fail/trait_implementation_wrong_parameter.nr new file mode 100644 index 00000000000..92469ae8fdb --- /dev/null +++ b/crates/nargo_cli/tests/compile_tests_data/fail/trait_implementation_wrong_parameter.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) -> Self { + Self { bar: x, array: [x, x] } + } +} + +fn main(x: Field, y: Field) { + let first = Foo::default(x,y); + assert(first.bar == x); +} diff --git a/crates/nargo_cli/tests/compile_tests_data/fail/trait_not_exists.nr b/crates/nargo_cli/tests/compile_tests_data/fail/trait_not_exists.nr new file mode 100644 index 00000000000..9dc57ee395f --- /dev/null +++ b/crates/nargo_cli/tests/compile_tests_data/fail/trait_not_exists.nr @@ -0,0 +1,18 @@ +use dep::std; + +struct Foo { + bar: Field, + array: [Field; 2], +} + +// Default trait does not exist +impl Default for Foo { + fn default(x: Field, y: Field) -> Self { + Self { bar: x, array: [x,y] } + } +} + +fn main(x: Field, y: Field) { + let first = Foo::default(x,y); + assert(first.bar == x); +} diff --git a/crates/nargo_cli/tests/compile_tests_data/fail/trait_wrong_method_name.nr b/crates/nargo_cli/tests/compile_tests_data/fail/trait_wrong_method_name.nr new file mode 100644 index 00000000000..0ba10815efa --- /dev/null +++ b/crates/nargo_cli/tests/compile_tests_data/fail/trait_wrong_method_name.nr @@ -0,0 +1,22 @@ +use dep::std; + +trait Default { + fn default(x: Field, y: Field) -> Self; +} + +struct Foo { + bar: Field, + array: [Field; 2], +} + +// wrong trait name method should not compile +impl Default for Foo { + fn default_wrong_name(x: Field, y: Field) -> Self { + Self { bar: x, array: [x,y] } + } +} + +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_tests_data/fail/trait_wrong_method_return_type.nr b/crates/nargo_cli/tests/compile_tests_data/fail/trait_wrong_method_return_type.nr new file mode 100644 index 00000000000..acd930a6d49 --- /dev/null +++ b/crates/nargo_cli/tests/compile_tests_data/fail/trait_wrong_method_return_type.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.bar == x); +} diff --git a/crates/nargo_cli/tests/compile_tests_data/fail/trait_wrong_parameter.nr b/crates/nargo_cli/tests/compile_tests_data/fail/trait_wrong_parameter.nr new file mode 100644 index 00000000000..2975aa6b1dd --- /dev/null +++ b/crates/nargo_cli/tests/compile_tests_data/fail/trait_wrong_parameter.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: Foo) -> Self { + Self { bar: x, array: [x, y.bar] } + } +} + +fn main(x: Field, y: Field) { + let first = Foo::default(x,y); + assert(first.bar == x); +} diff --git a/crates/nargo_cli/tests/compile_tests_data/fail/trait_wrong_parameters_count.nr b/crates/nargo_cli/tests/compile_tests_data/fail/trait_wrong_parameters_count.nr new file mode 100644 index 00000000000..92469ae8fdb --- /dev/null +++ b/crates/nargo_cli/tests/compile_tests_data/fail/trait_wrong_parameters_count.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) -> Self { + Self { bar: x, array: [x, x] } + } +} + +fn main(x: Field, y: Field) { + let first = Foo::default(x,y); + assert(first.bar == x); +} 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 a40653d0dbe..7640d1aa7e0 100644 --- a/crates/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/crates/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -6,15 +6,15 @@ use crate::{ hir::def_collector::dc_crate::{UnresolvedStruct, UnresolvedTrait}, node_interner::{StructId, TraitId}, parser::SubModule, - Ident, LetStatement, NoirFunction, NoirStruct, NoirTrait, NoirTypeAlias, ParsedModule, - TypeImpl, + FunctionReturnType, Ident, LetStatement, NoirFunction, NoirStruct, NoirTrait, NoirTypeAlias, + ParsedModule, TraitConstraint, TraitImpl, TraitImplItem, TraitItem, TypeImpl, UnresolvedType, }; use super::{ dc_crate::{DefCollector, UnresolvedFunctions, UnresolvedGlobal, UnresolvedTypeAlias}, errors::{DefCollectorErrorKind, DuplicateType}, }; -use crate::hir::def_map::{parse_file, LocalModuleId, ModuleData, ModuleId}; +use crate::hir::def_map::{parse_file, LocalModuleId, ModuleData, ModuleDefId, ModuleId}; use crate::hir::resolution::import::ImportDirective; use crate::hir::Context; @@ -65,9 +65,141 @@ pub fn collect_defs( collector.collect_functions(context, ast.functions, errors); + collector.collect_trait_impls(context, ast.trait_impls, errors); + collector.collect_impls(context, ast.impls); } +fn check_trait_method_implementation_generics( + _generics: &[Ident], + _noir_function: &NoirFunction, + _trait_name: &str, +) -> Result<(), DefCollectorErrorKind> { + // TODO + Ok(()) +} + +fn check_trait_method_implementation_parameters( + parameters: &Vec<(Ident, UnresolvedType)>, + noir_function: &NoirFunction, + trait_name: &String, +) -> Result<(), DefCollectorErrorKind> { + let expected_num_parameters = parameters.len(); + let actual_num_parameters = noir_function.def.parameters.len(); + if actual_num_parameters != expected_num_parameters { + let method_name = noir_function.name(); + let primary_message = format!( + "Mismatch - expected {expected_num_parameters} arguments, but got {actual_num_parameters} of trait `{trait_name}` implementation `{method_name}`"); + return Err(DefCollectorErrorKind::MismatchTraitSignature { + primary_message, + secondary_message: "".to_string(), + span: noir_function.name_ident().span(), + }); + } + for (count, (pattern, typ, _abi_vis)) in noir_function.def.parameters.iter().enumerate() { + let (_expected_name, expected_type) = ¶meters[count]; + if typ != expected_type { + return Err(DefCollectorErrorKind::MismatchTraitSignature { + primary_message: format!( + "Mismatch signature of method {} that implemtns trait {}", + noir_function.name(), + trait_name, + ), + secondary_message: format!( + "`{}: {}` expected", + pattern.name_ident().0.contents, + expected_type, + ), + span: pattern.name_ident().span(), + }); + } + } + Ok(()) +} + +fn check_trait_method_implementation_trait_constains( + _where_clause: &[TraitConstraint], + _noir_function: &NoirFunction, + _trait_name: &str, +) -> Result<(), DefCollectorErrorKind> { + // TODO + Ok(()) +} + +fn check_trait_method_implementation_return_type( + return_type: &FunctionReturnType, + noir_function: &NoirFunction, + trait_name: &String, +) -> Result<(), DefCollectorErrorKind> { + match return_type { + FunctionReturnType::Default(_span) => Ok(()), + FunctionReturnType::Ty(typ, _span) => { + if !(typ == &noir_function.return_type()) { + Err(DefCollectorErrorKind::MismatchTraitSignature { + primary_message: format!( + "Mismatch return type of method with name {} that implements trait {}", + noir_function.name(), + trait_name + ), + secondary_message: "".to_string(), + span: noir_function.name_ident().span(), + }) + } else { + Ok(()) + } + } + } +} + +fn check_trait_method_implementation( + r#trait: &NoirTrait, + noir_function: &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 == noir_function.def.name.0.contents { + // name matches, check for parameters, return type and where clause + check_trait_method_implementation_generics( + generics, + noir_function, + &r#trait.name.0.contents, + )?; + check_trait_method_implementation_parameters( + parameters, + noir_function, + &r#trait.name.0.contents, + )?; + check_trait_method_implementation_trait_constains( + where_clause, + noir_function, + &r#trait.name.0.contents, + )?; + check_trait_method_implementation_return_type( + return_type, + noir_function, + &r#trait.name.0.contents, + )?; + return Ok(()); + } + } + } + + Err(DefCollectorErrorKind::MethodNotInTrait { + trait_name: r#trait.name.0.contents.to_string(), + trait_span: r#trait.name.span(), + impl_method_name: noir_function.name().to_string(), + impl_method_span: noir_function.name_ident().span(), + }) +} + impl<'a> ModCollector<'a> { fn collect_globals( &mut self, @@ -121,6 +253,104 @@ impl<'a> ModCollector<'a> { } } + fn collect_trait_impls( + &mut self, + context: &mut Context, + impls: Vec, + errors: &mut Vec, + ) { + for trait_impl in impls { + let trait_name = trait_impl.trait_name.clone(); + let module = &self.def_collector.def_map.modules[self.module_id.0]; + match module.find_name(&trait_name).types { + Some((module_def_id, _visibility)) => self.collect_trait_from_module( + context, + module_def_id, + trait_impl, + errors, + &trait_name, + ), + None => { + let error = DefCollectorErrorKind::TraitNotFound { + trait_name: trait_name.to_string(), + span: trait_name.span(), + }; + errors.push(error.into_file_diagnostic(self.file_id)); + } + } + } + } + + fn get_unresolved_trait(&self, module_def_id: ModuleDefId) -> Option<&UnresolvedTrait> { + match module_def_id { + ModuleDefId::TraitId(trait_id) => self.def_collector.collected_traits.get(&trait_id), + _ => None, + } + } + + fn collect_trait_from_module( + &mut self, + context: &mut Context, + module_def_id: ModuleDefId, + trait_impl: TraitImpl, + errors: &mut Vec, + trait_name: &Ident, + ) { + if let Some(collected_trait) = self.get_unresolved_trait(module_def_id) { + let trait_def = collected_trait.trait_def.clone(); + 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 collected_implementations = + self.collect_trait_implementations(context, &trait_impl, &trait_def, errors); + let key = (impl_object_type, self.module_id); + self.def_collector.collected_impls.entry(key).or_default().push(( + impl_generics, + impl_type_span, + collected_implementations, + )); + } else { + let error = DefCollectorErrorKind::NotATrait { + primary_message: format!( + "{} is not a trait, therefore it can't be implemented", + trait_name + ), + secondary_message: "".to_string(), + span: trait_name.span(), + }; + errors.push(error.into_file_diagnostic(self.file_id)); + } + } + + fn collect_trait_implementations( + &mut self, + context: &mut Context, + trait_impl: &TraitImpl, + trait_def: &NoirTrait, + errors: &mut Vec, + ) -> UnresolvedFunctions { + let mut unresolved_functions = + UnresolvedFunctions { file_id: self.file_id, functions: Vec::new() }; + for item in &trait_impl.items { + if let TraitImplItem::Function(noir_function) = item { + match check_trait_method_implementation(trait_def, &noir_function) { + Ok(()) => { + let func_id = context.def_interner.push_empty_fn(); + context + .def_interner + .push_function_definition(noir_function.name().to_owned(), func_id); + unresolved_functions.push_fn(self.module_id, func_id, noir_function.clone()); + } + Err(error) => { + errors.push(error.into_file_diagnostic(self.file_id)); + } + } + } else { + } + } + unresolved_functions + } + fn collect_functions( &mut self, context: &mut Context, diff --git a/crates/noirc_frontend/src/hir/def_collector/errors.rs b/crates/noirc_frontend/src/hir/def_collector/errors.rs index 8e6f4ec39f3..46971d68612 100644 --- a/crates/noirc_frontend/src/hir/def_collector/errors.rs +++ b/crates/noirc_frontend/src/hir/def_collector/errors.rs @@ -30,6 +30,19 @@ pub enum DefCollectorErrorKind { NonStructTypeInImpl { span: Span }, #[error("Cannot `impl` a type defined outside the current crate")] ForeignImpl { span: Span, type_name: String }, + #[error("Mismatch signature of trait")] + MismatchTraitSignature { primary_message: String, secondary_message: String, span: Span }, + #[error("Method is not defined in trait")] + MethodNotInTrait { + trait_name: String, + trait_span: Span, + impl_method_name: String, + impl_method_span: Span, + }, + #[error("Not a trait")] + NotATrait { primary_message: String, secondary_message: String, span: Span }, + #[error("Trait {trait_name} not found")] + TraitNotFound { trait_name: String, span: Span }, } impl DefCollectorErrorKind { @@ -47,7 +60,6 @@ impl fmt::Display for DuplicateType { DuplicateType::TypeDefinition => write!(f, "type definition"), DuplicateType::Trait => write!(f, "trait definition"), DuplicateType::Import => write!(f, "import"), - DuplicateType::Trait => write!(f, "trait"), } } } @@ -93,6 +105,28 @@ impl From for Diagnostic { format!("{type_name} was defined outside the current crate"), span, ), + DefCollectorErrorKind::TraitNotFound { trait_name, span } => Diagnostic::simple_error( + format!("Trait {} not found", trait_name), + "".to_string(), + span, + ), + DefCollectorErrorKind::MismatchTraitSignature { + primary_message, + secondary_message, + span, + } => Diagnostic::simple_error(primary_message, secondary_message, span), + DefCollectorErrorKind::MethodNotInTrait { + trait_name, + trait_span: _, + impl_method_name, + impl_method_span, + } => { + 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::NotATrait { primary_message, secondary_message, span } => { + Diagnostic::simple_error(primary_message, secondary_message, span) + } } } } From 5d00169c537bb242881189523e29a57089aa3faf Mon Sep 17 00:00:00 2001 From: Yordan Madzhunkov Date: Wed, 9 Aug 2023 16:00:16 +0300 Subject: [PATCH 04/31] Resolve traits --- .../tests/test_data/traits/Nargo.toml | 6 +++++ .../tests/test_data/traits/Prover.toml | 2 ++ .../tests/test_data/traits/src/main.nr | 21 +++++++++++++++++ .../src/hir/def_collector/dc_crate.rs | 15 +++++++++++- crates/noirc_frontend/src/node_interner.rs | 23 ++++++++++++++++++- 5 files changed, 65 insertions(+), 2 deletions(-) create mode 100644 crates/nargo_cli/tests/test_data/traits/Nargo.toml create mode 100644 crates/nargo_cli/tests/test_data/traits/Prover.toml create mode 100644 crates/nargo_cli/tests/test_data/traits/src/main.nr diff --git a/crates/nargo_cli/tests/test_data/traits/Nargo.toml b/crates/nargo_cli/tests/test_data/traits/Nargo.toml new file mode 100644 index 00000000000..629f55ef064 --- /dev/null +++ b/crates/nargo_cli/tests/test_data/traits/Nargo.toml @@ -0,0 +1,6 @@ +[package] +name = "traits" +authors = [""] +compiler_version = "0.1" + +[dependencies] diff --git a/crates/nargo_cli/tests/test_data/traits/Prover.toml b/crates/nargo_cli/tests/test_data/traits/Prover.toml new file mode 100644 index 00000000000..71805e71e8e --- /dev/null +++ b/crates/nargo_cli/tests/test_data/traits/Prover.toml @@ -0,0 +1,2 @@ +x = "5" +y = "1" \ No newline at end of file diff --git a/crates/nargo_cli/tests/test_data/traits/src/main.nr b/crates/nargo_cli/tests/test_data/traits/src/main.nr new file mode 100644 index 00000000000..2333c5da244 --- /dev/null +++ b/crates/nargo_cli/tests/test_data/traits/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) -> Self { + Self { bar: x, array: [x,y] } + } +} + +fn main(x: Field, y: Field) { + let first = Foo::default(x,y); + assert(first.bar == x); +} 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 2ae71d070c1..34a7afa7cd7 100644 --- a/crates/noirc_frontend/src/hir/def_collector/dc_crate.rs +++ b/crates/noirc_frontend/src/hir/def_collector/dc_crate.rs @@ -182,6 +182,7 @@ impl DefCollector { resolve_type_aliases(context, def_collector.collected_type_aliases, crate_id, errors); // Must resolve structs before we resolve globals. + resolve_traits(context, def_collector.collected_traits, crate_id, errors); resolve_structs(context, def_collector.collected_types, crate_id, errors); // We must wait to resolve non-integer globals until after we resolve structs since structs @@ -380,6 +381,19 @@ fn resolve_structs( } } +/// Create the mappings from TypeId -> TraitType +/// so that expressions can access the elements of traits +fn resolve_traits( + context: &mut Context, + traits: HashMap, + _crate_id: CrateId, + _errors: &mut [FileDiagnostic], +) { + for (type_id, typ) in &traits { + context.def_interner.push_empty_trait(*type_id, typ); + } +} + fn resolve_struct_fields( context: &mut Context, krate: CrateId, @@ -450,7 +464,6 @@ fn resolve_impls( generics, errors, ); - if self_type != Type::Error { for (file_id, method_id) in &file_func_ids { let method_name = interner.function_name(method_id).to_owned(); diff --git a/crates/noirc_frontend/src/node_interner.rs b/crates/noirc_frontend/src/node_interner.rs index 5a45cfee42b..78397a9ac6f 100644 --- a/crates/noirc_frontend/src/node_interner.rs +++ b/crates/noirc_frontend/src/node_interner.rs @@ -7,7 +7,7 @@ use noirc_errors::{Location, Span, Spanned}; use crate::ast::Ident; use crate::graph::CrateId; -use crate::hir::def_collector::dc_crate::{UnresolvedStruct, UnresolvedTypeAlias}; +use crate::hir::def_collector::dc_crate::{UnresolvedStruct, UnresolvedTrait, UnresolvedTypeAlias}; use crate::hir::def_map::{LocalModuleId, ModuleId}; use crate::hir::StorageSlot; use crate::hir_def::stmt::HirLetStatement; @@ -70,6 +70,7 @@ pub struct NodeInterner { // TODO: We may be able to remove the Shared wrapper once traits are no longer types. // We'd just lookup their methods as needed through the NodeInterner. traits: HashMap>, + /// 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. @@ -326,6 +327,26 @@ impl NodeInterner { self.id_to_type.insert(expr_id.into(), typ); } + pub fn push_empty_trait(&mut self, type_id: TraitId, typ: &UnresolvedTrait) { + self.traits.insert( + type_id, + Shared::new(Trait::new( + 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 + // can refer to it with generic arguments before the generic parameters themselves + // are resolved. + let id = TypeVariableId(0); + (id, Shared::new(TypeBinding::Unbound(id))) + }), + )), + ); + } + pub fn push_empty_struct(&mut self, type_id: StructId, typ: &UnresolvedStruct) { self.structs.insert( type_id, From a6615f22ae8107c290e946d3073fd09eb6bdc72f Mon Sep 17 00:00:00 2001 From: Yordan Madzhunkov Date: Thu, 17 Aug 2023 15:31:09 +0300 Subject: [PATCH 05/31] Separate collected trait implementations --- .../src/hir/def_collector/dc_crate.rs | 14 ++++++++++++++ .../noirc_frontend/src/hir/def_collector/dc_mod.rs | 2 +- 2 files changed, 15 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 34a7afa7cd7..0145591174c 100644 --- a/crates/noirc_frontend/src/hir/def_collector/dc_crate.rs +++ b/crates/noirc_frontend/src/hir/def_collector/dc_crate.rs @@ -72,6 +72,8 @@ pub struct DefCollector { pub(crate) collected_traits: HashMap, pub(crate) collected_globals: Vec, pub(crate) collected_impls: ImplMap, + pub(crate) collected_traits_impls: ImplMap, + } /// Maps the type and the module id in which the impl is defined to the functions contained in that @@ -90,6 +92,7 @@ impl DefCollector { collected_type_aliases: HashMap::new(), collected_traits: HashMap::new(), collected_impls: HashMap::new(), + collected_traits_impls: HashMap::new(), collected_globals: vec![], } } @@ -197,6 +200,8 @@ 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); + // Lower each function in the crate. This is now possible since imports have been resolved let file_func_ids = resolve_free_functions( &mut context.def_interner, @@ -215,10 +220,19 @@ 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, + ); + type_check_globals(&mut context.def_interner, file_global_ids, errors); // 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); } } 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 7640d1aa7e0..02cef6d5813 100644 --- a/crates/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/crates/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -304,7 +304,7 @@ impl<'a> ModCollector<'a> { let collected_implementations = self.collect_trait_implementations(context, &trait_impl, &trait_def, errors); let key = (impl_object_type, self.module_id); - self.def_collector.collected_impls.entry(key).or_default().push(( + self.def_collector.collected_traits_impls.entry(key).or_default().push(( impl_generics, impl_type_span, collected_implementations, From 568119f6f6814485b1452e8688d43caafcce35c0 Mon Sep 17 00:00:00 2001 From: Nikolay Nikolov Date: Tue, 15 Aug 2023 21:56:46 +0300 Subject: [PATCH 06/31] fix(testsuite): converted traits tests to proper nargo packages Converted the traits-related tests that are supposed to fail, to the proper format (nargo packages) in the crates/nargo_cli/tests/compile_failure directory. This ensures that the tests are now executed when 'cargo test' is run. --- .../tests/compile_failure/dup_trait_declaration/Nargo.toml | 7 +++++++ .../dup_trait_declaration/src/main.nr} | 0 .../compile_failure/dup_trait_implementation/Nargo.toml | 7 +++++++ .../dup_trait_implementation/src/main.nr} | 0 .../tests/compile_failure/impl_struct_not_trait/Nargo.toml | 7 +++++++ .../impl_struct_not_trait/src/main.nr} | 0 .../trait_implementation_wrong_parameter/Nargo.toml | 7 +++++++ .../trait_implementation_wrong_parameter/src/main.nr} | 0 .../tests/compile_failure/trait_not_exists/Nargo.toml | 7 +++++++ .../trait_not_exists/src/main.nr} | 0 .../compile_failure/trait_wrong_method_name/Nargo.toml | 7 +++++++ .../trait_wrong_method_name/src/main.nr} | 0 .../trait_wrong_method_return_type/Nargo.toml | 7 +++++++ .../trait_wrong_method_return_type/src/main.nr} | 0 .../tests/compile_failure/trait_wrong_parameter/Nargo.toml | 7 +++++++ .../trait_wrong_parameter/src/main.nr} | 0 .../trait_wrong_parameters_count/Nargo.toml | 7 +++++++ .../trait_wrong_parameters_count/src/main.nr} | 0 18 files changed, 63 insertions(+) create mode 100644 crates/nargo_cli/tests/compile_failure/dup_trait_declaration/Nargo.toml rename crates/nargo_cli/tests/{compile_tests_data/fail/dup_trait_declaration.nr => compile_failure/dup_trait_declaration/src/main.nr} (100%) create mode 100644 crates/nargo_cli/tests/compile_failure/dup_trait_implementation/Nargo.toml rename crates/nargo_cli/tests/{compile_tests_data/fail/dup_trait_implementation.nr => compile_failure/dup_trait_implementation/src/main.nr} (100%) create mode 100644 crates/nargo_cli/tests/compile_failure/impl_struct_not_trait/Nargo.toml rename crates/nargo_cli/tests/{compile_tests_data/fail/impl_struct_not_trait.nr => compile_failure/impl_struct_not_trait/src/main.nr} (100%) create mode 100644 crates/nargo_cli/tests/compile_failure/trait_implementation_wrong_parameter/Nargo.toml rename crates/nargo_cli/tests/{compile_tests_data/fail/trait_implementation_wrong_parameter.nr => compile_failure/trait_implementation_wrong_parameter/src/main.nr} (100%) create mode 100644 crates/nargo_cli/tests/compile_failure/trait_not_exists/Nargo.toml rename crates/nargo_cli/tests/{compile_tests_data/fail/trait_not_exists.nr => compile_failure/trait_not_exists/src/main.nr} (100%) create mode 100644 crates/nargo_cli/tests/compile_failure/trait_wrong_method_name/Nargo.toml rename crates/nargo_cli/tests/{compile_tests_data/fail/trait_wrong_method_name.nr => compile_failure/trait_wrong_method_name/src/main.nr} (100%) create mode 100644 crates/nargo_cli/tests/compile_failure/trait_wrong_method_return_type/Nargo.toml rename crates/nargo_cli/tests/{compile_tests_data/fail/trait_wrong_method_return_type.nr => compile_failure/trait_wrong_method_return_type/src/main.nr} (100%) create mode 100644 crates/nargo_cli/tests/compile_failure/trait_wrong_parameter/Nargo.toml rename crates/nargo_cli/tests/{compile_tests_data/fail/trait_wrong_parameter.nr => compile_failure/trait_wrong_parameter/src/main.nr} (100%) create mode 100644 crates/nargo_cli/tests/compile_failure/trait_wrong_parameters_count/Nargo.toml rename crates/nargo_cli/tests/{compile_tests_data/fail/trait_wrong_parameters_count.nr => compile_failure/trait_wrong_parameters_count/src/main.nr} (100%) diff --git a/crates/nargo_cli/tests/compile_failure/dup_trait_declaration/Nargo.toml b/crates/nargo_cli/tests/compile_failure/dup_trait_declaration/Nargo.toml new file mode 100644 index 00000000000..214116185f4 --- /dev/null +++ b/crates/nargo_cli/tests/compile_failure/dup_trait_declaration/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "dup_trait_declaration" +type = "bin" +authors = [""] +compiler_version = "0.9.0" + +[dependencies] \ No newline at end of file diff --git a/crates/nargo_cli/tests/compile_tests_data/fail/dup_trait_declaration.nr b/crates/nargo_cli/tests/compile_failure/dup_trait_declaration/src/main.nr similarity index 100% rename from crates/nargo_cli/tests/compile_tests_data/fail/dup_trait_declaration.nr rename to crates/nargo_cli/tests/compile_failure/dup_trait_declaration/src/main.nr diff --git a/crates/nargo_cli/tests/compile_failure/dup_trait_implementation/Nargo.toml b/crates/nargo_cli/tests/compile_failure/dup_trait_implementation/Nargo.toml new file mode 100644 index 00000000000..708e26777d6 --- /dev/null +++ b/crates/nargo_cli/tests/compile_failure/dup_trait_implementation/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "dup_trait_implementation" +type = "bin" +authors = [""] +compiler_version = "0.9.0" + +[dependencies] \ No newline at end of file diff --git a/crates/nargo_cli/tests/compile_tests_data/fail/dup_trait_implementation.nr b/crates/nargo_cli/tests/compile_failure/dup_trait_implementation/src/main.nr similarity index 100% rename from crates/nargo_cli/tests/compile_tests_data/fail/dup_trait_implementation.nr rename to crates/nargo_cli/tests/compile_failure/dup_trait_implementation/src/main.nr diff --git a/crates/nargo_cli/tests/compile_failure/impl_struct_not_trait/Nargo.toml b/crates/nargo_cli/tests/compile_failure/impl_struct_not_trait/Nargo.toml new file mode 100644 index 00000000000..3c6943f1ce1 --- /dev/null +++ b/crates/nargo_cli/tests/compile_failure/impl_struct_not_trait/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "impl_struct_not_trait" +type = "bin" +authors = [""] +compiler_version = "0.9.0" + +[dependencies] \ No newline at end of file diff --git a/crates/nargo_cli/tests/compile_tests_data/fail/impl_struct_not_trait.nr b/crates/nargo_cli/tests/compile_failure/impl_struct_not_trait/src/main.nr similarity index 100% rename from crates/nargo_cli/tests/compile_tests_data/fail/impl_struct_not_trait.nr rename to crates/nargo_cli/tests/compile_failure/impl_struct_not_trait/src/main.nr diff --git a/crates/nargo_cli/tests/compile_failure/trait_implementation_wrong_parameter/Nargo.toml b/crates/nargo_cli/tests/compile_failure/trait_implementation_wrong_parameter/Nargo.toml new file mode 100644 index 00000000000..ef62b1e6c3a --- /dev/null +++ b/crates/nargo_cli/tests/compile_failure/trait_implementation_wrong_parameter/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "trait_implementation_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_tests_data/fail/trait_implementation_wrong_parameter.nr b/crates/nargo_cli/tests/compile_failure/trait_implementation_wrong_parameter/src/main.nr similarity index 100% rename from crates/nargo_cli/tests/compile_tests_data/fail/trait_implementation_wrong_parameter.nr rename to crates/nargo_cli/tests/compile_failure/trait_implementation_wrong_parameter/src/main.nr diff --git a/crates/nargo_cli/tests/compile_failure/trait_not_exists/Nargo.toml b/crates/nargo_cli/tests/compile_failure/trait_not_exists/Nargo.toml new file mode 100644 index 00000000000..6c8927de9c6 --- /dev/null +++ b/crates/nargo_cli/tests/compile_failure/trait_not_exists/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "trait_not_exists" +type = "bin" +authors = [""] +compiler_version = "0.9.0" + +[dependencies] \ No newline at end of file diff --git a/crates/nargo_cli/tests/compile_tests_data/fail/trait_not_exists.nr b/crates/nargo_cli/tests/compile_failure/trait_not_exists/src/main.nr similarity index 100% rename from crates/nargo_cli/tests/compile_tests_data/fail/trait_not_exists.nr rename to crates/nargo_cli/tests/compile_failure/trait_not_exists/src/main.nr diff --git a/crates/nargo_cli/tests/compile_failure/trait_wrong_method_name/Nargo.toml b/crates/nargo_cli/tests/compile_failure/trait_wrong_method_name/Nargo.toml new file mode 100644 index 00000000000..c84f1f3c1c7 --- /dev/null +++ b/crates/nargo_cli/tests/compile_failure/trait_wrong_method_name/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "trait_wrong_method_name" +type = "bin" +authors = [""] +compiler_version = "0.9.0" + +[dependencies] \ No newline at end of file diff --git a/crates/nargo_cli/tests/compile_tests_data/fail/trait_wrong_method_name.nr b/crates/nargo_cli/tests/compile_failure/trait_wrong_method_name/src/main.nr similarity index 100% rename from crates/nargo_cli/tests/compile_tests_data/fail/trait_wrong_method_name.nr rename to crates/nargo_cli/tests/compile_failure/trait_wrong_method_name/src/main.nr diff --git a/crates/nargo_cli/tests/compile_failure/trait_wrong_method_return_type/Nargo.toml b/crates/nargo_cli/tests/compile_failure/trait_wrong_method_return_type/Nargo.toml new file mode 100644 index 00000000000..95e3e222ca3 --- /dev/null +++ b/crates/nargo_cli/tests/compile_failure/trait_wrong_method_return_type/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_tests_data/fail/trait_wrong_method_return_type.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/compile_tests_data/fail/trait_wrong_method_return_type.nr rename to crates/nargo_cli/tests/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/compile_failure/trait_wrong_parameter/Nargo.toml new file mode 100644 index 00000000000..7299ec69e7a --- /dev/null +++ b/crates/nargo_cli/tests/compile_failure/trait_wrong_parameter/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_tests_data/fail/trait_wrong_parameter.nr b/crates/nargo_cli/tests/compile_failure/trait_wrong_parameter/src/main.nr similarity index 100% rename from crates/nargo_cli/tests/compile_tests_data/fail/trait_wrong_parameter.nr rename to crates/nargo_cli/tests/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/compile_failure/trait_wrong_parameters_count/Nargo.toml new file mode 100644 index 00000000000..a60cf09e828 --- /dev/null +++ b/crates/nargo_cli/tests/compile_failure/trait_wrong_parameters_count/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "trait_wrong_parameters_count" +type = "bin" +authors = [""] +compiler_version = "0.9.0" + +[dependencies] \ No newline at end of file diff --git a/crates/nargo_cli/tests/compile_tests_data/fail/trait_wrong_parameters_count.nr b/crates/nargo_cli/tests/compile_failure/trait_wrong_parameters_count/src/main.nr similarity index 100% rename from crates/nargo_cli/tests/compile_tests_data/fail/trait_wrong_parameters_count.nr rename to crates/nargo_cli/tests/compile_failure/trait_wrong_parameters_count/src/main.nr From 788ef0715d0783fffac16314fd23f3523019e193 Mon Sep 17 00:00:00 2001 From: Nikolay Nikolov Date: Wed, 16 Aug 2023 00:42:58 +0300 Subject: [PATCH 07/31] fix(testsuite): added Prover.toml files to the traits compile_failure tests --- .../tests/compile_failure/dup_trait_declaration/Prover.toml | 2 ++ .../tests/compile_failure/dup_trait_implementation/Prover.toml | 2 ++ .../tests/compile_failure/impl_struct_not_trait/Prover.toml | 2 ++ .../tests/compile_failure/trait_not_exists/Prover.toml | 2 ++ .../tests/compile_failure/trait_wrong_method_name/Prover.toml | 2 ++ .../compile_failure/trait_wrong_method_return_type/Prover.toml | 2 ++ .../tests/compile_failure/trait_wrong_parameter/Prover.toml | 2 ++ .../compile_failure/trait_wrong_parameters_count/Prover.toml | 2 ++ 8 files changed, 16 insertions(+) create mode 100644 crates/nargo_cli/tests/compile_failure/dup_trait_declaration/Prover.toml create mode 100644 crates/nargo_cli/tests/compile_failure/dup_trait_implementation/Prover.toml create mode 100644 crates/nargo_cli/tests/compile_failure/impl_struct_not_trait/Prover.toml create mode 100644 crates/nargo_cli/tests/compile_failure/trait_not_exists/Prover.toml create mode 100644 crates/nargo_cli/tests/compile_failure/trait_wrong_method_name/Prover.toml create mode 100644 crates/nargo_cli/tests/compile_failure/trait_wrong_method_return_type/Prover.toml create mode 100644 crates/nargo_cli/tests/compile_failure/trait_wrong_parameter/Prover.toml create mode 100644 crates/nargo_cli/tests/compile_failure/trait_wrong_parameters_count/Prover.toml diff --git a/crates/nargo_cli/tests/compile_failure/dup_trait_declaration/Prover.toml b/crates/nargo_cli/tests/compile_failure/dup_trait_declaration/Prover.toml new file mode 100644 index 00000000000..2c1854573a4 --- /dev/null +++ b/crates/nargo_cli/tests/compile_failure/dup_trait_declaration/Prover.toml @@ -0,0 +1,2 @@ +x = 1 +y = 2 diff --git a/crates/nargo_cli/tests/compile_failure/dup_trait_implementation/Prover.toml b/crates/nargo_cli/tests/compile_failure/dup_trait_implementation/Prover.toml new file mode 100644 index 00000000000..2c1854573a4 --- /dev/null +++ b/crates/nargo_cli/tests/compile_failure/dup_trait_implementation/Prover.toml @@ -0,0 +1,2 @@ +x = 1 +y = 2 diff --git a/crates/nargo_cli/tests/compile_failure/impl_struct_not_trait/Prover.toml b/crates/nargo_cli/tests/compile_failure/impl_struct_not_trait/Prover.toml new file mode 100644 index 00000000000..2c1854573a4 --- /dev/null +++ b/crates/nargo_cli/tests/compile_failure/impl_struct_not_trait/Prover.toml @@ -0,0 +1,2 @@ +x = 1 +y = 2 diff --git a/crates/nargo_cli/tests/compile_failure/trait_not_exists/Prover.toml b/crates/nargo_cli/tests/compile_failure/trait_not_exists/Prover.toml new file mode 100644 index 00000000000..2c1854573a4 --- /dev/null +++ b/crates/nargo_cli/tests/compile_failure/trait_not_exists/Prover.toml @@ -0,0 +1,2 @@ +x = 1 +y = 2 diff --git a/crates/nargo_cli/tests/compile_failure/trait_wrong_method_name/Prover.toml b/crates/nargo_cli/tests/compile_failure/trait_wrong_method_name/Prover.toml new file mode 100644 index 00000000000..2c1854573a4 --- /dev/null +++ b/crates/nargo_cli/tests/compile_failure/trait_wrong_method_name/Prover.toml @@ -0,0 +1,2 @@ +x = 1 +y = 2 diff --git a/crates/nargo_cli/tests/compile_failure/trait_wrong_method_return_type/Prover.toml b/crates/nargo_cli/tests/compile_failure/trait_wrong_method_return_type/Prover.toml new file mode 100644 index 00000000000..2c1854573a4 --- /dev/null +++ b/crates/nargo_cli/tests/compile_failure/trait_wrong_method_return_type/Prover.toml @@ -0,0 +1,2 @@ +x = 1 +y = 2 diff --git a/crates/nargo_cli/tests/compile_failure/trait_wrong_parameter/Prover.toml b/crates/nargo_cli/tests/compile_failure/trait_wrong_parameter/Prover.toml new file mode 100644 index 00000000000..2c1854573a4 --- /dev/null +++ b/crates/nargo_cli/tests/compile_failure/trait_wrong_parameter/Prover.toml @@ -0,0 +1,2 @@ +x = 1 +y = 2 diff --git a/crates/nargo_cli/tests/compile_failure/trait_wrong_parameters_count/Prover.toml b/crates/nargo_cli/tests/compile_failure/trait_wrong_parameters_count/Prover.toml new file mode 100644 index 00000000000..2c1854573a4 --- /dev/null +++ b/crates/nargo_cli/tests/compile_failure/trait_wrong_parameters_count/Prover.toml @@ -0,0 +1,2 @@ +x = 1 +y = 2 From 5032e7d342d5ccff5179a04a7823ae713afa1c30 Mon Sep 17 00:00:00 2001 From: Yordan Madzhunkov Date: Thu, 17 Aug 2023 11:28:33 +0300 Subject: [PATCH 08/31] fix(testsuite): Move trait test into correct place so it can run automatically --- .../tests/{test_data => execution_success}/traits/Nargo.toml | 1 + .../tests/{test_data => execution_success}/traits/Prover.toml | 0 .../tests/{test_data => execution_success}/traits/src/main.nr | 0 3 files changed, 1 insertion(+) rename crates/nargo_cli/tests/{test_data => execution_success}/traits/Nargo.toml (86%) rename crates/nargo_cli/tests/{test_data => execution_success}/traits/Prover.toml (100%) rename crates/nargo_cli/tests/{test_data => execution_success}/traits/src/main.nr (100%) diff --git a/crates/nargo_cli/tests/test_data/traits/Nargo.toml b/crates/nargo_cli/tests/execution_success/traits/Nargo.toml similarity index 86% rename from crates/nargo_cli/tests/test_data/traits/Nargo.toml rename to crates/nargo_cli/tests/execution_success/traits/Nargo.toml index 629f55ef064..75fb80c4bfa 100644 --- a/crates/nargo_cli/tests/test_data/traits/Nargo.toml +++ b/crates/nargo_cli/tests/execution_success/traits/Nargo.toml @@ -1,5 +1,6 @@ [package] name = "traits" +type = "bin" authors = [""] compiler_version = "0.1" diff --git a/crates/nargo_cli/tests/test_data/traits/Prover.toml b/crates/nargo_cli/tests/execution_success/traits/Prover.toml similarity index 100% rename from crates/nargo_cli/tests/test_data/traits/Prover.toml rename to crates/nargo_cli/tests/execution_success/traits/Prover.toml diff --git a/crates/nargo_cli/tests/test_data/traits/src/main.nr b/crates/nargo_cli/tests/execution_success/traits/src/main.nr similarity index 100% rename from crates/nargo_cli/tests/test_data/traits/src/main.nr rename to crates/nargo_cli/tests/execution_success/traits/src/main.nr From ed1790a990f3ff942d15b600835ec4f9e5c1076a Mon Sep 17 00:00:00 2001 From: Yordan Madzhunkov Date: Mon, 21 Aug 2023 09:50:00 +0300 Subject: [PATCH 09/31] Rename trait_not_exists -> trait_not_in_scope --- .../{trait_not_exists => trait_not_in_scope}/Nargo.toml | 4 ++-- .../{trait_not_exists => trait_not_in_scope}/Prover.toml | 0 .../{trait_not_exists => trait_not_in_scope}/src/main.nr | 0 3 files changed, 2 insertions(+), 2 deletions(-) rename crates/nargo_cli/tests/compile_failure/{trait_not_exists => trait_not_in_scope}/Nargo.toml (60%) rename crates/nargo_cli/tests/compile_failure/{trait_not_exists => trait_not_in_scope}/Prover.toml (100%) rename crates/nargo_cli/tests/compile_failure/{trait_not_exists => trait_not_in_scope}/src/main.nr (100%) diff --git a/crates/nargo_cli/tests/compile_failure/trait_not_exists/Nargo.toml b/crates/nargo_cli/tests/compile_failure/trait_not_in_scope/Nargo.toml similarity index 60% rename from crates/nargo_cli/tests/compile_failure/trait_not_exists/Nargo.toml rename to crates/nargo_cli/tests/compile_failure/trait_not_in_scope/Nargo.toml index 6c8927de9c6..22d31e22e29 100644 --- a/crates/nargo_cli/tests/compile_failure/trait_not_exists/Nargo.toml +++ b/crates/nargo_cli/tests/compile_failure/trait_not_in_scope/Nargo.toml @@ -1,7 +1,7 @@ [package] -name = "trait_not_exists" +name = "trait_not_in_scope" type = "bin" authors = [""] compiler_version = "0.9.0" -[dependencies] \ No newline at end of file +[dependencies] diff --git a/crates/nargo_cli/tests/compile_failure/trait_not_exists/Prover.toml b/crates/nargo_cli/tests/compile_failure/trait_not_in_scope/Prover.toml similarity index 100% rename from crates/nargo_cli/tests/compile_failure/trait_not_exists/Prover.toml rename to crates/nargo_cli/tests/compile_failure/trait_not_in_scope/Prover.toml diff --git a/crates/nargo_cli/tests/compile_failure/trait_not_exists/src/main.nr b/crates/nargo_cli/tests/compile_failure/trait_not_in_scope/src/main.nr similarity index 100% rename from crates/nargo_cli/tests/compile_failure/trait_not_exists/src/main.nr rename to crates/nargo_cli/tests/compile_failure/trait_not_in_scope/src/main.nr From 1c6359252334592132fc021d6af1d7cabd918034 Mon Sep 17 00:00:00 2001 From: Yordan Madzhunkov Date: Mon, 21 Aug 2023 14:28:18 +0300 Subject: [PATCH 10/31] Resolve some of PR comments --- .../src/hir/def_collector/dc_crate.rs | 5 +- .../src/hir/def_collector/dc_mod.rs | 109 +++++------------- .../src/hir/def_collector/errors.rs | 86 ++++++++++---- 3 files changed, 99 insertions(+), 101 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 0145591174c..53be33a6a55 100644 --- a/crates/noirc_frontend/src/hir/def_collector/dc_crate.rs +++ b/crates/noirc_frontend/src/hir/def_collector/dc_crate.rs @@ -73,7 +73,6 @@ pub struct DefCollector { pub(crate) collected_globals: Vec, pub(crate) collected_impls: ImplMap, pub(crate) collected_traits_impls: ImplMap, - } /// Maps the type and the module id in which the impl is defined to the functions contained in that @@ -403,8 +402,8 @@ fn resolve_traits( _crate_id: CrateId, _errors: &mut [FileDiagnostic], ) { - for (type_id, typ) in &traits { - context.def_interner.push_empty_trait(*type_id, typ); + for (trait_id, unresolved_trait) in &traits { + context.def_interner.push_empty_trait(*trait_id, unresolved_trait); } } 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 02cef6d5813..4537b37c092 100644 --- a/crates/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/crates/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -7,7 +7,7 @@ use crate::{ node_interner::{StructId, TraitId}, parser::SubModule, FunctionReturnType, Ident, LetStatement, NoirFunction, NoirStruct, NoirTrait, NoirTypeAlias, - ParsedModule, TraitConstraint, TraitImpl, TraitImplItem, TraitItem, TypeImpl, UnresolvedType, + ParsedModule, TraitImpl, TraitImplItem, TraitItem, TypeImpl, UnresolvedType, }; use super::{ @@ -70,62 +70,35 @@ pub fn collect_defs( collector.collect_impls(context, ast.impls); } -fn check_trait_method_implementation_generics( - _generics: &[Ident], - _noir_function: &NoirFunction, - _trait_name: &str, -) -> Result<(), DefCollectorErrorKind> { - // TODO - Ok(()) -} - fn check_trait_method_implementation_parameters( - parameters: &Vec<(Ident, UnresolvedType)>, + expected_parameters: &Vec<(Ident, UnresolvedType)>, noir_function: &NoirFunction, trait_name: &String, ) -> Result<(), DefCollectorErrorKind> { - let expected_num_parameters = parameters.len(); + let expected_num_parameters = expected_parameters.len(); let actual_num_parameters = noir_function.def.parameters.len(); if actual_num_parameters != expected_num_parameters { - let method_name = noir_function.name(); - let primary_message = format!( - "Mismatch - expected {expected_num_parameters} arguments, but got {actual_num_parameters} of trait `{trait_name}` implementation `{method_name}`"); - return Err(DefCollectorErrorKind::MismatchTraitSignature { - primary_message, - secondary_message: "".to_string(), - span: noir_function.name_ident().span(), + return Err(DefCollectorErrorKind::MismatchTraitImplementationNumParameters { + actual_num_parameters, + expected_num_parameters, + trait_name: trait_name.clone(), + impl_ident: noir_function.name_ident().clone(), }); } - for (count, (pattern, typ, _abi_vis)) in noir_function.def.parameters.iter().enumerate() { - let (_expected_name, expected_type) = ¶meters[count]; + for (count, (parameter, typ, _abi_vis)) in noir_function.def.parameters.iter().enumerate() { + let (_expected_name, expected_type) = &expected_parameters[count]; if typ != expected_type { - return Err(DefCollectorErrorKind::MismatchTraitSignature { - primary_message: format!( - "Mismatch signature of method {} that implemtns trait {}", - noir_function.name(), - trait_name, - ), - secondary_message: format!( - "`{}: {}` expected", - pattern.name_ident().0.contents, - expected_type, - ), - span: pattern.name_ident().span(), + return Err(DefCollectorErrorKind::MismatchTraitImlementationParameter { + trait_name: trait_name.clone(), + expected_type: expected_type.clone(), + impl_method: noir_function.name().to_string(), + parameter: parameter.name_ident().clone(), }); } } Ok(()) } -fn check_trait_method_implementation_trait_constains( - _where_clause: &[TraitConstraint], - _noir_function: &NoirFunction, - _trait_name: &str, -) -> Result<(), DefCollectorErrorKind> { - // TODO - Ok(()) -} - fn check_trait_method_implementation_return_type( return_type: &FunctionReturnType, noir_function: &NoirFunction, @@ -134,15 +107,10 @@ fn check_trait_method_implementation_return_type( match return_type { FunctionReturnType::Default(_span) => Ok(()), FunctionReturnType::Ty(typ, _span) => { - if !(typ == &noir_function.return_type()) { - Err(DefCollectorErrorKind::MismatchTraitSignature { - primary_message: format!( - "Mismatch return type of method with name {} that implements trait {}", - noir_function.name(), - trait_name - ), - secondary_message: "".to_string(), - span: noir_function.name_ident().span(), + if typ != &noir_function.return_type() { + Err(DefCollectorErrorKind::MismatchTraitImplementationReturnType { + trait_name: trait_name.clone(), + impl_ident: noir_function.name_ident().clone(), }) } else { Ok(()) @@ -158,30 +126,20 @@ fn check_trait_method_implementation( for item in &r#trait.items { if let TraitItem::Function { name, - generics, + generics: _, parameters, return_type, - where_clause, + where_clause: _, body: _, } = item { if name.0.contents == noir_function.def.name.0.contents { - // name matches, check for parameters, return type and where clause - check_trait_method_implementation_generics( - generics, - noir_function, - &r#trait.name.0.contents, - )?; + // name matches, check for parameters - count and type, return type check_trait_method_implementation_parameters( parameters, noir_function, &r#trait.name.0.contents, )?; - check_trait_method_implementation_trait_constains( - where_clause, - noir_function, - &r#trait.name.0.contents, - )?; check_trait_method_implementation_return_type( return_type, noir_function, @@ -193,10 +151,8 @@ fn check_trait_method_implementation( } Err(DefCollectorErrorKind::MethodNotInTrait { - trait_name: r#trait.name.0.contents.to_string(), - trait_span: r#trait.name.span(), - impl_method_name: noir_function.name().to_string(), - impl_method_span: noir_function.name_ident().span(), + trait_name: r#trait.name.clone(), + impl_method: noir_function.def.name.clone(), }) } @@ -310,14 +266,7 @@ impl<'a> ModCollector<'a> { collected_implementations, )); } else { - let error = DefCollectorErrorKind::NotATrait { - primary_message: format!( - "{} is not a trait, therefore it can't be implemented", - trait_name - ), - secondary_message: "".to_string(), - span: trait_name.span(), - }; + let error = DefCollectorErrorKind::NotATrait { not_a_trait_name: trait_name.clone() }; errors.push(error.into_file_diagnostic(self.file_id)); } } @@ -333,13 +282,17 @@ impl<'a> ModCollector<'a> { UnresolvedFunctions { file_id: self.file_id, functions: Vec::new() }; for item in &trait_impl.items { if let TraitImplItem::Function(noir_function) = item { - match check_trait_method_implementation(trait_def, &noir_function) { + match check_trait_method_implementation(trait_def, noir_function) { Ok(()) => { let func_id = context.def_interner.push_empty_fn(); context .def_interner .push_function_definition(noir_function.name().to_owned(), func_id); - unresolved_functions.push_fn(self.module_id, func_id, noir_function.clone()); + unresolved_functions.push_fn( + self.module_id, + func_id, + noir_function.clone(), + ); } Err(error) => { 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 46971d68612..b80a4b5bbfa 100644 --- a/crates/noirc_frontend/src/hir/def_collector/errors.rs +++ b/crates/noirc_frontend/src/hir/def_collector/errors.rs @@ -1,5 +1,6 @@ use crate::hir::resolution::import::PathResolutionError; use crate::Ident; +use crate::UnresolvedType; use noirc_errors::CustomDiagnostic as Diagnostic; use noirc_errors::FileDiagnostic; @@ -31,17 +32,26 @@ pub enum DefCollectorErrorKind { #[error("Cannot `impl` a type defined outside the current crate")] ForeignImpl { span: Span, type_name: String }, #[error("Mismatch signature of trait")] - MismatchTraitSignature { primary_message: String, secondary_message: String, span: Span }, - #[error("Method is not defined in trait")] - MethodNotInTrait { + 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, - trait_span: Span, - impl_method_name: String, - impl_method_span: Span, + impl_ident: Ident, }, - #[error("Not a trait")] - NotATrait { primary_message: String, secondary_message: String, span: Span }, - #[error("Trait {trait_name} not found")] + #[error("Method is not defined in trait")] + MethodNotInTrait { trait_name: Ident, impl_method: Ident }, + #[error("Only traits can be implemented")] + NotATrait { not_a_trait_name: Ident }, + #[error("Trait not found")] TraitNotFound { trait_name: String, span: Span }, } @@ -110,22 +120,58 @@ impl From for Diagnostic { "".to_string(), span, ), - DefCollectorErrorKind::MismatchTraitSignature { - primary_message, - secondary_message, - span, - } => Diagnostic::simple_error(primary_message, secondary_message, span), - DefCollectorErrorKind::MethodNotInTrait { + DefCollectorErrorKind::MismatchTraitImplementationReturnType { + trait_name, + impl_ident, + } => { + let span = impl_ident.span().clone(); + 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, - trait_span: _, - impl_method_name, - impl_method_span, + impl_method, + parameter, + expected_type, } => { + 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) + } + 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"); Diagnostic::simple_error(primary_message, "".to_owned(), impl_method_span) } - DefCollectorErrorKind::NotATrait { primary_message, secondary_message, span } => { - Diagnostic::simple_error(primary_message, secondary_message, span) + DefCollectorErrorKind::NotATrait { not_a_trait_name } => { + let span = not_a_trait_name.0.span(); + let name = ¬_a_trait_name.0.contents; + Diagnostic::simple_error( + format!("{name} is not a trait, therefore it can't be implemented"), + String::new(), + span, + ) } } } From 4f5d1df25317ea902fa707e83a997cb306dd9c2c Mon Sep 17 00:00:00 2001 From: Yordan Madzhunkov Date: Mon, 21 Aug 2023 14:46:26 +0300 Subject: [PATCH 11/31] better naming as requsted in PR --- .../src/hir/def_collector/dc_mod.rs | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 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 4537b37c092..63767d7f8a6 100644 --- a/crates/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/crates/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -72,26 +72,26 @@ pub fn collect_defs( fn check_trait_method_implementation_parameters( expected_parameters: &Vec<(Ident, UnresolvedType)>, - noir_function: &NoirFunction, + impl_method: &NoirFunction, trait_name: &String, ) -> Result<(), DefCollectorErrorKind> { let expected_num_parameters = expected_parameters.len(); - let actual_num_parameters = noir_function.def.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.clone(), - impl_ident: noir_function.name_ident().clone(), + impl_ident: impl_method.name_ident().clone(), }); } - for (count, (parameter, typ, _abi_vis)) in noir_function.def.parameters.iter().enumerate() { + for (count, (parameter, typ, _abi_vis)) in impl_method.def.parameters.iter().enumerate() { let (_expected_name, expected_type) = &expected_parameters[count]; if typ != expected_type { return Err(DefCollectorErrorKind::MismatchTraitImlementationParameter { trait_name: trait_name.clone(), expected_type: expected_type.clone(), - impl_method: noir_function.name().to_string(), + impl_method: impl_method.name().to_string(), parameter: parameter.name_ident().clone(), }); } @@ -100,17 +100,17 @@ fn check_trait_method_implementation_parameters( } fn check_trait_method_implementation_return_type( - return_type: &FunctionReturnType, - noir_function: &NoirFunction, + expected_return_type: &FunctionReturnType, + impl_method: &NoirFunction, trait_name: &String, ) -> Result<(), DefCollectorErrorKind> { - match return_type { + match expected_return_type { FunctionReturnType::Default(_span) => Ok(()), - FunctionReturnType::Ty(typ, _span) => { - if typ != &noir_function.return_type() { + FunctionReturnType::Ty(expected_type, _span) => { + if expected_type != &impl_method.return_type() { Err(DefCollectorErrorKind::MismatchTraitImplementationReturnType { trait_name: trait_name.clone(), - impl_ident: noir_function.name_ident().clone(), + impl_ident: impl_method.name_ident().clone(), }) } else { Ok(()) From a7f7188ceae555d442a3a344773b74365e6faacc Mon Sep 17 00:00:00 2001 From: Yordan Madzhunkov Date: Mon, 21 Aug 2023 14:49:06 +0300 Subject: [PATCH 12/31] Remove empty else and rename noir_function -> impl_method --- .../src/hir/def_collector/dc_mod.rs | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 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 63767d7f8a6..f8c9eb0a587 100644 --- a/crates/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/crates/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -121,7 +121,7 @@ fn check_trait_method_implementation_return_type( fn check_trait_method_implementation( r#trait: &NoirTrait, - noir_function: &NoirFunction, + impl_method: &NoirFunction, ) -> Result<(), DefCollectorErrorKind> { for item in &r#trait.items { if let TraitItem::Function { @@ -133,16 +133,16 @@ fn check_trait_method_implementation( body: _, } = item { - if name.0.contents == noir_function.def.name.0.contents { + 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, - noir_function, + impl_method, &r#trait.name.0.contents, )?; check_trait_method_implementation_return_type( return_type, - noir_function, + impl_method, &r#trait.name.0.contents, )?; return Ok(()); @@ -152,7 +152,7 @@ fn check_trait_method_implementation( Err(DefCollectorErrorKind::MethodNotInTrait { trait_name: r#trait.name.clone(), - impl_method: noir_function.def.name.clone(), + impl_method: impl_method.def.name.clone(), }) } @@ -281,24 +281,23 @@ impl<'a> ModCollector<'a> { let mut unresolved_functions = UnresolvedFunctions { file_id: self.file_id, functions: Vec::new() }; for item in &trait_impl.items { - if let TraitImplItem::Function(noir_function) = item { - match check_trait_method_implementation(trait_def, noir_function) { + 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(noir_function.name().to_owned(), func_id); + .push_function_definition(impl_method.name().to_owned(), func_id); unresolved_functions.push_fn( self.module_id, func_id, - noir_function.clone(), + impl_method.clone(), ); } Err(error) => { errors.push(error.into_file_diagnostic(self.file_id)); } } - } else { } } unresolved_functions From e46c4377db08085306579e387e86f7ae84f56a4a Mon Sep 17 00:00:00 2001 From: Yordan Madzhunkov Date: Mon, 21 Aug 2023 15:18:03 +0300 Subject: [PATCH 13/31] Check for matching of trait implementation method, when unspecified return type --- .../src/hir/def_collector/dc_mod.rs | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 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 f8c9eb0a587..d3793ab1690 100644 --- a/crates/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/crates/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -105,7 +105,15 @@ fn check_trait_method_implementation_return_type( trait_name: &String, ) -> Result<(), DefCollectorErrorKind> { match expected_return_type { - FunctionReturnType::Default(_span) => Ok(()), + FunctionReturnType::Default(_span) => match &impl_method.def.return_type { + FunctionReturnType::Default(_span) => Ok(()), + FunctionReturnType::Ty(_found_type, _span) => { + Err(DefCollectorErrorKind::MismatchTraitImplementationReturnType { + trait_name: trait_name.clone(), + impl_ident: impl_method.name_ident().clone(), + }) + } + }, FunctionReturnType::Ty(expected_type, _span) => { if expected_type != &impl_method.return_type() { Err(DefCollectorErrorKind::MismatchTraitImplementationReturnType { @@ -288,11 +296,7 @@ impl<'a> ModCollector<'a> { 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(), - ); + unresolved_functions.push_fn(self.module_id, func_id, impl_method.clone()); } Err(error) => { errors.push(error.into_file_diagnostic(self.file_id)); From 44d9ee444549ee6febaee686a8c27aff5b1e1467 Mon Sep 17 00:00:00 2001 From: Yordan Madzhunkov Date: Mon, 21 Aug 2023 18:21:26 +0300 Subject: [PATCH 14/31] Apply cargo clippy suggestions --- .../noirc_frontend/src/hir/def_collector/dc_mod.rs | 12 ++++++------ .../noirc_frontend/src/hir/def_collector/errors.rs | 2 +- 2 files changed, 7 insertions(+), 7 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 d3793ab1690..9e739dca6b7 100644 --- a/crates/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/crates/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -73,7 +73,7 @@ pub fn collect_defs( fn check_trait_method_implementation_parameters( expected_parameters: &Vec<(Ident, UnresolvedType)>, impl_method: &NoirFunction, - trait_name: &String, + trait_name: &str, ) -> Result<(), DefCollectorErrorKind> { let expected_num_parameters = expected_parameters.len(); let actual_num_parameters = impl_method.def.parameters.len(); @@ -81,7 +81,7 @@ fn check_trait_method_implementation_parameters( return Err(DefCollectorErrorKind::MismatchTraitImplementationNumParameters { actual_num_parameters, expected_num_parameters, - trait_name: trait_name.clone(), + trait_name: trait_name.to_owned(), impl_ident: impl_method.name_ident().clone(), }); } @@ -89,7 +89,7 @@ fn check_trait_method_implementation_parameters( let (_expected_name, expected_type) = &expected_parameters[count]; if typ != expected_type { return Err(DefCollectorErrorKind::MismatchTraitImlementationParameter { - trait_name: trait_name.clone(), + trait_name: trait_name.to_owned(), expected_type: expected_type.clone(), impl_method: impl_method.name().to_string(), parameter: parameter.name_ident().clone(), @@ -102,14 +102,14 @@ fn check_trait_method_implementation_parameters( fn check_trait_method_implementation_return_type( expected_return_type: &FunctionReturnType, impl_method: &NoirFunction, - trait_name: &String, + trait_name: &str, ) -> Result<(), DefCollectorErrorKind> { match expected_return_type { FunctionReturnType::Default(_span) => match &impl_method.def.return_type { FunctionReturnType::Default(_span) => Ok(()), FunctionReturnType::Ty(_found_type, _span) => { Err(DefCollectorErrorKind::MismatchTraitImplementationReturnType { - trait_name: trait_name.clone(), + trait_name: trait_name.to_owned(), impl_ident: impl_method.name_ident().clone(), }) } @@ -117,7 +117,7 @@ fn check_trait_method_implementation_return_type( FunctionReturnType::Ty(expected_type, _span) => { if expected_type != &impl_method.return_type() { Err(DefCollectorErrorKind::MismatchTraitImplementationReturnType { - trait_name: trait_name.clone(), + trait_name: trait_name.to_owned(), impl_ident: impl_method.name_ident().clone(), }) } else { diff --git a/crates/noirc_frontend/src/hir/def_collector/errors.rs b/crates/noirc_frontend/src/hir/def_collector/errors.rs index b80a4b5bbfa..e865d297966 100644 --- a/crates/noirc_frontend/src/hir/def_collector/errors.rs +++ b/crates/noirc_frontend/src/hir/def_collector/errors.rs @@ -124,7 +124,7 @@ impl From for Diagnostic { trait_name, impl_ident, } => { - let span = impl_ident.span().clone(); + 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}"), From b41bab5c93fe608c1947a122e65621b8cef38a0b Mon Sep 17 00:00:00 2001 From: Yordan Madzhunkov Date: Tue, 22 Aug 2023 11:54:08 +0300 Subject: [PATCH 15/31] Delete duplicated test --- .../Nargo.toml | 7 ------- .../src/main.nr | 21 ------------------- 2 files changed, 28 deletions(-) delete mode 100644 crates/nargo_cli/tests/compile_failure/trait_implementation_wrong_parameter/Nargo.toml delete mode 100644 crates/nargo_cli/tests/compile_failure/trait_implementation_wrong_parameter/src/main.nr diff --git a/crates/nargo_cli/tests/compile_failure/trait_implementation_wrong_parameter/Nargo.toml b/crates/nargo_cli/tests/compile_failure/trait_implementation_wrong_parameter/Nargo.toml deleted file mode 100644 index ef62b1e6c3a..00000000000 --- a/crates/nargo_cli/tests/compile_failure/trait_implementation_wrong_parameter/Nargo.toml +++ /dev/null @@ -1,7 +0,0 @@ -[package] -name = "trait_implementation_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_implementation_wrong_parameter/src/main.nr b/crates/nargo_cli/tests/compile_failure/trait_implementation_wrong_parameter/src/main.nr deleted file mode 100644 index 92469ae8fdb..00000000000 --- a/crates/nargo_cli/tests/compile_failure/trait_implementation_wrong_parameter/src/main.nr +++ /dev/null @@ -1,21 +0,0 @@ -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) -> Self { - Self { bar: x, array: [x, x] } - } -} - -fn main(x: Field, y: Field) { - let first = Foo::default(x,y); - assert(first.bar == x); -} From 7046cf1fdc1d10f0c2cc7e22b0e5334a44a6a927 Mon Sep 17 00:00:00 2001 From: Yordan Madzhunkov Date: Wed, 23 Aug 2023 10:56:51 +0300 Subject: [PATCH 16/31] Partial trait resolution --- .../trait_wrong_parameter_type/Nargo.toml | 7 ++ .../trait_wrong_parameter_type/Prover.toml | 2 + .../trait_wrong_parameter_type/src/main.nr | 7 ++ .../src/hir/def_collector/dc_crate.rs | 115 +++++++++++++++++- crates/noirc_frontend/src/hir_def/types.rs | 6 +- crates/noirc_frontend/src/node_interner.rs | 5 + 6 files changed, 135 insertions(+), 7 deletions(-) create mode 100644 crates/nargo_cli/tests/compile_failure/trait_wrong_parameter_type/Nargo.toml create mode 100644 crates/nargo_cli/tests/compile_failure/trait_wrong_parameter_type/Prover.toml create mode 100644 crates/nargo_cli/tests/compile_failure/trait_wrong_parameter_type/src/main.nr diff --git a/crates/nargo_cli/tests/compile_failure/trait_wrong_parameter_type/Nargo.toml b/crates/nargo_cli/tests/compile_failure/trait_wrong_parameter_type/Nargo.toml new file mode 100644 index 00000000000..95e3e222ca3 --- /dev/null +++ b/crates/nargo_cli/tests/compile_failure/trait_wrong_parameter_type/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_parameter_type/Prover.toml b/crates/nargo_cli/tests/compile_failure/trait_wrong_parameter_type/Prover.toml new file mode 100644 index 00000000000..2c1854573a4 --- /dev/null +++ b/crates/nargo_cli/tests/compile_failure/trait_wrong_parameter_type/Prover.toml @@ -0,0 +1,2 @@ +x = 1 +y = 2 diff --git a/crates/nargo_cli/tests/compile_failure/trait_wrong_parameter_type/src/main.nr b/crates/nargo_cli/tests/compile_failure/trait_wrong_parameter_type/src/main.nr new file mode 100644 index 00000000000..2ba1ee13e70 --- /dev/null +++ b/crates/nargo_cli/tests/compile_failure/trait_wrong_parameter_type/src/main.nr @@ -0,0 +1,7 @@ +trait Default { + fn default(x: Field, y: NotAType) -> Field; +} + +fn main(x: Field, y: Field) { + assert(y == x); +} 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 53be33a6a55..cf428defb1e 100644 --- a/crates/noirc_frontend/src/hir/def_collector/dc_crate.rs +++ b/crates/noirc_frontend/src/hir/def_collector/dc_crate.rs @@ -3,6 +3,8 @@ use super::errors::{DefCollectorErrorKind, DuplicateType}; use crate::graph::CrateId; use crate::hir::def_map::{CrateDefMap, LocalModuleId, ModuleId}; use crate::hir::resolution::errors::ResolverError; +//use crate::hir::resolution::errors::ResolverError::PathResolutionError; +use crate::hir::resolution::import::PathResolutionError; use crate::hir::resolution::resolver::Resolver; use crate::hir::resolution::{ import::{resolve_imports, ImportDirective}, @@ -12,9 +14,9 @@ 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, Generics, Ident, LetStatement, Literal, NoirFunction, NoirStruct, NoirTrait, - NoirTypeAlias, ParsedModule, Shared, StructType, Type, TypeBinding, UnresolvedGenerics, - UnresolvedType, + ExpressionKind, FunctionReturnType, Generics, Ident, LetStatement, Literal, NoirFunction, + NoirStruct, NoirTrait, NoirTypeAlias, ParsedModule, Shared, StructType, TraitItem, + TraitItemType, Type, TypeBinding, UnresolvedGenerics, UnresolvedType, }; use fm::FileId; use iter_extended::vecmap; @@ -22,6 +24,7 @@ use noirc_errors::Span; use noirc_errors::{CustomDiagnostic, FileDiagnostic}; use std::collections::HashMap; use std::rc::Rc; +use std::vec; /// Stores all of the unresolved functions in a particular file/mod pub struct UnresolvedFunctions { @@ -183,8 +186,8 @@ impl DefCollector { resolve_type_aliases(context, def_collector.collected_type_aliases, crate_id, errors); - // Must resolve structs before we resolve globals. resolve_traits(context, def_collector.collected_traits, crate_id, errors); + // Must resolve structs before we resolve globals. resolve_structs(context, def_collector.collected_types, crate_id, errors); // We must wait to resolve non-integer globals until after we resolve structs since structs @@ -394,17 +397,117 @@ fn resolve_structs( } } +fn resolve_trait_types( + _context: &mut Context, + _crate_id: CrateId, + _unresolved_trait: &UnresolvedTrait, + _errors: &mut [FileDiagnostic], +) -> Vec { + // TODO + vec![] +} +fn resolve_trait_constants( + _context: &mut Context, + _crate_id: CrateId, + _unresolved_trait: &UnresolvedTrait, + _errors: &mut [FileDiagnostic], +) -> Vec { + // TODO + vec![] +} +fn resolve_trait_methods( + context: &mut Context, + crate_id: CrateId, + unresolved_trait: &UnresolvedTrait, + errors: &mut Vec, +) -> Vec { + let interner = &mut context.def_interner; + let def_maps = &mut context.def_maps; + + let path_resolver = StandardPathResolver::new(ModuleId { + local_id: unresolved_trait.module_id, + krate: crate_id, + }); + let file = def_maps[&crate_id].file_id(unresolved_trait.module_id); + + let mut res = vec![]; + + for item in &unresolved_trait.trait_def.items { + if let TraitItem::Function { + name, + generics: _, + parameters, + return_type, + where_clause: _, + body: _, + } = item + { + let mut resolver = Resolver::new(interner, &path_resolver, def_maps, file); + + //let self_type = resolver.resolve_type(unresolved_trait.clone()); + //resolver.set_self_type(self_type.clone()); + + 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, _span) => { + Some(resolver.resolve_type(unresolved_type.clone())) + } + }; + let name = name.clone(); + // TODO + let generics: Generics = vec![]; + let span: Span = name.span(); + let f = TraitItemType::Function { + name, + generics, + arguments, + return_type: resolved_return_type, + span, + }; + res.push(f); + // TODO, this is HACK and it should be removed when we implement a resolution of Self type in trait + let new_errors: Vec = resolver + .take_errors() + .iter() + .cloned() + .filter(|resolution_error| match resolution_error { + ResolverError::PathResolutionError(PathResolutionError::Unresolved(ident)) => { + &ident.0.contents != "Self" + } + _ => true, + }) + .collect(); + extend_errors(errors, file, new_errors); + } + } + res +} + /// Create the mappings from TypeId -> TraitType /// so that expressions can access the elements of traits fn resolve_traits( context: &mut Context, traits: HashMap, - _crate_id: CrateId, - _errors: &mut [FileDiagnostic], + crate_id: CrateId, + errors: &mut Vec, ) { for (trait_id, unresolved_trait) in &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)); + // 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)); + context.def_interner.update_trait(trait_id, |trait_def| { + trait_def.set_items(items); + }); + } } fn resolve_struct_fields( diff --git a/crates/noirc_frontend/src/hir_def/types.rs b/crates/noirc_frontend/src/hir_def/types.rs index 7988c20d244..8372f7a0355 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: Type, + return_type: Option, span: Span, }, @@ -196,6 +196,10 @@ impl Trait { ) -> Trait { Trait { id, name, span, items, generics } } + + pub fn set_items(&mut self, items: Vec) { + self.items = items; + } } impl std::fmt::Display for Trait { diff --git a/crates/noirc_frontend/src/node_interner.rs b/crates/noirc_frontend/src/node_interner.rs index 78397a9ac6f..ae53a71cba6 100644 --- a/crates/noirc_frontend/src/node_interner.rs +++ b/crates/noirc_frontend/src/node_interner.rs @@ -389,6 +389,11 @@ impl NodeInterner { f(&mut value); } + pub fn update_trait(&mut self, trait_id: TraitId, f: impl FnOnce(&mut Trait)) { + let mut value = self.traits.get_mut(&trait_id).unwrap().borrow_mut(); + f(&mut value); + } + pub fn set_type_alias(&mut self, type_id: TypeAliasId, typ: Type, generics: Generics) { let type_alias_type = &mut self.type_aliases[type_id.0]; type_alias_type.set_type_and_generics(typ, generics); From 3f98928d131fc4dbea722327187594eb840b289d Mon Sep 17 00:00:00 2001 From: Yordan Madzhunkov Date: Thu, 24 Aug 2023 15:03:55 +0300 Subject: [PATCH 17/31] Extract comparison of return type into stuct assosiated method --- crates/noirc_frontend/src/ast/expression.rs | 15 +++++++++ .../src/hir/def_collector/dc_crate.rs | 32 +++++++++---------- .../src/hir/def_collector/dc_mod.rs | 27 ++++------------ 3 files changed, 38 insertions(+), 36 deletions(-) diff --git a/crates/noirc_frontend/src/ast/expression.rs b/crates/noirc_frontend/src/ast/expression.rs index 170d3c569c9..1d189ebc93e 100644 --- a/crates/noirc_frontend/src/ast/expression.rs +++ b/crates/noirc_frontend/src/ast/expression.rs @@ -663,3 +663,18 @@ impl Display for FunctionReturnType { } } } + +impl FunctionReturnType { + pub fn is_equivelent(&self, other: &FunctionReturnType) -> bool { + match self { + FunctionReturnType::Default(_span) => match other { + FunctionReturnType::Default(_span) => true, + FunctionReturnType::Ty(_found_type, _span) => false, + }, + FunctionReturnType::Ty(lhs_type, _span) => match other { + FunctionReturnType::Default(_span) => false, + FunctionReturnType::Ty(rhs_type, _span) => lhs_type == rhs_type, + }, + } + } +} 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 cf428defb1e..037fa3d6c44 100644 --- a/crates/noirc_frontend/src/hir/def_collector/dc_crate.rs +++ b/crates/noirc_frontend/src/hir/def_collector/dc_crate.rs @@ -415,6 +415,7 @@ fn resolve_trait_constants( // TODO vec![] } + fn resolve_trait_methods( context: &mut Context, crate_id: CrateId, @@ -443,10 +444,6 @@ fn resolve_trait_methods( } = item { let mut resolver = Resolver::new(interner, &path_resolver, def_maps, file); - - //let self_type = resolver.resolve_type(unresolved_trait.clone()); - //resolver.set_self_type(self_type.clone()); - let arguments = vecmap(parameters, |param| resolver.resolve_type(param.1.clone())); let resolved_return_type = match return_type { FunctionReturnType::Default(_) => None, @@ -466,24 +463,27 @@ fn resolve_trait_methods( span, }; res.push(f); - // TODO, this is HACK and it should be removed when we implement a resolution of Self type in trait - let new_errors: Vec = resolver - .take_errors() - .iter() - .cloned() - .filter(|resolution_error| match resolution_error { - ResolverError::PathResolutionError(PathResolutionError::Unresolved(ident)) => { - &ident.0.contents != "Self" - } - _ => true, - }) - .collect(); + let new_errors = take_errors_filter_self_not_resolved(resolver); extend_errors(errors, file, new_errors); } } res } +fn take_errors_filter_self_not_resolved(resolver: Resolver<'_>) -> Vec { + resolver + .take_errors() + .iter() + .cloned() + .filter(|resolution_error| match resolution_error { + ResolverError::PathResolutionError(PathResolutionError::Unresolved(ident)) => { + &ident.0.contents != "Self" + } + _ => true, + }) + .collect() +} + /// Create the mappings from TypeId -> TraitType /// so that expressions can access the elements of traits fn resolve_traits( 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 9e739dca6b7..f0a64f4bea9 100644 --- a/crates/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/crates/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -104,26 +104,13 @@ fn check_trait_method_implementation_return_type( impl_method: &NoirFunction, trait_name: &str, ) -> Result<(), DefCollectorErrorKind> { - match expected_return_type { - FunctionReturnType::Default(_span) => match &impl_method.def.return_type { - FunctionReturnType::Default(_span) => Ok(()), - FunctionReturnType::Ty(_found_type, _span) => { - Err(DefCollectorErrorKind::MismatchTraitImplementationReturnType { - trait_name: trait_name.to_owned(), - impl_ident: impl_method.name_ident().clone(), - }) - } - }, - FunctionReturnType::Ty(expected_type, _span) => { - if expected_type != &impl_method.return_type() { - Err(DefCollectorErrorKind::MismatchTraitImplementationReturnType { - trait_name: trait_name.to_owned(), - impl_ident: impl_method.name_ident().clone(), - }) - } else { - Ok(()) - } - } + if expected_return_type.is_equivelent(&impl_method.def.return_type) { + Ok(()) + } else { + Err(DefCollectorErrorKind::MismatchTraitImplementationReturnType { + trait_name: trait_name.to_owned(), + impl_ident: impl_method.name_ident().clone(), + }) } } From 68f0a15a3c473be4c0a3d423aa274d6f286db3ff Mon Sep 17 00:00:00 2001 From: Yordan Madzhunkov Date: Fri, 25 Aug 2023 11:05:48 +0300 Subject: [PATCH 18/31] Add test of a trait with default implemetation that is used --- .../trait_default_implementation/Nargo.toml | 7 +++++ .../trait_default_implementation/Prover.toml | 2 ++ .../trait_default_implementation/src/main.nr | 26 +++++++++++++++++++ 3 files changed, 35 insertions(+) create mode 100644 crates/nargo_cli/tests/execution_success/trait_default_implementation/Nargo.toml create mode 100644 crates/nargo_cli/tests/execution_success/trait_default_implementation/Prover.toml create mode 100644 crates/nargo_cli/tests/execution_success/trait_default_implementation/src/main.nr diff --git a/crates/nargo_cli/tests/execution_success/trait_default_implementation/Nargo.toml b/crates/nargo_cli/tests/execution_success/trait_default_implementation/Nargo.toml new file mode 100644 index 00000000000..75fb80c4bfa --- /dev/null +++ b/crates/nargo_cli/tests/execution_success/trait_default_implementation/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "traits" +type = "bin" +authors = [""] +compiler_version = "0.1" + +[dependencies] diff --git a/crates/nargo_cli/tests/execution_success/trait_default_implementation/Prover.toml b/crates/nargo_cli/tests/execution_success/trait_default_implementation/Prover.toml new file mode 100644 index 00000000000..71805e71e8e --- /dev/null +++ b/crates/nargo_cli/tests/execution_success/trait_default_implementation/Prover.toml @@ -0,0 +1,2 @@ +x = "5" +y = "1" \ No newline at end of file diff --git a/crates/nargo_cli/tests/execution_success/trait_default_implementation/src/main.nr b/crates/nargo_cli/tests/execution_success/trait_default_implementation/src/main.nr new file mode 100644 index 00000000000..e1f29ce3f48 --- /dev/null +++ b/crates/nargo_cli/tests/execution_success/trait_default_implementation/src/main.nr @@ -0,0 +1,26 @@ +use dep::std; + +trait Default { + fn default(x: Field, y: Field) -> Self; + + fn method2(x: Field) -> Field { + x + } + +} + +struct Foo { + bar: Field, + array: [Field; 2], +} + +impl Default for Foo { + fn default(x: Field,y: Field) -> Self { + Self { bar: x, array: [x,y] } + } +} + +fn main(x: Field) { + let first = Foo::method2(x); + assert(first == x); +} From d5caec40d44f811e99344e9f51fb5f3688338a65 Mon Sep 17 00:00:00 2001 From: Yordan Madzhunkov Date: Fri, 25 Aug 2023 13:25:07 +0300 Subject: [PATCH 19/31] Allow usage of default trait implementation --- .../trait_missing_implementation/Nargo.toml | 7 + .../trait_missing_implementation/src/main.nr | 24 ++++ .../src/hir/def_collector/dc_mod.rs | 123 +++++++++++++----- .../src/hir/def_collector/errors.rs | 18 +++ 4 files changed, 137 insertions(+), 35 deletions(-) create mode 100644 crates/nargo_cli/tests/compile_failure/trait_missing_implementation/Nargo.toml create mode 100644 crates/nargo_cli/tests/compile_failure/trait_missing_implementation/src/main.nr diff --git a/crates/nargo_cli/tests/compile_failure/trait_missing_implementation/Nargo.toml b/crates/nargo_cli/tests/compile_failure/trait_missing_implementation/Nargo.toml new file mode 100644 index 00000000000..75fb80c4bfa --- /dev/null +++ b/crates/nargo_cli/tests/compile_failure/trait_missing_implementation/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "traits" +type = "bin" +authors = [""] +compiler_version = "0.1" + +[dependencies] 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 new file mode 100644 index 00000000000..bc74b328592 --- /dev/null +++ b/crates/nargo_cli/tests/compile_failure/trait_missing_implementation/src/main.nr @@ -0,0 +1,24 @@ +use dep::std; + +trait Default { + fn default(x: Field, y: Field) -> Self; + + fn method2(x: Field) -> Field; + +} + +struct Foo { + bar: Field, + array: [Field; 2], +} + +impl Default for Foo { + fn default(x: Field,y: Field) -> Self { + Self { bar: x, array: [x,y] } + } +} + +fn main(x: Field) { + let first = Foo::method2(x); + assert(first == x); +} 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 f0a64f4bea9..1bb7504f25f 100644 --- a/crates/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/crates/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -7,7 +7,7 @@ use crate::{ node_interner::{StructId, TraitId}, parser::SubModule, FunctionReturnType, Ident, LetStatement, NoirFunction, NoirStruct, NoirTrait, NoirTypeAlias, - ParsedModule, TraitImpl, TraitImplItem, TraitItem, TypeImpl, UnresolvedType, + ParsedModule, Pattern, TraitImpl, TraitImplItem, TraitItem, TypeImpl, UnresolvedType, }; use super::{ @@ -214,13 +214,32 @@ impl<'a> ModCollector<'a> { let trait_name = trait_impl.trait_name.clone(); let module = &self.def_collector.def_map.modules[self.module_id.0]; match module.find_name(&trait_name).types { - Some((module_def_id, _visibility)) => self.collect_trait_from_module( - context, - module_def_id, - trait_impl, - errors, - &trait_name, - ), + 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( + context, + &trait_impl, + &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, + )); + } else { + let error = DefCollectorErrorKind::NotATrait { + not_a_trait_name: trait_name.clone(), + }; + errors.push(error.into_file_diagnostic(self.file_id)); + } + } None => { let error = DefCollectorErrorKind::TraitNotFound { trait_name: trait_name.to_string(), @@ -239,33 +258,6 @@ impl<'a> ModCollector<'a> { } } - fn collect_trait_from_module( - &mut self, - context: &mut Context, - module_def_id: ModuleDefId, - trait_impl: TraitImpl, - errors: &mut Vec, - trait_name: &Ident, - ) { - if let Some(collected_trait) = self.get_unresolved_trait(module_def_id) { - let trait_def = collected_trait.trait_def.clone(); - 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 collected_implementations = - self.collect_trait_implementations(context, &trait_impl, &trait_def, errors); - 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, - )); - } else { - let error = DefCollectorErrorKind::NotATrait { not_a_trait_name: trait_name.clone() }; - errors.push(error.into_file_diagnostic(self.file_id)); - } - } - fn collect_trait_implementations( &mut self, context: &mut Context, @@ -275,6 +267,7 @@ impl<'a> ModCollector<'a> { ) -> UnresolvedFunctions { let mut unresolved_functions = UnresolvedFunctions { file_id: self.file_id, functions: Vec::new() }; + for item in &trait_impl.items { if let TraitImplItem::Function(impl_method) = item { match check_trait_method_implementation(trait_def, impl_method) { @@ -291,6 +284,66 @@ impl<'a> ModCollector<'a> { } } } + + for item in &trait_def.items { + if let TraitItem::Function { + name, + generics, + parameters, + return_type, + where_clause, + body, + } = item + { + let is_implemented = unresolved_functions + .functions + .iter() + .any(|(_, _, func_impl)| func_impl.name() == name.0.contents); + if !is_implemented { + if body.is_some() { + let method_name = name.0.contents.clone(); + let func_id = context.def_interner.push_empty_fn(); + context.def_interner.push_function_definition(method_name, func_id); + let p = parameters + .iter() + .map(|(ident, unresolved_type)| { + ( + Pattern::Identifier(ident.clone()), + unresolved_type.clone(), + noirc_abi::AbiVisibility::Private, + ) + }) + .collect(); + let impl_method = NoirFunction::normal(crate::FunctionDefinition { + name: name.clone(), + attribute: None, + is_open: false, + is_internal: false, + is_unconstrained: false, + generics: generics.clone(), + parameters: p, + body: body.as_ref().unwrap().clone(), + span: name.span(), + where_clause: where_clause.clone(), + return_type: return_type.clone(), + return_visibility: noirc_abi::AbiVisibility::Private, + return_distinctness: noirc_abi::AbiDistinctness::DuplicationAllowed, + }); + unresolved_functions.push_fn(self.module_id, func_id, impl_method); + } else { + let error = DefCollectorErrorKind::TraitMissedMethodImplementation { + trait_name: trait_def.name.clone(), + method_name: name.clone(), + trait_impl_span: trait_impl.object_type_span, + }; + errors.push(error.into_file_diagnostic(self.file_id)); + + // Emit error that implementation of trait method is missing + } + } + } + } + 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 e865d297966..2aab7f514d9 100644 --- a/crates/noirc_frontend/src/hir/def_collector/errors.rs +++ b/crates/noirc_frontend/src/hir/def_collector/errors.rs @@ -53,6 +53,8 @@ pub enum DefCollectorErrorKind { NotATrait { not_a_trait_name: Ident }, #[error("Trait not found")] TraitNotFound { trait_name: String, span: Span }, + #[error("Missing Trait method implementation")] + TraitMissedMethodImplementation { trait_name: Ident, method_name: Ident, trait_impl_span: Span }, } impl DefCollectorErrorKind { @@ -164,6 +166,22 @@ impl From for Diagnostic { 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 { + trait_name, + method_name, + trait_impl_span, + } => { + 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" + ); + Diagnostic::simple_error( + primary_message, + format!("Please implement {impl_method_name} here"), + trait_impl_span, + ) + } DefCollectorErrorKind::NotATrait { not_a_trait_name } => { let span = not_a_trait_name.0.span(); let name = ¬_a_trait_name.0.contents; From 295f057387c3577c154a6c75ca467dd0995b6cb7 Mon Sep 17 00:00:00 2001 From: Yordan Madzhunkov Date: Fri, 25 Aug 2023 13:38:48 +0300 Subject: [PATCH 20/31] RP remark: fix typo --- crates/noirc_frontend/src/ast/expression.rs | 2 +- crates/noirc_frontend/src/hir/def_collector/dc_mod.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/noirc_frontend/src/ast/expression.rs b/crates/noirc_frontend/src/ast/expression.rs index 1d189ebc93e..00570b1d732 100644 --- a/crates/noirc_frontend/src/ast/expression.rs +++ b/crates/noirc_frontend/src/ast/expression.rs @@ -665,7 +665,7 @@ impl Display for FunctionReturnType { } impl FunctionReturnType { - pub fn is_equivelent(&self, other: &FunctionReturnType) -> bool { + pub fn is_equivalent(&self, other: &FunctionReturnType) -> bool { match self { FunctionReturnType::Default(_span) => match other { FunctionReturnType::Default(_span) => true, 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 1bb7504f25f..ccb7215f0e0 100644 --- a/crates/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/crates/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -104,7 +104,7 @@ fn check_trait_method_implementation_return_type( impl_method: &NoirFunction, trait_name: &str, ) -> Result<(), DefCollectorErrorKind> { - if expected_return_type.is_equivelent(&impl_method.def.return_type) { + if expected_return_type.is_equivalent(&impl_method.def.return_type) { Ok(()) } else { Err(DefCollectorErrorKind::MismatchTraitImplementationReturnType { From 36096d4b82f34e254871c1e6cb1b6eacb5c94a06 Mon Sep 17 00:00:00 2001 From: Yordan Madzhunkov Date: Fri, 25 Aug 2023 15:34:29 +0300 Subject: [PATCH 21/31] Maybem it's better this way --- crates/noirc_frontend/src/ast/expression.rs | 14 -------------- crates/noirc_frontend/src/ast/mod.rs | 9 +++++++++ .../noirc_frontend/src/hir/def_collector/dc_mod.rs | 4 +--- 3 files changed, 10 insertions(+), 17 deletions(-) diff --git a/crates/noirc_frontend/src/ast/expression.rs b/crates/noirc_frontend/src/ast/expression.rs index 00570b1d732..d65138ea1a2 100644 --- a/crates/noirc_frontend/src/ast/expression.rs +++ b/crates/noirc_frontend/src/ast/expression.rs @@ -664,17 +664,3 @@ impl Display for FunctionReturnType { } } -impl FunctionReturnType { - pub fn is_equivalent(&self, other: &FunctionReturnType) -> bool { - match self { - FunctionReturnType::Default(_span) => match other { - FunctionReturnType::Default(_span) => true, - FunctionReturnType::Ty(_found_type, _span) => false, - }, - FunctionReturnType::Ty(lhs_type, _span) => match other { - FunctionReturnType::Default(_span) => false, - FunctionReturnType::Ty(rhs_type, _span) => lhs_type == rhs_type, - }, - } - } -} diff --git a/crates/noirc_frontend/src/ast/mod.rs b/crates/noirc_frontend/src/ast/mod.rs index e92d333fd69..a91c6033398 100644 --- a/crates/noirc_frontend/src/ast/mod.rs +++ b/crates/noirc_frontend/src/ast/mod.rs @@ -187,6 +187,15 @@ impl UnresolvedTypeData { } } +impl From for UnresolvedType { + fn from(item: FunctionReturnType) -> Self { + match item { + FunctionReturnType::Default(_span) => UnresolvedType::Unit, + FunctionReturnType::Ty(typ, _span) => typ.clone(), + } + } +} + #[derive(Debug, PartialEq, Eq, Copy, Clone, Hash)] pub enum Signedness { Unsigned, 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 ccb7215f0e0..8dbc8ee1624 100644 --- a/crates/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/crates/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -104,7 +104,7 @@ fn check_trait_method_implementation_return_type( impl_method: &NoirFunction, trait_name: &str, ) -> Result<(), DefCollectorErrorKind> { - if expected_return_type.is_equivalent(&impl_method.def.return_type) { + if UnresolvedType::from(expected_return_type.clone()) == UnresolvedType::from(impl_method.def.return_type.clone()) { Ok(()) } else { Err(DefCollectorErrorKind::MismatchTraitImplementationReturnType { @@ -337,8 +337,6 @@ impl<'a> ModCollector<'a> { trait_impl_span: trait_impl.object_type_span, }; errors.push(error.into_file_diagnostic(self.file_id)); - - // Emit error that implementation of trait method is missing } } } From 87ed6e22fb7978e42b698398e93c5bbf5e0606b1 Mon Sep 17 00:00:00 2001 From: Yordan Madzhunkov Date: Fri, 25 Aug 2023 15:40:05 +0300 Subject: [PATCH 22/31] Fix clippy warnings --- crates/noirc_frontend/src/ast/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/noirc_frontend/src/ast/mod.rs b/crates/noirc_frontend/src/ast/mod.rs index a91c6033398..dd53b799d5b 100644 --- a/crates/noirc_frontend/src/ast/mod.rs +++ b/crates/noirc_frontend/src/ast/mod.rs @@ -191,7 +191,7 @@ impl From for UnresolvedType { fn from(item: FunctionReturnType) -> Self { match item { FunctionReturnType::Default(_span) => UnresolvedType::Unit, - FunctionReturnType::Ty(typ, _span) => typ.clone(), + FunctionReturnType::Ty(typ, _span) => typ, } } } From 28edd7751355483127a9a7ebe1fb882385098d68 Mon Sep 17 00:00:00 2001 From: Yordan Madzhunkov Date: Fri, 25 Aug 2023 16:56:52 +0300 Subject: [PATCH 23/31] Add test of override ot Trait default method --- .../trait_default_implementation/Nargo.toml | 2 +- .../trait_override_implementation/Nargo.toml | 7 +++++ .../trait_override_implementation/Prover.toml | 2 ++ .../trait_override_implementation/src/main.nr | 30 +++++++++++++++++++ 4 files changed, 40 insertions(+), 1 deletion(-) create mode 100644 crates/nargo_cli/tests/execution_success/trait_override_implementation/Nargo.toml create mode 100644 crates/nargo_cli/tests/execution_success/trait_override_implementation/Prover.toml create mode 100644 crates/nargo_cli/tests/execution_success/trait_override_implementation/src/main.nr diff --git a/crates/nargo_cli/tests/execution_success/trait_default_implementation/Nargo.toml b/crates/nargo_cli/tests/execution_success/trait_default_implementation/Nargo.toml index 75fb80c4bfa..7fcb4d0281e 100644 --- a/crates/nargo_cli/tests/execution_success/trait_default_implementation/Nargo.toml +++ b/crates/nargo_cli/tests/execution_success/trait_default_implementation/Nargo.toml @@ -1,5 +1,5 @@ [package] -name = "traits" +name = "trait_default_implementation" type = "bin" authors = [""] compiler_version = "0.1" diff --git a/crates/nargo_cli/tests/execution_success/trait_override_implementation/Nargo.toml b/crates/nargo_cli/tests/execution_success/trait_override_implementation/Nargo.toml new file mode 100644 index 00000000000..7ab5a62b5ba --- /dev/null +++ b/crates/nargo_cli/tests/execution_success/trait_override_implementation/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "trait_override_implementation" +type = "bin" +authors = [""] +compiler_version = "0.1" + +[dependencies] diff --git a/crates/nargo_cli/tests/execution_success/trait_override_implementation/Prover.toml b/crates/nargo_cli/tests/execution_success/trait_override_implementation/Prover.toml new file mode 100644 index 00000000000..71805e71e8e --- /dev/null +++ b/crates/nargo_cli/tests/execution_success/trait_override_implementation/Prover.toml @@ -0,0 +1,2 @@ +x = "5" +y = "1" \ No newline at end of file diff --git a/crates/nargo_cli/tests/execution_success/trait_override_implementation/src/main.nr b/crates/nargo_cli/tests/execution_success/trait_override_implementation/src/main.nr new file mode 100644 index 00000000000..92e19f97d50 --- /dev/null +++ b/crates/nargo_cli/tests/execution_success/trait_override_implementation/src/main.nr @@ -0,0 +1,30 @@ +use dep::std; + +trait Default { + fn default(x: Field, y: Field) -> Self; + + fn method2(x: Field) -> Field { + x + } + +} + +struct Foo { + bar: Field, + array: [Field; 2], +} + +impl Default for Foo { + fn default(x: Field,y: Field) -> Self { + Self { bar: x, array: [x,y] } + } + + fn method2(x: Field) -> Field { + x * 3 + } +} + +fn main(x: Field) { + let first = Foo::method2(x); + assert(first == 3 * x); +} From d2873efe4ea5d6a56749e2b2aa791836fc431709 Mon Sep 17 00:00:00 2001 From: Yordan Madzhunkov Date: Fri, 25 Aug 2023 17:32:48 +0300 Subject: [PATCH 24/31] Try to fix 'nix flake check -L' --- crates/noirc_frontend/src/ast/expression.rs | 1 - .../noirc_frontend/src/hir/def_collector/dc_mod.rs | 12 ++++++++---- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/crates/noirc_frontend/src/ast/expression.rs b/crates/noirc_frontend/src/ast/expression.rs index d65138ea1a2..170d3c569c9 100644 --- a/crates/noirc_frontend/src/ast/expression.rs +++ b/crates/noirc_frontend/src/ast/expression.rs @@ -663,4 +663,3 @@ impl Display for FunctionReturnType { } } } - 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 8dbc8ee1624..49931437f63 100644 --- a/crates/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/crates/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -18,6 +18,8 @@ use crate::hir::def_map::{parse_file, LocalModuleId, ModuleData, ModuleDefId, Mo use crate::hir::resolution::import::ImportDirective; use crate::hir::Context; +use noirc_abi::{AbiDistinctness, AbiVisibility}; + /// Given a module collect all definitions into ModuleData struct ModCollector<'a> { pub(crate) def_collector: &'a mut DefCollector, @@ -104,7 +106,9 @@ fn check_trait_method_implementation_return_type( impl_method: &NoirFunction, trait_name: &str, ) -> Result<(), DefCollectorErrorKind> { - if UnresolvedType::from(expected_return_type.clone()) == UnresolvedType::from(impl_method.def.return_type.clone()) { + if UnresolvedType::from(expected_return_type.clone()) + == UnresolvedType::from(impl_method.def.return_type.clone()) + { Ok(()) } else { Err(DefCollectorErrorKind::MismatchTraitImplementationReturnType { @@ -310,7 +314,7 @@ impl<'a> ModCollector<'a> { ( Pattern::Identifier(ident.clone()), unresolved_type.clone(), - noirc_abi::AbiVisibility::Private, + AbiVisibility::Private, ) }) .collect(); @@ -326,8 +330,8 @@ impl<'a> ModCollector<'a> { span: name.span(), where_clause: where_clause.clone(), return_type: return_type.clone(), - return_visibility: noirc_abi::AbiVisibility::Private, - return_distinctness: noirc_abi::AbiDistinctness::DuplicationAllowed, + return_visibility: AbiVisibility::Private, + return_distinctness: AbiDistinctness::DuplicationAllowed, }); unresolved_functions.push_fn(self.module_id, func_id, impl_method); } else { From dd88b9673ca483fdc34b911be24f3cad85bf9069 Mon Sep 17 00:00:00 2001 From: Yordan Madzhunkov Date: Sat, 26 Aug 2023 10:12:51 +0300 Subject: [PATCH 25/31] Try to fix CI dependency failed build --- crates/noirc_frontend/src/ast/expression.rs | 37 ++++++++++ crates/noirc_frontend/src/ast/mod.rs | 4 +- .../src/hir/def_collector/dc_mod.rs | 72 ++++++++----------- 3 files changed, 67 insertions(+), 46 deletions(-) diff --git a/crates/noirc_frontend/src/ast/expression.rs b/crates/noirc_frontend/src/ast/expression.rs index 170d3c569c9..173e6dc2b30 100644 --- a/crates/noirc_frontend/src/ast/expression.rs +++ b/crates/noirc_frontend/src/ast/expression.rs @@ -626,6 +626,43 @@ impl Display for Lambda { } } +impl FunctionDefinition { + pub fn normal( + name: &Ident, + generics: &UnresolvedGenerics, + parameters: &[(Ident, UnresolvedType)], + body: &BlockExpression, + where_clause: &[TraitConstraint], + return_type: &FunctionReturnType, + ) -> FunctionDefinition { + let p = parameters + .iter() + .map(|(ident, unresolved_type)| { + ( + Pattern::Identifier(ident.clone()), + unresolved_type.clone(), + noirc_abi::AbiVisibility::Private, + ) + }) + .collect(); + FunctionDefinition { + name: name.clone(), + attribute: None, + is_open: false, + is_internal: false, + is_unconstrained: false, + generics: generics.clone(), + parameters: p, + body: body.clone(), + span: name.span(), + where_clause: where_clause.to_vec(), + return_type: return_type.clone(), + return_visibility: noirc_abi::AbiVisibility::Private, + return_distinctness: noirc_abi::AbiDistinctness::DuplicationAllowed, + } + } +} + impl Display for FunctionDefinition { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { if let Some(attribute) = &self.attribute { diff --git a/crates/noirc_frontend/src/ast/mod.rs b/crates/noirc_frontend/src/ast/mod.rs index dd53b799d5b..b25fc67551d 100644 --- a/crates/noirc_frontend/src/ast/mod.rs +++ b/crates/noirc_frontend/src/ast/mod.rs @@ -188,10 +188,10 @@ impl UnresolvedTypeData { } impl From for UnresolvedType { - fn from(item: FunctionReturnType) -> Self { + pub fn from_func_return_type(item: &FunctionReturnType) -> Self { match item { FunctionReturnType::Default(_span) => UnresolvedType::Unit, - FunctionReturnType::Ty(typ, _span) => typ, + FunctionReturnType::Ty(typ, _span) => typ.clone(), } } } 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 49931437f63..7d5cd7fa599 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,9 @@ use crate::{ hir::def_collector::dc_crate::{UnresolvedStruct, UnresolvedTrait}, node_interner::{StructId, TraitId}, parser::SubModule, - FunctionReturnType, Ident, LetStatement, NoirFunction, NoirStruct, NoirTrait, NoirTypeAlias, - ParsedModule, Pattern, TraitImpl, TraitImplItem, TraitItem, TypeImpl, UnresolvedType, + FunctionDefinition, FunctionReturnType, Ident, LetStatement, NoirFunction, NoirStruct, + NoirTrait, NoirTypeAlias, ParsedModule, TraitImpl, TraitImplItem, TraitItem, TypeImpl, + UnresolvedType, }; use super::{ @@ -18,8 +19,6 @@ use crate::hir::def_map::{parse_file, LocalModuleId, ModuleData, ModuleDefId, Mo use crate::hir::resolution::import::ImportDirective; use crate::hir::Context; -use noirc_abi::{AbiDistinctness, AbiVisibility}; - /// Given a module collect all definitions into ModuleData struct ModCollector<'a> { pub(crate) def_collector: &'a mut DefCollector, @@ -106,8 +105,8 @@ fn check_trait_method_implementation_return_type( impl_method: &NoirFunction, trait_name: &str, ) -> Result<(), DefCollectorErrorKind> { - if UnresolvedType::from(expected_return_type.clone()) - == UnresolvedType::from(impl_method.def.return_type.clone()) + if UnresolvedType::from_func_return_type(expected_return_type) + == UnresolvedType::from_func_return_type(&impl_method.def.return_type) { Ok(()) } else { @@ -304,48 +303,33 @@ impl<'a> ModCollector<'a> { .iter() .any(|(_, _, func_impl)| func_impl.name() == name.0.contents); if !is_implemented { - if body.is_some() { - let method_name = name.0.contents.clone(); - let func_id = context.def_interner.push_empty_fn(); - context.def_interner.push_function_definition(method_name, func_id); - let p = parameters - .iter() - .map(|(ident, unresolved_type)| { - ( - Pattern::Identifier(ident.clone()), - unresolved_type.clone(), - AbiVisibility::Private, - ) - }) - .collect(); - let impl_method = NoirFunction::normal(crate::FunctionDefinition { - name: name.clone(), - attribute: None, - is_open: false, - is_internal: false, - is_unconstrained: false, - generics: generics.clone(), - parameters: p, - body: body.as_ref().unwrap().clone(), - span: name.span(), - where_clause: where_clause.clone(), - return_type: return_type.clone(), - return_visibility: AbiVisibility::Private, - return_distinctness: AbiDistinctness::DuplicationAllowed, - }); - unresolved_functions.push_fn(self.module_id, func_id, impl_method); - } else { - let error = DefCollectorErrorKind::TraitMissedMethodImplementation { - trait_name: trait_def.name.clone(), - method_name: name.clone(), - trait_impl_span: trait_impl.object_type_span, - }; - errors.push(error.into_file_diagnostic(self.file_id)); + match body { + Some(body) => { + let method_name = name.0.contents.clone(); + let func_id = context.def_interner.push_empty_fn(); + context.def_interner.push_function_definition(method_name, func_id); + let impl_method = NoirFunction::normal(FunctionDefinition::normal( + name, + generics, + parameters, + body, + where_clause, + return_type, + )); + unresolved_functions.push_fn(self.module_id, func_id, impl_method); + } + None => { + let error = DefCollectorErrorKind::TraitMissedMethodImplementation { + trait_name: trait_def.name.clone(), + method_name: name.clone(), + trait_impl_span: trait_impl.object_type_span, + }; + errors.push(error.into_file_diagnostic(self.file_id)); + } } } } } - unresolved_functions } From 961550b2b4886f60ec8daf474dfe6ca26cd184ee Mon Sep 17 00:00:00 2001 From: Yordan Madzhunkov Date: Sat, 26 Aug 2023 10:26:07 +0300 Subject: [PATCH 26/31] An other try --- crates/noirc_frontend/src/ast/expression.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/noirc_frontend/src/ast/expression.rs b/crates/noirc_frontend/src/ast/expression.rs index 173e6dc2b30..879ee397477 100644 --- a/crates/noirc_frontend/src/ast/expression.rs +++ b/crates/noirc_frontend/src/ast/expression.rs @@ -641,7 +641,7 @@ impl FunctionDefinition { ( Pattern::Identifier(ident.clone()), unresolved_type.clone(), - noirc_abi::AbiVisibility::Private, + Visibility::Private, ) }) .collect(); @@ -657,8 +657,8 @@ impl FunctionDefinition { span: name.span(), where_clause: where_clause.to_vec(), return_type: return_type.clone(), - return_visibility: noirc_abi::AbiVisibility::Private, - return_distinctness: noirc_abi::AbiDistinctness::DuplicationAllowed, + return_visibility: Visibility::Private, + return_distinctness: Distinctness::DuplicationAllowed, } } } From bbe62b69f3530f7893b6d03059e495dbad750241 Mon Sep 17 00:00:00 2001 From: Yordan Madzhunkov <52652109+yordanmadzhunkov@users.noreply.github.com> Date: Tue, 29 Aug 2023 09:15:31 +0300 Subject: [PATCH 27/31] Update crates/noirc_frontend/src/ast/mod.rs convert to `from_func_return_type` to struct method. Definitely a better approach. Co-authored-by: jfecher --- crates/noirc_frontend/src/ast/mod.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/crates/noirc_frontend/src/ast/mod.rs b/crates/noirc_frontend/src/ast/mod.rs index b25fc67551d..e5abab38f9d 100644 --- a/crates/noirc_frontend/src/ast/mod.rs +++ b/crates/noirc_frontend/src/ast/mod.rs @@ -187,14 +187,15 @@ impl UnresolvedTypeData { } } -impl From for UnresolvedType { - pub fn from_func_return_type(item: &FunctionReturnType) -> Self { +impl FunctionReturnType { + pub fn get_type(&self) -> &UnresolvedType { match item { - FunctionReturnType::Default(_span) => UnresolvedType::Unit, - FunctionReturnType::Ty(typ, _span) => typ.clone(), + FunctionReturnType::Default(_span) => &UnresolvedType::Unit, + FunctionReturnType::Ty(typ, _span) => typ, } } } +} #[derive(Debug, PartialEq, Eq, Copy, Clone, Hash)] pub enum Signedness { From 28a3aaa35c41282f46420347a630448b0da944ec Mon Sep 17 00:00:00 2001 From: Yordan Madzhunkov <52652109+yordanmadzhunkov@users.noreply.github.com> Date: Tue, 29 Aug 2023 09:16:23 +0300 Subject: [PATCH 28/31] Update crates/noirc_frontend/src/hir/def_collector/dc_crate.rs clean up Co-authored-by: jfecher --- crates/noirc_frontend/src/hir/def_collector/dc_crate.rs | 1 - 1 file changed, 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 037fa3d6c44..03c0aad7c0f 100644 --- a/crates/noirc_frontend/src/hir/def_collector/dc_crate.rs +++ b/crates/noirc_frontend/src/hir/def_collector/dc_crate.rs @@ -3,7 +3,6 @@ use super::errors::{DefCollectorErrorKind, DuplicateType}; use crate::graph::CrateId; use crate::hir::def_map::{CrateDefMap, LocalModuleId, ModuleId}; use crate::hir::resolution::errors::ResolverError; -//use crate::hir::resolution::errors::ResolverError::PathResolutionError; use crate::hir::resolution::import::PathResolutionError; use crate::hir::resolution::resolver::Resolver; use crate::hir::resolution::{ From 2ef48e2417dfa814f9138c241b7ba966b71e3ec4 Mon Sep 17 00:00:00 2001 From: Yordan Madzhunkov Date: Tue, 29 Aug 2023 09:39:29 +0300 Subject: [PATCH 29/31] Build fix and cargo fmt --- crates/noirc_frontend/src/ast/expression.rs | 15 ++++++++++----- crates/noirc_frontend/src/ast/mod.rs | 10 ---------- .../src/hir/def_collector/dc_mod.rs | 4 +--- 3 files changed, 11 insertions(+), 18 deletions(-) diff --git a/crates/noirc_frontend/src/ast/expression.rs b/crates/noirc_frontend/src/ast/expression.rs index 879ee397477..f6e085cc95b 100644 --- a/crates/noirc_frontend/src/ast/expression.rs +++ b/crates/noirc_frontend/src/ast/expression.rs @@ -638,11 +638,7 @@ impl FunctionDefinition { let p = parameters .iter() .map(|(ident, unresolved_type)| { - ( - Pattern::Identifier(ident.clone()), - unresolved_type.clone(), - Visibility::Private, - ) + (Pattern::Identifier(ident.clone()), unresolved_type.clone(), Visibility::Private) }) .collect(); FunctionDefinition { @@ -692,6 +688,15 @@ impl Display for FunctionDefinition { } } +impl FunctionReturnType { + pub fn get_type(&self) -> &UnresolvedType { + match self { + FunctionReturnType::Default(_span) => &UnresolvedType::Unit, + FunctionReturnType::Ty(typ, _span) => typ, + } + } +} + impl Display for FunctionReturnType { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { diff --git a/crates/noirc_frontend/src/ast/mod.rs b/crates/noirc_frontend/src/ast/mod.rs index e5abab38f9d..e92d333fd69 100644 --- a/crates/noirc_frontend/src/ast/mod.rs +++ b/crates/noirc_frontend/src/ast/mod.rs @@ -187,16 +187,6 @@ impl UnresolvedTypeData { } } -impl FunctionReturnType { - pub fn get_type(&self) -> &UnresolvedType { - match item { - FunctionReturnType::Default(_span) => &UnresolvedType::Unit, - FunctionReturnType::Ty(typ, _span) => typ, - } - } -} -} - #[derive(Debug, PartialEq, Eq, Copy, Clone, Hash)] pub enum Signedness { Unsigned, 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 7d5cd7fa599..dad2c25eb6e 100644 --- a/crates/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/crates/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -105,9 +105,7 @@ fn check_trait_method_implementation_return_type( impl_method: &NoirFunction, trait_name: &str, ) -> Result<(), DefCollectorErrorKind> { - if UnresolvedType::from_func_return_type(expected_return_type) - == UnresolvedType::from_func_return_type(&impl_method.def.return_type) - { + if expected_return_type.get_type() == impl_method.def.return_type.get_type() { Ok(()) } else { Err(DefCollectorErrorKind::MismatchTraitImplementationReturnType { From 05b5208eba3f947945ca5b393da92d3a093ca97d Mon Sep 17 00:00:00 2001 From: Yordan Madzhunkov Date: Tue, 29 Aug 2023 10:29:09 +0300 Subject: [PATCH 30/31] Fix build after rebase of master --- crates/noirc_frontend/src/ast/expression.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/noirc_frontend/src/ast/expression.rs b/crates/noirc_frontend/src/ast/expression.rs index f6e085cc95b..6c29c89051f 100644 --- a/crates/noirc_frontend/src/ast/expression.rs +++ b/crates/noirc_frontend/src/ast/expression.rs @@ -3,7 +3,7 @@ use std::fmt::Display; use crate::token::{Attribute, Token}; use crate::{ Distinctness, Ident, Path, Pattern, Recoverable, Statement, TraitConstraint, UnresolvedType, - Visibility, + UnresolvedTypeData, Visibility, }; use acvm::FieldElement; use iter_extended::vecmap; @@ -689,10 +689,10 @@ impl Display for FunctionDefinition { } impl FunctionReturnType { - pub fn get_type(&self) -> &UnresolvedType { + pub fn get_type(&self) -> &UnresolvedTypeData { match self { - FunctionReturnType::Default(_span) => &UnresolvedType::Unit, - FunctionReturnType::Ty(typ, _span) => typ, + FunctionReturnType::Default(_span) => &UnresolvedTypeData::Unit, + FunctionReturnType::Ty(typ, _span) => &typ.typ, } } } From 43ec02d8eea523c357d96e0eb68c1f4305398a0c Mon Sep 17 00:00:00 2001 From: Yordan Madzhunkov Date: Tue, 29 Aug 2023 11:41:19 +0300 Subject: [PATCH 31/31] Fix trait tests --- crates/noirc_frontend/src/hir/def_collector/dc_mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 dad2c25eb6e..f72b72f54df 100644 --- a/crates/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/crates/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -88,7 +88,7 @@ fn check_trait_method_implementation_parameters( } for (count, (parameter, typ, _abi_vis)) in impl_method.def.parameters.iter().enumerate() { let (_expected_name, expected_type) = &expected_parameters[count]; - if typ != expected_type { + if typ.typ != expected_type.typ { return Err(DefCollectorErrorKind::MismatchTraitImlementationParameter { trait_name: trait_name.to_owned(), expected_type: expected_type.clone(),