From faec260955202a9a911cbba9227a4ee9d8b59c75 Mon Sep 17 00:00:00 2001 From: Yordan Madzhunkov Date: Thu, 20 Jul 2023 16:30:57 +0300 Subject: [PATCH 01/27] Temp commit, code compiles --- .../test_data_ssa_refactor/traits/Nargo.toml | 7 +++ .../test_data_ssa_refactor/traits/Prover.toml | 2 + .../test_data_ssa_refactor/traits/src/main.nr | 22 ++++++++++ crates/noirc_frontend/src/ast/traits.rs | 1 + .../src/hir/def_collector/dc_crate.rs | 43 +++++++++++++++++-- .../src/hir/def_collector/dc_mod.rs | 38 +++++++++++++++- .../src/hir/def_map/item_scope.rs | 1 + .../src/hir/def_map/module_data.rs | 7 ++- .../src/hir/def_map/module_def.rs | 30 ++++++++++++- .../src/hir/resolution/import.rs | 4 +- .../src/hir/resolution/resolver.rs | 43 +++++++++++++++++-- crates/noirc_frontend/src/hir_def/types.rs | 39 ++++++++++++++++- crates/noirc_frontend/src/node_interner.rs | 41 +++++++++++++++++- crates/noirc_frontend/src/parser/parser.rs | 10 ++--- 14 files changed, 267 insertions(+), 21 deletions(-) create mode 100644 crates/nargo_cli/tests/test_data_ssa_refactor/traits/Nargo.toml create mode 100644 crates/nargo_cli/tests/test_data_ssa_refactor/traits/Prover.toml create mode 100644 crates/nargo_cli/tests/test_data_ssa_refactor/traits/src/main.nr diff --git a/crates/nargo_cli/tests/test_data_ssa_refactor/traits/Nargo.toml b/crates/nargo_cli/tests/test_data_ssa_refactor/traits/Nargo.toml new file mode 100644 index 00000000000..d9434868157 --- /dev/null +++ b/crates/nargo_cli/tests/test_data_ssa_refactor/traits/Nargo.toml @@ -0,0 +1,7 @@ + + [package] + authors = [""] + compiler_version = "0.1" + + [dependencies] + \ No newline at end of file diff --git a/crates/nargo_cli/tests/test_data_ssa_refactor/traits/Prover.toml b/crates/nargo_cli/tests/test_data_ssa_refactor/traits/Prover.toml new file mode 100644 index 00000000000..7d59cc81807 --- /dev/null +++ b/crates/nargo_cli/tests/test_data_ssa_refactor/traits/Prover.toml @@ -0,0 +1,2 @@ +x = "0" +y = "1" \ No newline at end of file diff --git a/crates/nargo_cli/tests/test_data_ssa_refactor/traits/src/main.nr b/crates/nargo_cli/tests/test_data_ssa_refactor/traits/src/main.nr new file mode 100644 index 00000000000..ccace748f0e --- /dev/null +++ b/crates/nargo_cli/tests/test_data_ssa_refactor/traits/src/main.nr @@ -0,0 +1,22 @@ +use dep::std; + +trait Animal { + fn default(x: Field, y: Field) -> Self; +} + +struct Foo { + bar: Field, + array: [Field; 2], +} + + +impl Animal for Foo { + fn default(x: Field,y: Field) -> Self { + Self { bar: 0, array: [x,y] } + } +} + +fn main(x: Field, y: Field) { + let first = Foo::default(x,y); + assert(first == x); +} diff --git a/crates/noirc_frontend/src/ast/traits.rs b/crates/noirc_frontend/src/ast/traits.rs index 1dacb73fc5e..30b25ae6e45 100644 --- a/crates/noirc_frontend/src/ast/traits.rs +++ b/crates/noirc_frontend/src/ast/traits.rs @@ -11,6 +11,7 @@ use crate::{Ident, NoirFunction, UnresolvedGenerics, UnresolvedType}; pub struct NoirTrait { pub name: Ident, pub generics: Vec, + pub span: Span, pub items: Vec, } 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 e974961a405..83078b80117 100644 --- a/crates/noirc_frontend/src/hir/def_collector/dc_crate.rs +++ b/crates/noirc_frontend/src/hir/def_collector/dc_crate.rs @@ -10,10 +10,11 @@ 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, TypeAliasId, TraitId}; use crate::{ - ExpressionKind, Generics, Ident, LetStatement, NoirFunction, NoirStruct, NoirTypeAlias, - ParsedModule, Shared, Type, TypeBinding, UnresolvedGenerics, UnresolvedType, + ExpressionKind, Generics, Ident, LetStatement, NoirFunction, NoirStruct, NoirTypeAlias, NoirTrait, + ParsedModule, Shared, Type, TypeBinding, UnresolvedGenerics, UnresolvedType, + }; use fm::FileId; use iter_extended::vecmap; @@ -40,6 +41,13 @@ 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, @@ -62,6 +70,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, } @@ -80,6 +89,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![], } @@ -95,6 +105,7 @@ impl DefCollector { root_file_id: FileId, errors: &mut Vec, ) { + println!("\n Hohoh"); let crate_id = def_map.krate; // Recursively resolve the dependencies @@ -169,6 +180,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 @@ -350,6 +362,31 @@ fn resolve_structs( } } +/// Create the mappings from TypeId -> StructType +/// so that expressions can access the fields of structs +fn resolve_traits( + context: &mut Context, + traits: HashMap, + crate_id: CrateId, + errors: &mut Vec, +) { + // We must first go through the struct list once to ensure all IDs are pushed to + // the def_interner map. This lets structs refer to each other regardless of declaration order + // without resolve_struct_fields non-deterministically unwrapping a value + // that isn't in the HashMap. + for (type_id, typ) in &traits { + context.def_interner.push_empty_trait(*type_id, typ); + } +/* + for (type_id, typ) in traits { + let (generics, fields) = resolve_struct_fields(context, crate_id, typ, errors); + context.def_interner.update_trait(type_id, |struct_def| { + struct_def.generics = generics; + }); + } +*/ +} + fn resolve_struct_fields( context: &mut Context, krate: CrateId, diff --git a/crates/noirc_frontend/src/hir/def_collector/dc_mod.rs b/crates/noirc_frontend/src/hir/def_collector/dc_mod.rs index 37c017ecb96..15348a35420 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,8 @@ use fm::FileId; use noirc_errors::FileDiagnostic; 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, NoirTypeAlias, ParsedModule, NoirTrait, TypeImpl, }; @@ -54,6 +54,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); @@ -219,6 +221,38 @@ 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 struct 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); + // And store the TypeId -> StructType 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_map/item_scope.rs b/crates/noirc_frontend/src/hir/def_map/item_scope.rs index 760088a3b7e..e0577f98bcc 100644 --- a/crates/noirc_frontend/src/hir/def_map/item_scope.rs +++ b/crates/noirc_frontend/src/hir/def_map/item_scope.rs @@ -49,6 +49,7 @@ impl ItemScope { ModuleDefId::FunctionId(_) => add_item(&mut self.values), ModuleDefId::TypeId(_) => add_item(&mut self.types), ModuleDefId::TypeAliasId(_) => add_item(&mut self.types), + ModuleDefId::InterfaceId(_) => add_item(&mut self.types), ModuleDefId::GlobalId(_) => add_item(&mut self.values), } } diff --git a/crates/noirc_frontend/src/hir/def_map/module_data.rs b/crates/noirc_frontend/src/hir/def_map/module_data.rs index 5b93d04fea7..3ea3bcc762d 100644 --- a/crates/noirc_frontend/src/hir/def_map/module_data.rs +++ b/crates/noirc_frontend/src/hir/def_map/module_data.rs @@ -3,7 +3,7 @@ use std::collections::HashMap; use fm::FileId; use crate::{ - node_interner::{FuncId, StmtId, StructId, TypeAliasId}, + node_interner::{FuncId, StmtId, StructId, TypeAliasId, TraitId}, Ident, }; @@ -65,6 +65,7 @@ impl ModuleData { self.declare(name, ModuleDefId::TypeId(id)) } + pub fn declare_type_alias( &mut self, name: Ident, @@ -73,6 +74,10 @@ impl ModuleData { self.declare(name, id.into()) } + pub fn declare_trait(&mut self, name: Ident, id: TraitId) -> Result<(), (Ident, Ident)> { + self.declare(name, ModuleDefId::InterfaceId(id)) + } + pub fn declare_child_module( &mut self, name: Ident, diff --git a/crates/noirc_frontend/src/hir/def_map/module_def.rs b/crates/noirc_frontend/src/hir/def_map/module_def.rs index b64ced78772..51e50363ca4 100644 --- a/crates/noirc_frontend/src/hir/def_map/module_def.rs +++ b/crates/noirc_frontend/src/hir/def_map/module_def.rs @@ -1,14 +1,16 @@ -use crate::node_interner::{FuncId, StmtId, StructId, TypeAliasId}; + +use crate::node_interner::{FuncId, StmtId, StructId, TypeAliasId, TraitId}; use super::ModuleId; -/// A generic ID that references either a module, function, type, or global +/// A generic ID that references either a module, function, type, interface or global #[derive(Debug, Copy, Clone, PartialEq, Eq)] pub enum ModuleDefId { ModuleId(ModuleId), FunctionId(FuncId), TypeId(StructId), TypeAliasId(TypeAliasId), + InterfaceId(TraitId), GlobalId(StmtId), } @@ -34,6 +36,13 @@ impl ModuleDefId { } } + pub fn as_trait(&self) -> Option { + match self { + ModuleDefId::InterfaceId(trait_id) => Some(*trait_id), + _ => None, + } + } + pub fn as_global(&self) -> Option { match self { ModuleDefId::GlobalId(stmt_id) => Some(*stmt_id), @@ -48,6 +57,7 @@ impl ModuleDefId { ModuleDefId::FunctionId(_) => "function", ModuleDefId::TypeId(_) => "type", ModuleDefId::TypeAliasId(_) => "type alias", + ModuleDefId::InterfaceId(_) => "trait", ModuleDefId::ModuleId(_) => "module", ModuleDefId::GlobalId(_) => "global", } @@ -112,6 +122,7 @@ impl TryFromModuleDefId for StructId { } } + impl TryFromModuleDefId for TypeAliasId { fn try_from(id: ModuleDefId) -> Option { id.as_type_alias() @@ -123,6 +134,21 @@ impl TryFromModuleDefId for TypeAliasId { fn description() -> String { "type alias".to_string() + + } +} + +impl TryFromModuleDefId for TraitId { + fn try_from(id: ModuleDefId) -> Option { + id.as_trait() + } + + fn dummy_id() -> Self { + TraitId::dummy_id() + } + + fn description() -> String { + "interface".to_string() } } diff --git a/crates/noirc_frontend/src/hir/resolution/import.rs b/crates/noirc_frontend/src/hir/resolution/import.rs index 9a6ef9b1b8b..c7a94cf4bd4 100644 --- a/crates/noirc_frontend/src/hir/resolution/import.rs +++ b/crates/noirc_frontend/src/hir/resolution/import.rs @@ -55,7 +55,7 @@ pub fn resolve_imports( def_maps: &HashMap, ) -> (Vec, Vec<(PathResolutionError, LocalModuleId)>) { let def_map = &def_maps[&crate_id]; - + partition_results(imports_to_resolve, |import_directive| { let allow_contracts = allow_referencing_contracts(def_maps, crate_id, import_directive.module_id); @@ -64,7 +64,6 @@ pub fn resolve_imports( let resolved_namespace = resolve_path_to_ns(&import_directive, def_map, def_maps, allow_contracts) .map_err(|error| (error, module_scope))?; - let name = resolve_path_name(&import_directive); Ok(ResolvedImport { name, resolved_namespace, module_scope }) }) @@ -153,6 +152,7 @@ fn resolve_name_in_module( // TODO: If impls are ever implemented, types can be used in a path ModuleDefId::TypeId(id) => id.0, ModuleDefId::TypeAliasId(_) => panic!("type aliases cannot be used in type namespace"), + ModuleDefId::InterfaceId(id) => id.0, ModuleDefId::GlobalId(_) => panic!("globals cannot be in the type namespace"), }; diff --git a/crates/noirc_frontend/src/hir/resolution/resolver.rs b/crates/noirc_frontend/src/hir/resolution/resolver.rs index 8b4f97dbd8e..f2752620f8a 100644 --- a/crates/noirc_frontend/src/hir/resolution/resolver.rs +++ b/crates/noirc_frontend/src/hir/resolution/resolver.rs @@ -26,7 +26,7 @@ use crate::graph::CrateId; use crate::hir::def_map::{ModuleDefId, ModuleId, TryFromModuleDefId, MAIN_FUNCTION}; use crate::hir_def::stmt::{HirAssignStatement, HirLValue, HirPattern}; use crate::node_interner::{ - DefinitionId, DefinitionKind, ExprId, FuncId, NodeInterner, StmtId, StructId, + DefinitionId, DefinitionKind, ExprId, FuncId, NodeInterner, StmtId, StructId, TraitId, }; use crate::{ hir::{def_map::CrateDefMap, resolution::path_resolver::PathResolver}, @@ -36,7 +36,7 @@ use crate::{ use crate::{ ArrayLiteral, ContractFunctionType, Generics, LValue, NoirStruct, NoirTypeAlias, Path, Pattern, Shared, StructType, Type, TypeAliasType, TypeBinding, TypeVariable, UnaryOp, - UnresolvedGenerics, UnresolvedType, UnresolvedTypeExpression, ERROR_IDENT, + UnresolvedGenerics, UnresolvedType, UnresolvedTypeExpression, ERROR_IDENT, TraitType, }; use fm::FileId; use iter_extended::vecmap; @@ -1227,6 +1227,10 @@ impl<'a> Resolver<'a> { self.interner.get_struct(type_id) } + pub fn get_trait(&self, type_id: TraitId) -> Shared { + self.interner.get_trait(type_id) + } + fn lookup(&mut self, path: Path) -> Result { let span = path.span(); let id = self.resolve_path(path)?; @@ -1434,6 +1438,7 @@ mod test { // and functions can be forward declared fn resolve_src_code(src: &str, func_namespace: Vec<&str>) -> Vec { let (program, errors) = parse_program(src); + println!("{:?}", errors); assert!(errors.is_empty()); let mut interner = NodeInterner::default(); @@ -1637,7 +1642,7 @@ mod test { x } "#; - + let errors = resolve_src_code(src, vec!["main", "foo"]); assert!(errors.is_empty()); } @@ -1694,6 +1699,38 @@ mod test { } } + fn resolve_struct_method_call_expr() { + let src = r#" + + struct Foo { + bar: Field + } + + impl Foo { + fn default(x: Field,_y: Field) -> Self { + Self { bar: x } + } + } + + fn main(x: Field, y: Field) { + let _foo = Foo::default(x,y); + } + "#; + + let errors = resolve_src_code(src, vec!["main", "Foo"]); + for err in &errors { + match err { + ResolverError::PathResolutionError(PathResolutionError::Unresolved(name)) => { + println!("\n\n name = {}\n\n", name.to_string()); + assert_eq!(name.to_string(), "default"); + } + _ => unimplemented!("expected an unresolved path"), + } + } + + assert!(errors.is_empty()); + } + fn path_unresolved_error(err: ResolverError, expected_unresolved_path: &str) { match err { ResolverError::PathResolutionError(PathResolutionError::Unresolved(name)) => { diff --git a/crates/noirc_frontend/src/hir_def/types.rs b/crates/noirc_frontend/src/hir_def/types.rs index df4c2f6c229..553a21b654e 100644 --- a/crates/noirc_frontend/src/hir_def/types.rs +++ b/crates/noirc_frontend/src/hir_def/types.rs @@ -13,7 +13,7 @@ use iter_extended::vecmap; use noirc_abi::AbiType; use noirc_errors::Span; -use crate::{node_interner::StructId, Ident, Signedness}; +use crate::{node_interner::StructId, node_interner::TraitId, Ident, Signedness}; use super::expr::{HirCallExpression, HirExpression, HirIdent}; @@ -124,6 +124,21 @@ pub struct StructType { pub span: Span, } + +// Represents a struct type in the type system. Each instance of this +/// rust struct will be shared across all Type::Struct variants that represent +/// the same struct type. +#[derive(Debug, Eq)] +pub struct TraitType { + /// A unique id representing this struct type. Used to check if two + /// struct types are equal. + pub id: TraitId, + + pub name: Ident, + pub span: Span, +} + + /// Corresponds to generic lists such as `` in the source /// program. The `TypeVariableId` portion is used to match two /// type variables to check for equality, while the `TypeVariable` is @@ -142,6 +157,28 @@ impl PartialEq for StructType { } } +impl std::hash::Hash for TraitType { + fn hash(&self, state: &mut H) { + self.id.hash(state); + } +} + +impl PartialEq for TraitType { + fn eq(&self, other: &Self) -> bool { + self.id == other.id + } +} + +impl TraitType { + pub fn new( + id: TraitId, + name: Ident, + span: Span, + ) -> TraitType { + TraitType { id, name, span} + } +} + impl StructType { pub fn new( id: StructId, diff --git a/crates/noirc_frontend/src/node_interner.rs b/crates/noirc_frontend/src/node_interner.rs index f5fea5c1ea7..fb06ee4362e 100644 --- a/crates/noirc_frontend/src/node_interner.rs +++ b/crates/noirc_frontend/src/node_interner.rs @@ -7,11 +7,11 @@ 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, UnresolvedTypeAlias, UnresolvedTrait}; use crate::hir::def_map::{LocalModuleId, ModuleId}; use crate::hir::StorageSlot; use crate::hir_def::stmt::HirLetStatement; -use crate::hir_def::types::{StructType, Type}; +use crate::hir_def::types::{StructType, TraitType, Type}; use crate::hir_def::{ expr::HirExpression, function::{FuncMeta, HirFunction}, @@ -60,6 +60,13 @@ pub struct NodeInterner { // Map type aliases to the actual type. // When resolving types, check against this map to see if a type alias is defined. type_aliases: Vec, + + // Trait map. + // + // Each struct definition is possibly shared across multiple type nodes. + // It is also mutated through the RefCell during name resolution to append + // methods from impls to the type. + 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 @@ -150,6 +157,18 @@ impl TypeAliasId { } } +#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)] +pub struct TraitId(pub ModuleId); + +impl TraitId { + // dummy id for error reporting + // This can be anything, as the program will ultimately fail + // after resolution + pub fn dummy_id() -> TraitId { + TraitId(ModuleId { krate: CrateId::dummy_id(), local_id: LocalModuleId::dummy_id() }) + } +} + macro_rules! into_index { ($id_type:ty) => { impl From<$id_type> for Index { @@ -262,6 +281,7 @@ impl Default for NodeInterner { id_to_type: HashMap::new(), structs: HashMap::new(), type_aliases: Vec::new(), + traits: HashMap::new(), instantiation_bindings: HashMap::new(), field_indices: HashMap::new(), next_type_variable_id: 0, @@ -304,6 +324,19 @@ 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(TraitType::new( + type_id, + typ.trait_def.name.clone(), + typ.trait_def.span, + )), + ); + } + + + pub fn push_empty_struct(&mut self, type_id: StructId, typ: &UnresolvedStruct) { self.structs.insert( type_id, @@ -551,6 +584,10 @@ impl NodeInterner { &self.type_aliases[id.0] } + pub fn get_trait(&self, id: TraitId) -> Shared { + self.traits[&id].clone() + } + pub fn get_global(&self, stmt_id: &StmtId) -> Option { self.globals.get(stmt_id).cloned() } diff --git a/crates/noirc_frontend/src/parser/parser.rs b/crates/noirc_frontend/src/parser/parser.rs index 6445205eae6..23f3885cda7 100644 --- a/crates/noirc_frontend/src/parser/parser.rs +++ b/crates/noirc_frontend/src/parser/parser.rs @@ -368,8 +368,8 @@ fn trait_definition() -> impl NoirParser { .then(trait_body()) .then_ignore(just(Token::RightBrace)) .validate(|((name, generics), items), span, emit| { - emit(ParserError::with_reason(ParserErrorReason::TraitsAreExperimental, span)); - TopLevelStatement::Trait(NoirTrait { name, generics, items }) + //emit(ParserError::with_reason(ParserErrorReason::TraitsAreExperimental, span)); + TopLevelStatement::Trait(NoirTrait { name, generics, span, items }) }) } @@ -467,7 +467,7 @@ fn trait_implementation() -> impl NoirParser { let (((impl_generics, trait_name), trait_generics), (object_type, object_type_span)) = other_args; - emit(ParserError::with_reason(ParserErrorReason::TraitsAreExperimental, span)); + //emit(ParserError::with_reason(ParserErrorReason::TraitsAreExperimental, span)); TopLevelStatement::TraitImpl(TraitImpl { impl_generics, trait_name, @@ -498,8 +498,8 @@ fn where_clause() -> impl NoirParser> { .then_ignore(just(Token::Colon)) .then(ident()) .then(generic_type_args(parse_type())) - .validate(|((typ, trait_name), trait_generics), span, emit| { - emit(ParserError::with_reason(ParserErrorReason::TraitsAreExperimental, span)); + .validate(|((typ, trait_name), trait_generics), _span, _emit| { + //emit(ParserError::with_reason(ParserErrorReason::TraitsAreExperimental, span)); TraitConstraint { typ, trait_name, trait_generics } }); From 01f3dddb5203d334f0fa9cbe06f2579a2afc64ad Mon Sep 17 00:00:00 2001 From: Yordan Madzhunkov Date: Thu, 20 Jul 2023 17:57:33 +0300 Subject: [PATCH 02/27] One more step ahead --- .../src/hir/resolution/resolver.rs | 5 ++-- crates/noirc_frontend/src/hir_def/types.rs | 24 +++++++++++++++---- .../src/monomorphization/mod.rs | 4 ++++ crates/noirc_frontend/src/node_interner.rs | 7 ++++-- 4 files changed, 32 insertions(+), 8 deletions(-) diff --git a/crates/noirc_frontend/src/hir/resolution/resolver.rs b/crates/noirc_frontend/src/hir/resolution/resolver.rs index f2752620f8a..1622a00c031 100644 --- a/crates/noirc_frontend/src/hir/resolution/resolver.rs +++ b/crates/noirc_frontend/src/hir/resolution/resolver.rs @@ -823,7 +823,8 @@ impl<'a> Resolver<'a> { | Type::Constant(_) | Type::NamedGeneric(_, _) | Type::NotConstant - | Type::Forall(_, _) => (), + | Type::Forall(_, _) + | Type::Trait(_) => (), Type::Array(length, element_type) => { if let Type::NamedGeneric(type_variable, name) = length.as_ref() { @@ -1420,6 +1421,7 @@ mod test { use fm::FileId; use iter_extended::vecmap; + use crate::hir::def_collector; use crate::hir::def_map::{ModuleData, ModuleId, ModuleOrigin}; use crate::hir::resolution::errors::ResolverError; use crate::hir::resolution::import::PathResolutionError; @@ -1438,7 +1440,6 @@ mod test { // and functions can be forward declared fn resolve_src_code(src: &str, func_namespace: Vec<&str>) -> Vec { let (program, errors) = parse_program(src); - println!("{:?}", errors); assert!(errors.is_empty()); let mut interner = NodeInterner::default(); diff --git a/crates/noirc_frontend/src/hir_def/types.rs b/crates/noirc_frontend/src/hir_def/types.rs index 553a21b654e..1e9c7e0f78b 100644 --- a/crates/noirc_frontend/src/hir_def/types.rs +++ b/crates/noirc_frontend/src/hir_def/types.rs @@ -51,6 +51,9 @@ pub enum Type { /// represents the generic arguments (if any) to this struct type. Struct(Shared, Vec), + + Trait(Shared), + /// A tuple type with the given list of fields in the order they appear in source code. Tuple(Vec), @@ -179,6 +182,12 @@ impl TraitType { } } +impl std::fmt::Display for TraitType { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "{}", self.name) + } +} + impl StructType { pub fn new( id: StructId, @@ -721,7 +730,8 @@ impl Type { | Type::Constant(_) | Type::NamedGeneric(_, _) | Type::NotConstant - | Type::Forall(_, _) => false, + | Type::Forall(_, _) + | Type::Trait(_) => false, Type::Array(length, elem) => { elem.contains_numeric_typevar(target_id) || named_generic_id_matches_target(length) @@ -809,6 +819,9 @@ impl std::fmt::Display for Type { write!(f, "{}<{}>", s.borrow(), args.join(", ")) } } + Type::Trait(s) => { + write!(f, "{}", s.borrow()) + } Type::Tuple(elements) => { let elements = vecmap(elements, ToString::to_string); write!(f, "({})", elements.join(", ")) @@ -1534,6 +1547,7 @@ impl Type { let fields = vecmap(fields, |(name, typ)| (name, typ.as_abi_type())); AbiType::Struct { fields } } + Type::Trait(_) => unreachable!(), Type::Tuple(_) => todo!("as_abi_type not yet implemented for tuple types"), Type::TypeVariable(_, _) => unreachable!(), Type::NamedGeneric(..) => unreachable!(), @@ -1668,7 +1682,8 @@ impl Type { | Type::Constant(_) | Type::Error | Type::NotConstant - | Type::Unit => self.clone(), + | Type::Unit + | Type::Trait(_) => self.clone(), } } @@ -1704,7 +1719,8 @@ impl Type { | Type::Constant(_) | Type::Error | Type::NotConstant - | Type::Unit => false, + | Type::Unit + | Type::Trait(_) => false, } } @@ -1747,7 +1763,7 @@ impl Type { MutableReference(element) => MutableReference(Box::new(element.follow_bindings())), // Expect that this function should only be called on instantiated types - Forall(..) => unreachable!(), + Forall(..) | Trait(..) => unreachable!(), FieldElement(_) | Integer(_, _, _) diff --git a/crates/noirc_frontend/src/monomorphization/mod.rs b/crates/noirc_frontend/src/monomorphization/mod.rs index dbe2ee080bf..7fa36b225c8 100644 --- a/crates/noirc_frontend/src/monomorphization/mod.rs +++ b/crates/noirc_frontend/src/monomorphization/mod.rs @@ -652,6 +652,10 @@ impl<'interner> Monomorphizer<'interner> { ast::Type::Tuple(fields) } + HirType::Trait(def) => { + unreachable!("Not sure what to do here def = {:?}", def) + } + HirType::Tuple(fields) => { let fields = vecmap(fields, Self::convert_type); ast::Type::Tuple(fields) diff --git a/crates/noirc_frontend/src/node_interner.rs b/crates/noirc_frontend/src/node_interner.rs index fb06ee4362e..19ec54595ab 100644 --- a/crates/noirc_frontend/src/node_interner.rs +++ b/crates/noirc_frontend/src/node_interner.rs @@ -325,12 +325,14 @@ impl NodeInterner { } pub fn push_empty_trait(&mut self, type_id: TraitId, typ: &UnresolvedTrait) { + println!("Adding trait with name = {:?}", typ.trait_def); self.traits.insert( type_id, Shared::new(TraitType::new( type_id, typ.trait_def.name.clone(), typ.trait_def.span, + //typ.trait_def.items, )), ); } @@ -659,7 +661,7 @@ impl NodeInterner { Type::Struct(struct_type, _generics) => { let key = (struct_type.borrow().id, method_name); self.struct_methods.insert(key, method_id) - } + }, Type::Error => None, other => { @@ -720,6 +722,7 @@ fn get_type_method_key(typ: &Type) -> Option { | Type::Error | Type::NotConstant | Type::Struct(_, _) - | Type::FmtString(_, _) => None, + | Type::FmtString(_, _) + | Type::Trait(_) => None, } } From d7c44d8ee6f6939303c36d6d1119b1d9cdd9ae97 Mon Sep 17 00:00:00 2001 From: Yordan Madzhunkov Date: Sun, 23 Jul 2023 11:37:21 +0300 Subject: [PATCH 03/27] Prep for Zahari --- .../test_data_ssa_refactor/traits/src/main.nr | 6 ++-- .../src/hir/def_collector/dc_crate.rs | 6 ++-- .../src/hir/def_collector/dc_mod.rs | 29 +++++++++++++++++-- .../src/hir/def_map/item_scope.rs | 1 + .../src/hir/def_map/module_data.rs | 2 ++ .../src/hir/resolution/import.rs | 17 +++++++---- 6 files changed, 47 insertions(+), 14 deletions(-) diff --git a/crates/nargo_cli/tests/test_data_ssa_refactor/traits/src/main.nr b/crates/nargo_cli/tests/test_data_ssa_refactor/traits/src/main.nr index ccace748f0e..c43d22330dd 100644 --- a/crates/nargo_cli/tests/test_data_ssa_refactor/traits/src/main.nr +++ b/crates/nargo_cli/tests/test_data_ssa_refactor/traits/src/main.nr @@ -1,6 +1,6 @@ use dep::std; -trait Animal { +trait Default { fn default(x: Field, y: Field) -> Self; } @@ -10,7 +10,7 @@ struct Foo { } -impl Animal for Foo { +impl Default for Foo { fn default(x: Field,y: Field) -> Self { Self { bar: 0, array: [x,y] } } @@ -18,5 +18,5 @@ impl Animal for Foo { fn main(x: Field, y: Field) { let first = Foo::default(x,y); - assert(first == x); + 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 83078b80117..7b17f4df538 100644 --- a/crates/noirc_frontend/src/hir/def_collector/dc_crate.rs +++ b/crates/noirc_frontend/src/hir/def_collector/dc_crate.rs @@ -105,7 +105,6 @@ impl DefCollector { root_file_id: FileId, errors: &mut Vec, ) { - println!("\n Hohoh"); let crate_id = def_map.krate; // Recursively resolve the dependencies @@ -367,8 +366,8 @@ fn resolve_structs( fn resolve_traits( context: &mut Context, traits: HashMap, - crate_id: CrateId, - errors: &mut Vec, + _crate_id: CrateId, + _errors: &mut Vec, ) { // We must first go through the struct list once to ensure all IDs are pushed to // the def_interner map. This lets structs refer to each other regardless of declaration order @@ -457,7 +456,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/hir/def_collector/dc_mod.rs b/crates/noirc_frontend/src/hir/def_collector/dc_mod.rs index 15348a35420..19314fc5a2d 100644 --- a/crates/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/crates/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -3,8 +3,8 @@ use noirc_errors::FileDiagnostic; use crate::{ graph::CrateId, hir::def_collector::dc_crate::{UnresolvedStruct, UnresolvedTrait}, node_interner::{StructId, TraitId}, - parser::SubModule, Ident, LetStatement, NoirFunction, NoirStruct, NoirTypeAlias, ParsedModule, NoirTrait, - TypeImpl, + parser::SubModule, Ident, LetStatement, NoirFunction, NoirStruct, NoirTypeAlias, ParsedModule, NoirTrait, TraitImpl, + TypeImpl, TraitImplItem, }; use super::{ @@ -62,6 +62,8 @@ pub fn collect_defs( collector.collect_functions(context, ast.functions, errors); + collector.collect_trait_impls(context, ast.trait_impls); + collector.collect_impls(context, ast.impls); } @@ -114,6 +116,29 @@ impl<'a> ModCollector<'a> { } } + fn collect_trait_impls(&mut self, context: &mut Context, impls: Vec) { + for r#impl in impls { + let mut unresolved_functions = + UnresolvedFunctions { file_id: self.file_id, functions: Vec::new() }; + + for item in r#impl.items { + match item { + TraitImplItem::Function(method) => { + let func_id = context.def_interner.push_empty_fn(); + context.def_interner.push_function_definition(method.name().to_owned(), func_id); + unresolved_functions.push_fn(self.module_id, func_id, method); + } + _ => { } + + } + } + + let key = (r#impl.object_type, self.module_id); + let methods = self.def_collector.collected_impls.entry(key).or_default(); + methods.push((r#impl.impl_generics, r#impl.object_type_span, unresolved_functions)); + } + } + fn collect_functions( &mut self, context: &mut Context, diff --git a/crates/noirc_frontend/src/hir/def_map/item_scope.rs b/crates/noirc_frontend/src/hir/def_map/item_scope.rs index e0577f98bcc..b3b7b291475 100644 --- a/crates/noirc_frontend/src/hir/def_map/item_scope.rs +++ b/crates/noirc_frontend/src/hir/def_map/item_scope.rs @@ -21,6 +21,7 @@ impl ItemScope { name: Ident, mod_def: ModuleDefId, ) -> Result<(), (Ident, Ident)> { + println!("Adding name = {:?} mod_def = {:?}", &name, &mod_def); self.add_item_to_namespace(name, mod_def)?; self.defs.push(mod_def); Ok(()) diff --git a/crates/noirc_frontend/src/hir/def_map/module_data.rs b/crates/noirc_frontend/src/hir/def_map/module_data.rs index 3ea3bcc762d..2e5256bfec4 100644 --- a/crates/noirc_frontend/src/hir/def_map/module_data.rs +++ b/crates/noirc_frontend/src/hir/def_map/module_data.rs @@ -62,6 +62,7 @@ impl ModuleData { } pub fn declare_struct(&mut self, name: Ident, id: StructId) -> Result<(), (Ident, Ident)> { + println!("Declare struct = {:?}", &name); self.declare(name, ModuleDefId::TypeId(id)) } @@ -75,6 +76,7 @@ impl ModuleData { } pub fn declare_trait(&mut self, name: Ident, id: TraitId) -> Result<(), (Ident, Ident)> { + println!("Declare trait = {:?}", &name); self.declare(name, ModuleDefId::InterfaceId(id)) } diff --git a/crates/noirc_frontend/src/hir/resolution/import.rs b/crates/noirc_frontend/src/hir/resolution/import.rs index c7a94cf4bd4..024fd8aab5e 100644 --- a/crates/noirc_frontend/src/hir/resolution/import.rs +++ b/crates/noirc_frontend/src/hir/resolution/import.rs @@ -84,6 +84,8 @@ pub fn resolve_path_to_ns( allow_contracts: bool, ) -> PathResolution { let import_path = &import_directive.path.segments; + println!("path_to_ns = {:?}", &import_path); + println!("directive = {:?}", &import_directive); match import_directive.path.kind { crate::ast::PathKind::Crate => { @@ -138,13 +140,13 @@ fn resolve_name_in_module( if current_ns.is_none() { return Err(PathResolutionError::Unresolved(first_segment.clone())); } - + for segment in import_path { let typ = match current_ns.take_types() { None => return Err(PathResolutionError::Unresolved(segment.clone())), Some(typ) => typ, }; - + // In the type namespace, only Mod can be used in a path. let new_module_id = match typ { ModuleDefId::ModuleId(id) => id, @@ -155,20 +157,25 @@ fn resolve_name_in_module( ModuleDefId::InterfaceId(id) => id.0, ModuleDefId::GlobalId(_) => panic!("globals cannot be in the type namespace"), }; - + current_mod = &def_maps[&new_module_id.krate].modules[new_module_id.local_id.0]; + // Check if namespace let found_ns = current_mod.find_name(segment); + println!("segment = {:?} found = {}", &segment, !found_ns.is_none()); + if segment.0.contents == "default".to_string() { + println!("\n {:?}", ¤t_mod); + } if found_ns.is_none() { return Err(PathResolutionError::Unresolved(segment.clone())); } - + println!("checkpoint 3"); // Check if it is a contract and we're calling from a non-contract context if current_mod.is_contract && !allow_contracts { return Err(PathResolutionError::ExternalContractUsed(segment.clone())); } - + println!("checkpoint 4 "); current_ns = found_ns; } From bc505a8701dacd2a6d2311fb864334f2ef659bf3 Mon Sep 17 00:00:00 2001 From: Yordan Madzhunkov Date: Sun, 23 Jul 2023 11:57:36 +0300 Subject: [PATCH 04/27] Fix build for Zahari --- crates/noirc_frontend/src/hir/def_collector/dc_mod.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 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 19314fc5a2d..52eb31d36db 100644 --- a/crates/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/crates/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -62,7 +62,7 @@ pub fn collect_defs( collector.collect_functions(context, ast.functions, errors); - collector.collect_trait_impls(context, ast.trait_impls); + collector.collect_trait_impls(context, ast.trait_impls, errors); collector.collect_impls(context, ast.impls); } @@ -116,17 +116,17 @@ impl<'a> ModCollector<'a> { } } - fn collect_trait_impls(&mut self, context: &mut Context, impls: Vec) { + fn collect_trait_impls(&mut self, context: &mut Context, impls: Vec, errors: &mut Vec,) { for r#impl in impls { let mut unresolved_functions = UnresolvedFunctions { file_id: self.file_id, functions: Vec::new() }; for item in r#impl.items { match item { - TraitImplItem::Function(method) => { + TraitImplItem::Function(noir_function) => { let func_id = context.def_interner.push_empty_fn(); - context.def_interner.push_function_definition(method.name().to_owned(), func_id); - unresolved_functions.push_fn(self.module_id, func_id, method); + context.def_interner.push_function_definition(noir_function.name().to_owned(), func_id); + unresolved_functions.push_fn(self.module_id, func_id, noir_function); } _ => { } From 74a3effc270ce27786b55e67bb6d9cb55b202f6f Mon Sep 17 00:00:00 2001 From: Zahary Karadjov Date: Sun, 23 Jul 2023 18:34:45 +0300 Subject: [PATCH 05/27] improve(traits/parsing): Implement more constructs supported by Rust Full list of changes: * The functions within trait definitions can now have a body * Parsing support for trait-associated constants (using the comptime keyword for now to avoid introducing a new keyword such as const). * Traits can have a where clause * The where clause is only allowed on generic definitions * The parsing of where clauses with multiple constraints has been fixed * More parsing tests for functions using traits and trait definitions. --- crates/noirc_frontend/src/ast/traits.rs | 39 +++++- .../src/hir/def_collector/dc_mod.rs | 2 +- crates/noirc_frontend/src/parser/errors.rs | 5 + crates/noirc_frontend/src/parser/parser.rs | 129 +++++++++++++++--- 4 files changed, 148 insertions(+), 27 deletions(-) diff --git a/crates/noirc_frontend/src/ast/traits.rs b/crates/noirc_frontend/src/ast/traits.rs index 30b25ae6e45..ad291562085 100644 --- a/crates/noirc_frontend/src/ast/traits.rs +++ b/crates/noirc_frontend/src/ast/traits.rs @@ -3,7 +3,7 @@ use std::fmt::Display; use iter_extended::vecmap; use noirc_errors::Span; -use crate::{Ident, NoirFunction, UnresolvedGenerics, UnresolvedType}; +use crate::{Ident, NoirFunction, UnresolvedGenerics, UnresolvedType, Expression, BlockExpression}; /// AST node for trait definitions: /// `trait name { ... items ... }` @@ -11,6 +11,7 @@ use crate::{Ident, NoirFunction, UnresolvedGenerics, UnresolvedType}; pub struct NoirTrait { pub name: Ident, pub generics: Vec, + pub where_clause: Vec, pub span: Span, pub items: Vec, } @@ -25,6 +26,12 @@ pub enum TraitItem { parameters: Vec<(Ident, UnresolvedType)>, return_type: UnresolvedType, where_clause: Vec, + body: Option, + }, + Constant { + name: Ident, + typ: UnresolvedType, + default_value: Option, }, Type { name: Ident, @@ -111,7 +118,13 @@ impl Display for NoirTrait { impl Display for TraitItem { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { - TraitItem::Function { name, generics, parameters, return_type, where_clause } => { + TraitItem::Function { + name, + generics, + parameters, + return_type, + where_clause, + body } => { let generics = vecmap(generics, |generic| generic.to_string()); let parameters = vecmap(parameters, |(name, typ)| format!("{name}: {typ}")); let where_clause = vecmap(where_clause, ToString::to_string); @@ -122,10 +135,26 @@ impl Display for TraitItem { write!( f, - "fn {name}<{}>({}) -> {} where {};", + "fn {name}<{}>({}) -> {} where {}", generics, parameters, return_type, where_clause - ) - } + )?; + + if let Some(body) = body { + write!(f, "{}", body) + } else { + write!(f, ";") + } + }, + TraitItem::Constant { name, typ, default_value } => { + // TODO: Shall we use `comptime` or `const` here? + write!(f, "comptime {}: {}", name, typ)?; + + if let Some(default_value) = default_value { + write!(f, "{};", default_value) + } else { + write!(f, ";") + } + }, TraitItem::Type { name } => write!(f, "type {name};"), } } diff --git a/crates/noirc_frontend/src/hir/def_collector/dc_mod.rs b/crates/noirc_frontend/src/hir/def_collector/dc_mod.rs index 52eb31d36db..44903ec2ce5 100644 --- a/crates/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/crates/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -116,7 +116,7 @@ impl<'a> ModCollector<'a> { } } - fn collect_trait_impls(&mut self, context: &mut Context, impls: Vec, errors: &mut Vec,) { + fn collect_trait_impls(&mut self, context: &mut Context, impls: Vec, _errors: &mut Vec,) { for r#impl in impls { let mut unresolved_functions = UnresolvedFunctions { file_id: self.file_id, functions: Vec::new() }; diff --git a/crates/noirc_frontend/src/parser/errors.rs b/crates/noirc_frontend/src/parser/errors.rs index 9b072d83b09..38ea138b54f 100644 --- a/crates/noirc_frontend/src/parser/errors.rs +++ b/crates/noirc_frontend/src/parser/errors.rs @@ -27,6 +27,11 @@ pub enum ParserErrorReason { PatternInTraitFunctionParameter, #[error("Traits are an experimental feature that are not yet in a working state")] TraitsAreExperimental, + // TODO(GenericParameterNotFoundInFunction): Consider implementing this + // #[error("The identifier '{0}' specified in the where clause is not present in the list of generic parameters of the function")] + // GenericParameterNotFoundInFunction(String), + #[error("Where clauses are allowed only on functions with generic parameters")] + WhereClauseOnNonGenericFunction, } /// Represents a parsing error, or a parsing error in the making. diff --git a/crates/noirc_frontend/src/parser/parser.rs b/crates/noirc_frontend/src/parser/parser.rs index 23f3885cda7..3825e8afa25 100644 --- a/crates/noirc_frontend/src/parser/parser.rs +++ b/crates/noirc_frontend/src/parser/parser.rs @@ -23,6 +23,8 @@ //! prevent other parsers from being tried afterward since there is no longer an error. Thus, they should //! be limited to cases like the above `fn` example where it is clear we shouldn't back out of the //! current parser to try alternative parsers in a `choice` expression. +use std::process::Command; + use super::{ foldl_with_span, labels::ParsingRuleLabel, parameter_name_recovery, parameter_recovery, parenthesized, then_commit, then_commit_ignore, top_level_statement_recovery, ExprParser, @@ -40,6 +42,7 @@ use crate::{ TraitImplItem, TraitItem, TypeImpl, UnaryOp, UnresolvedTypeExpression, UseTree, UseTreeKind, }; +use chumsky::chain::Chain; use chumsky::prelude::*; use iter_extended::vecmap; use noirc_abi::{AbiDistinctness, AbiVisibility}; @@ -168,9 +171,9 @@ fn function_definition(allow_self: bool) -> impl NoirParser { .then(function_return_type()) .then(where_clause()) .then(block(expression())) - .map(|(((args, ret), where_clause), body)| { + .validate(|(((args, ret), where_clause), body), span, emit| { let ((((attribute, modifiers), name), generics), parameters) = args; - + validate_where_clause(&generics, &where_clause, span, emit); FunctionDefinition { span: name.0.span(), name, @@ -364,22 +367,41 @@ fn trait_definition() -> impl NoirParser { keyword(Keyword::Trait) .ignore_then(ident()) .then(generics()) + .then(where_clause()) .then_ignore(just(Token::LeftBrace)) .then(trait_body()) .then_ignore(just(Token::RightBrace)) - .validate(|((name, generics), items), span, emit| { - //emit(ParserError::with_reason(ParserErrorReason::TraitsAreExperimental, span)); - TopLevelStatement::Trait(NoirTrait { name, generics, span, items }) + .validate(|(((name, generics), where_clause), items), span, emit| { + validate_where_clause(&generics, &where_clause, span, emit); + TopLevelStatement::Trait(NoirTrait { name, generics, where_clause, span, items }) }) } fn trait_body() -> impl NoirParser> { trait_function_declaration() .or(trait_type_declaration()) + .or(trait_constant_declaration()) .separated_by(just(Token::Semicolon)) .allow_trailing() } +fn optional_default_value() -> impl NoirParser> { + ignore_then_commit(just(Token::Assign), expression()) + .or_not() +} + +fn trait_constant_declaration() -> impl NoirParser { + // TODO: Shall we use `comptime` or `const` here? + keyword(Keyword::CompTime) + .ignore_then(ident()) + .then_ignore(just(Token::Colon)) + .then(parse_type()) + .then(optional_default_value()) + .validate(|((name, typ), default_value), _span, _emit| { + TraitItem::Constant { name, typ, default_value } + }) +} + /// trait_function_declaration: 'fn' ident generics '(' declaration_parameters ')' function_return_type fn trait_function_declaration() -> impl NoirParser { keyword(Keyword::Fn) @@ -388,24 +410,44 @@ fn trait_function_declaration() -> impl NoirParser { .then(parenthesized(function_declaration_parameters())) .then(function_return_type().map(|(_, typ)| typ)) .then(where_clause()) - .map(|((((name, generics), parameters), return_type), where_clause)| TraitItem::Function { - name, - generics, - parameters, - return_type, - where_clause, + .then(block(expression()).or_not()) + .validate(|(((((name, generics), parameters), return_type), where_clause), body), span, emit| { + validate_where_clause(&generics, &where_clause, span, emit); + TraitItem::Function { + name, + generics, + parameters, + return_type, + where_clause, + body, + } }) } +fn validate_where_clause(generics: &Vec, where_clause: &Vec, span: Span, emit: &mut dyn FnMut(ParserError)) { + if !where_clause.is_empty() && generics.is_empty() { + emit(ParserError::with_reason( + ParserErrorReason::WhereClauseOnNonGenericFunction, + span + )); + } + + // TODO(GenericParameterNotFoundInFunction): + // Even though Rust supports where clauses that don't mention any of the generic + // parameters, these are of dubious value and can be accidentally produced by + // typos in the code, so we can consider producing compile-time errors for them. + // + // https://doc.rust-lang.org/reference/items/generics.html#where-clauses +} + /// Function declaration parameters differ from other parameters in that parameter /// patterns are not allowed in declarations. All parameters must be identifiers. fn function_declaration_parameters() -> impl NoirParser> { let typ = parse_type().recover_via(parameter_recovery()); let typ = just(Token::Colon).ignore_then(typ); - let parameter = ident().recover_via(parameter_name_recovery()).then(typ); - - let parameter = parameter.or(self_parameter().validate(|param, span, emit| { + let full_parameter = ident().recover_via(parameter_name_recovery()).then(typ); + let self_parameter = self_parameter().validate(|param, span, emit| { match param.0 { Pattern::Identifier(ident) => (ident, param.1), other => { @@ -418,7 +460,9 @@ fn function_declaration_parameters() -> impl NoirParser impl NoirParser { .then_ignore(just(Token::LeftBrace)) .then(trait_implementation_body()) .then_ignore(just(Token::RightBrace)) - .validate(|args, span, emit| { + .validate(|args, _span, _emit| { let ((other_args, where_clause), items) = args; let (((impl_generics, trait_name), trait_generics), (object_type, object_type_span)) = other_args; - //emit(ParserError::with_reason(ParserErrorReason::TraitsAreExperimental, span)); TopLevelStatement::TraitImpl(TraitImpl { impl_generics, trait_name, @@ -499,12 +542,11 @@ fn where_clause() -> impl NoirParser> { .then(ident()) .then(generic_type_args(parse_type())) .validate(|((typ, trait_name), trait_generics), _span, _emit| { - //emit(ParserError::with_reason(ParserErrorReason::TraitsAreExperimental, span)); TraitConstraint { typ, trait_name, trait_generics } }); keyword(Keyword::Where) - .ignore_then(constraints.repeated()) + .ignore_then(constraints.separated_by(just(Token::Comma))) .or_not() .map(|option| option.unwrap_or_default()) } @@ -1748,7 +1790,8 @@ mod test { "fn func_name() {}", "fn f(foo: pub u8, y : pub Field) -> u8 { x + a }", "fn f(f: pub Field, y : Field, z : comptime Field) -> u8 { x + a }", - "fn func_name(f: Field, y : pub Field, z : pub [u8;5],) {}", + "fn f(f: pub Field, y : T, z : comptime Field) -> u8 { x + a }", + "fn func_name(f: Field, y : T) where T: SomeTrait {}", "fn func_name(x: [Field], y : [Field;2],y : pub [Field;2], z : pub [u8;5]) {}", "fn main(x: pub u8, y: pub u8) -> distinct pub [u8; 2] { [x, y] }", ], @@ -1756,7 +1799,51 @@ mod test { parse_all_failing( function_definition(false), - vec!["fn x2( f: []Field,,) {}", "fn ( f: []Field) {}", "fn ( f: []Field) {}"], + vec![ + "fn x2( f: []Field,,) {}", + "fn ( f: []Field) {}", + "fn ( f: []Field) {}", + // TODO: Check for more specific error messages + "fn func_name(f: Field, y : pub Field, z : pub [u8;5],) where T: {}", + "fn func_name(f: Field, y : pub Field, z : pub [u8;5],) where SomeTrait {}", + "fn func_name(f: Field, y : pub Field, z : pub [u8;5],) SomeTrait {}", + "fn func_name(f: Field, y : pub Field, z : pub [u8;5],) where T: SomeTrait {}", + // TODO(GenericParameterNotFoundInFunction) + // Consider making this a compilation error: + // "fn func_name(f: Field, y : pub Field, z : pub [u8;5],) where T: SomeTrait {}", + ], + ); + } + + #[test] + fn parse_trait() { + parse_all( + trait_definition(), + vec![ + // Empty traits are legal in Rust and sometimes used as a way to whitelist certain types + // for a particular operation. Also known as `tag` or `marker` traits: + // https://stackoverflow.com/questions/71895489/what-is-the-purpose-of-defining-empty-impl-in-rust + "trait Empty {}", + "trait TraitWithDefaultBody { fn foo(self) {}; }", + "trait TraitAcceptingMutableRef { fn foo(&mut self); }", + "trait TraitWithTypeBoundOperation { fn identity() -> Self; }", + "trait TraitWithAssociatedType { type Element; fn item(self, index: Field) -> Self::Element; }", + // TODO: Shall we use `const` instead? + "trait TraitWithAssociatedConstant { comptime Size: Field; }", + "trait TraitWithAssociatedConstantWithDefaultValue { comptime Size: Field = 10; }", + "trait GenericTrait { fn elem(&mut self, index: Field) -> T; }", + "trait GenericTraitWithConstraints where T: SomeTrait { fn elem(self, index: Field) -> T; }", + "trait TraitWithMultipleGenericParams where A: SomeTrait, B: AnotherTrait { comptime Size: Field; fn zero() -> Self; }", + ], + ); + + parse_all_failing( + trait_definition(), + vec![ + "trait MissingBody", + "trait WrongDelimiter { fn foo() -> u8, fn bar() -> u8 }", + "trait WhereClauseWithoutGenerics where A: SomeTrait { }" + ] ); } From adf6807f94dd836107125074c38979d14020aad2 Mon Sep 17 00:00:00 2001 From: Zahary Karadjov Date: Mon, 24 Jul 2023 00:49:26 +0300 Subject: [PATCH 06/27] improve(hir/trait-types): add support for generic parameters --- .../test_data_ssa_refactor/struct/Nargo.toml | 6 ++ crates/noirc_frontend/src/ast/traits.rs | 5 +- .../src/hir/def_collector/dc_crate.rs | 5 +- .../src/hir/def_collector/dc_mod.rs | 23 ++++-- .../src/hir/def_collector/errors.rs | 15 ++++ .../src/hir/def_map/item_scope.rs | 2 +- .../src/hir/def_map/module_data.rs | 4 +- .../src/hir/def_map/module_def.rs | 8 +- .../src/hir/resolution/import.rs | 12 +-- .../src/hir/resolution/resolver.rs | 15 ++-- crates/noirc_frontend/src/hir_def/types.rs | 76 +++++++++++++++---- .../src/monomorphization/mod.rs | 2 +- crates/noirc_frontend/src/node_interner.rs | 15 +++- crates/noirc_frontend/src/parser/parser.rs | 3 - 14 files changed, 139 insertions(+), 52 deletions(-) create mode 100644 crates/nargo_cli/tests/test_data_ssa_refactor/struct/Nargo.toml diff --git a/crates/nargo_cli/tests/test_data_ssa_refactor/struct/Nargo.toml b/crates/nargo_cli/tests/test_data_ssa_refactor/struct/Nargo.toml new file mode 100644 index 00000000000..4590df7446a --- /dev/null +++ b/crates/nargo_cli/tests/test_data_ssa_refactor/struct/Nargo.toml @@ -0,0 +1,6 @@ +[package] +authors = [""] +compiler_version = "0.1" + +[dependencies] + diff --git a/crates/noirc_frontend/src/ast/traits.rs b/crates/noirc_frontend/src/ast/traits.rs index ad291562085..d17ca98e1f3 100644 --- a/crates/noirc_frontend/src/ast/traits.rs +++ b/crates/noirc_frontend/src/ast/traits.rs @@ -76,6 +76,7 @@ pub struct TraitConstraint { #[derive(Clone, Debug)] pub enum TraitImplItem { Function(NoirFunction), + Constant(Ident, UnresolvedType, Expression), Type { name: Ident, alias: UnresolvedType }, } @@ -189,7 +190,9 @@ impl Display for TraitImplItem { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { TraitImplItem::Function(function) => function.fmt(f), - TraitImplItem::Type { name, alias } => write!(f, "type {name} = {alias}"), + TraitImplItem::Type { name, alias } => write!(f, "type {name} = {alias};"), + // TODO: Should we use `comptime` or `const` here? + TraitImplItem::Constant(name, typ, value) => write!(f, "comptime {}: {} = {};", name, typ, value), } } } 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 7b17f4df538..cdf69f2bba1 100644 --- a/crates/noirc_frontend/src/hir/def_collector/dc_crate.rs +++ b/crates/noirc_frontend/src/hir/def_collector/dc_crate.rs @@ -47,7 +47,6 @@ pub struct UnresolvedTrait { pub trait_def: NoirTrait, } - #[derive(Clone)] pub struct UnresolvedTypeAlias { pub file_id: FileId, @@ -361,8 +360,8 @@ fn resolve_structs( } } -/// Create the mappings from TypeId -> StructType -/// so that expressions can access the fields of structs +/// Create the mappings from TypeId -> TraitType +/// so that expressions can access the elements of traits fn resolve_traits( context: &mut Context, traits: HashMap, 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 44903ec2ce5..93375b7fa80 100644 --- a/crates/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/crates/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -122,14 +122,19 @@ impl<'a> ModCollector<'a> { UnresolvedFunctions { file_id: self.file_id, functions: Vec::new() }; for item in r#impl.items { - match item { + match item { TraitImplItem::Function(noir_function) => { 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); + + } + TraitImplItem::Constant(_name, _typ, _value) => { + // TODO: Implement this } - _ => { } - + TraitImplItem::Type { name: _, alias: _} => { + // TODO: Implement this + }, } } @@ -199,7 +204,7 @@ impl<'a> ModCollector<'a> { self.def_collector.def_map.modules[self.module_id.0].declare_struct(name, id); if let Err((first_def, second_def)) = result { - let err = DefCollectorErrorKind::DuplicateFunction { first_def, second_def }; + let err = DefCollectorErrorKind::DuplicateTypeDef { first_def, second_def }; errors.push(err.into_file_diagnostic(self.file_id)); } @@ -263,11 +268,17 @@ impl<'a> ModCollector<'a> { None => continue, }; - // Add the struct to scope so its path can be looked up later + // 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); - // And store the TypeId -> StructType mapping somewhere it is reachable + + + if let Err((first_def, second_def)) = result { + let err = DefCollectorErrorKind::DuplicateTypeDef { first_def, second_def }; + errors.push(err.into_file_diagnostic(self.file_id)); + } + // And store the TypeId -> TraitType mapping somewhere it is reachable let unresolved = UnresolvedTrait { file_id: self.file_id, module_id: self.module_id, diff --git a/crates/noirc_frontend/src/hir/def_collector/errors.rs b/crates/noirc_frontend/src/hir/def_collector/errors.rs index d18df58d64d..4b61ed8b743 100644 --- a/crates/noirc_frontend/src/hir/def_collector/errors.rs +++ b/crates/noirc_frontend/src/hir/def_collector/errors.rs @@ -10,6 +10,8 @@ use thiserror::Error; pub enum DefCollectorErrorKind { #[error("duplicate function found in namespace")] DuplicateFunction { first_def: Ident, second_def: Ident }, + #[error("duplicate type definition found in namespace")] + DuplicateTypeDef { first_def: Ident, second_def: Ident }, #[error("duplicate function found in namespace")] DuplicateModuleDecl { first_def: Ident, second_def: Ident }, #[error("duplicate import")] @@ -46,6 +48,19 @@ impl From for Diagnostic { diag.add_secondary("second definition found here".to_string(), second_span); diag } + DefCollectorErrorKind::DuplicateTypeDef { first_def, second_def } => { + let first_span = first_def.0.span(); + let second_span = second_def.0.span(); + let func_name = &first_def.0.contents; + + let mut diag = Diagnostic::simple_error( + format!("duplicate definitions of {func_name} type found"), + "first definition found here".to_string(), + first_span, + ); + diag.add_secondary("second definition found here".to_string(), second_span); + diag + } DefCollectorErrorKind::DuplicateModuleDecl { first_def, second_def } => { let first_span = first_def.0.span(); let second_span = second_def.0.span(); diff --git a/crates/noirc_frontend/src/hir/def_map/item_scope.rs b/crates/noirc_frontend/src/hir/def_map/item_scope.rs index b3b7b291475..109102dfe65 100644 --- a/crates/noirc_frontend/src/hir/def_map/item_scope.rs +++ b/crates/noirc_frontend/src/hir/def_map/item_scope.rs @@ -50,7 +50,7 @@ impl ItemScope { ModuleDefId::FunctionId(_) => add_item(&mut self.values), ModuleDefId::TypeId(_) => add_item(&mut self.types), ModuleDefId::TypeAliasId(_) => add_item(&mut self.types), - ModuleDefId::InterfaceId(_) => add_item(&mut self.types), + ModuleDefId::TraitId(_) => add_item(&mut self.types), ModuleDefId::GlobalId(_) => add_item(&mut self.values), } } diff --git a/crates/noirc_frontend/src/hir/def_map/module_data.rs b/crates/noirc_frontend/src/hir/def_map/module_data.rs index 2e5256bfec4..3b100a7b225 100644 --- a/crates/noirc_frontend/src/hir/def_map/module_data.rs +++ b/crates/noirc_frontend/src/hir/def_map/module_data.rs @@ -62,7 +62,6 @@ impl ModuleData { } pub fn declare_struct(&mut self, name: Ident, id: StructId) -> Result<(), (Ident, Ident)> { - println!("Declare struct = {:?}", &name); self.declare(name, ModuleDefId::TypeId(id)) } @@ -76,8 +75,7 @@ impl ModuleData { } pub fn declare_trait(&mut self, name: Ident, id: TraitId) -> Result<(), (Ident, Ident)> { - println!("Declare trait = {:?}", &name); - self.declare(name, ModuleDefId::InterfaceId(id)) + self.declare(name, ModuleDefId::TraitId(id)) } pub fn declare_child_module( diff --git a/crates/noirc_frontend/src/hir/def_map/module_def.rs b/crates/noirc_frontend/src/hir/def_map/module_def.rs index 51e50363ca4..f92955f781d 100644 --- a/crates/noirc_frontend/src/hir/def_map/module_def.rs +++ b/crates/noirc_frontend/src/hir/def_map/module_def.rs @@ -10,7 +10,7 @@ pub enum ModuleDefId { FunctionId(FuncId), TypeId(StructId), TypeAliasId(TypeAliasId), - InterfaceId(TraitId), + TraitId(TraitId), GlobalId(StmtId), } @@ -38,7 +38,7 @@ impl ModuleDefId { pub fn as_trait(&self) -> Option { match self { - ModuleDefId::InterfaceId(trait_id) => Some(*trait_id), + ModuleDefId::TraitId(trait_id) => Some(*trait_id), _ => None, } } @@ -57,7 +57,7 @@ impl ModuleDefId { ModuleDefId::FunctionId(_) => "function", ModuleDefId::TypeId(_) => "type", ModuleDefId::TypeAliasId(_) => "type alias", - ModuleDefId::InterfaceId(_) => "trait", + ModuleDefId::TraitId(_) => "trait", ModuleDefId::ModuleId(_) => "module", ModuleDefId::GlobalId(_) => "global", } @@ -148,7 +148,7 @@ impl TryFromModuleDefId for TraitId { } fn description() -> String { - "interface".to_string() + "trait".to_string() } } diff --git a/crates/noirc_frontend/src/hir/resolution/import.rs b/crates/noirc_frontend/src/hir/resolution/import.rs index 024fd8aab5e..bf8a18397d5 100644 --- a/crates/noirc_frontend/src/hir/resolution/import.rs +++ b/crates/noirc_frontend/src/hir/resolution/import.rs @@ -55,7 +55,7 @@ pub fn resolve_imports( def_maps: &HashMap, ) -> (Vec, Vec<(PathResolutionError, LocalModuleId)>) { let def_map = &def_maps[&crate_id]; - + partition_results(imports_to_resolve, |import_directive| { let allow_contracts = allow_referencing_contracts(def_maps, crate_id, import_directive.module_id); @@ -140,13 +140,13 @@ fn resolve_name_in_module( if current_ns.is_none() { return Err(PathResolutionError::Unresolved(first_segment.clone())); } - + for segment in import_path { let typ = match current_ns.take_types() { None => return Err(PathResolutionError::Unresolved(segment.clone())), Some(typ) => typ, }; - + // In the type namespace, only Mod can be used in a path. let new_module_id = match typ { ModuleDefId::ModuleId(id) => id, @@ -154,12 +154,12 @@ fn resolve_name_in_module( // TODO: If impls are ever implemented, types can be used in a path ModuleDefId::TypeId(id) => id.0, ModuleDefId::TypeAliasId(_) => panic!("type aliases cannot be used in type namespace"), - ModuleDefId::InterfaceId(id) => id.0, + ModuleDefId::TraitId(id) => id.0, ModuleDefId::GlobalId(_) => panic!("globals cannot be in the type namespace"), }; - + current_mod = &def_maps[&new_module_id.krate].modules[new_module_id.local_id.0]; - + // Check if namespace let found_ns = current_mod.find_name(segment); diff --git a/crates/noirc_frontend/src/hir/resolution/resolver.rs b/crates/noirc_frontend/src/hir/resolution/resolver.rs index 1622a00c031..673bf6105ea 100644 --- a/crates/noirc_frontend/src/hir/resolution/resolver.rs +++ b/crates/noirc_frontend/src/hir/resolution/resolver.rs @@ -823,8 +823,8 @@ impl<'a> Resolver<'a> { | Type::Constant(_) | Type::NamedGeneric(_, _) | Type::NotConstant - | Type::Forall(_, _) - | Type::Trait(_) => (), + | Type::Trait(..) + | Type::Forall(_, _) => (), Type::Array(length, element_type) => { if let Type::NamedGeneric(type_variable, name) = length.as_ref() { @@ -855,6 +855,9 @@ impl<'a> Resolver<'a> { } } } + Type::Trait(trait_type, generics) => { + // TODO: Implement this + } Type::MutableReference(element) => Self::find_numeric_generics_in_type(element, found), Type::String(length) => { if let Type::NamedGeneric(type_variable, name) = length.as_ref() { @@ -1643,7 +1646,7 @@ mod test { x } "#; - + let errors = resolve_src_code(src, vec!["main", "foo"]); assert!(errors.is_empty()); } @@ -1702,17 +1705,17 @@ mod test { fn resolve_struct_method_call_expr() { let src = r#" - + struct Foo { bar: Field } - + impl Foo { fn default(x: Field,_y: Field) -> Self { Self { bar: x } } } - + fn main(x: Field, y: Field) { let _foo = Foo::default(x,y); } diff --git a/crates/noirc_frontend/src/hir_def/types.rs b/crates/noirc_frontend/src/hir_def/types.rs index 1e9c7e0f78b..a790abb9605 100644 --- a/crates/noirc_frontend/src/hir_def/types.rs +++ b/crates/noirc_frontend/src/hir_def/types.rs @@ -51,8 +51,10 @@ pub enum Type { /// represents the generic arguments (if any) to this struct type. Struct(Shared, Vec), - - Trait(Shared), + /// A user-defined trait type. The `Shared` field here refers to + /// the shared definition for each instance of this trait type. The `Vec` + /// represents the generic arguments (if any) to this trait type. + Trait(Shared, Vec), /// A tuple type with the given list of fields in the order they appear in source code. Tuple(Vec), @@ -127,21 +129,47 @@ pub struct StructType { pub span: Span, } +#[derive(Debug, PartialEq, Eq)] +pub enum TraitItemType { + /// A function declaration in a trait. + Function { + name: Ident, + generics: Generics, + arguments: Vec, + return_type: Type, + span: Span, + }, -// Represents a struct type in the type system. Each instance of this -/// rust struct will be shared across all Type::Struct variants that represent -/// the same struct type. + /// A constant declaration in a trait. + Constant { + name: Ident, + ty: Type, + span: Span, + }, + + /// A type declaration in a trait. + Type { + name: Ident, + ty: Type, + span: Span, + }, +} +/// Represents a trait type in the type system. Each instance of this +/// rust struct will be shared across all Type::Trait variants that represent +/// the same trait type. #[derive(Debug, Eq)] pub struct TraitType { /// A unique id representing this struct type. Used to check if two /// struct types are equal. pub id: TraitId, + items: Vec, + pub name: Ident, + pub generics: Generics, pub span: Span, } - /// Corresponds to generic lists such as `` in the source /// program. The `TypeVariableId` portion is used to match two /// type variables to check for equality, while the `TypeVariable` is @@ -177,8 +205,10 @@ impl TraitType { id: TraitId, name: Ident, span: Span, + items: Vec, + generics: Generics, ) -> TraitType { - TraitType { id, name, span} + TraitType { id, name, span, items, generics } } } @@ -730,8 +760,8 @@ impl Type { | Type::Constant(_) | Type::NamedGeneric(_, _) | Type::NotConstant - | Type::Forall(_, _) - | Type::Trait(_) => false, + | Type::Trait(..) + | Type::Forall(_, _) => false, Type::Array(length, elem) => { elem.contains_numeric_typevar(target_id) || named_generic_id_matches_target(length) @@ -753,6 +783,10 @@ impl Type { } }) } + Type::Trait(trait_type, generics) => { + // TODO: Implement this + false + } Type::MutableReference(element) => element.contains_numeric_typevar(target_id), Type::String(length) => named_generic_id_matches_target(length), Type::FmtString(length, elements) => { @@ -819,8 +853,14 @@ impl std::fmt::Display for Type { write!(f, "{}<{}>", s.borrow(), args.join(", ")) } } - Type::Trait(s) => { - write!(f, "{}", s.borrow()) + Type::Trait(s, args) => { + // TODO: Extract this into a shared helper for traits and structs + let args = vecmap(args, |arg| arg.to_string()); + if args.is_empty() { + write!(f, "{}", s.borrow()) + } else { + write!(f, "{}<{}>", s.borrow(), args.join(", ")) + } } Type::Tuple(elements) => { let elements = vecmap(elements, ToString::to_string); @@ -1547,7 +1587,7 @@ impl Type { let fields = vecmap(fields, |(name, typ)| (name, typ.as_abi_type())); AbiType::Struct { fields } } - Type::Trait(_) => unreachable!(), + Type::Trait(_, _) => unreachable!("traits cannot be used in the abi"), Type::Tuple(_) => todo!("as_abi_type not yet implemented for tuple types"), Type::TypeVariable(_, _) => unreachable!(), Type::NamedGeneric(..) => unreachable!(), @@ -1654,6 +1694,12 @@ impl Type { let args = vecmap(args, |arg| arg.substitute(type_bindings)); Type::Struct(fields.clone(), args) } + Type::Trait(items, args) => { + let args = vecmap(args, |arg| arg.substitute(type_bindings)); + // TODO: Don't we need to substitute any reference to the generic parameters within the + // items as well here? How does it work for structs? + Type::Trait(items.clone(), args) + } Type::Tuple(fields) => { let fields = vecmap(fields, |field| field.substitute(type_bindings)); Type::Tuple(fields) @@ -1683,7 +1729,7 @@ impl Type { | Type::Error | Type::NotConstant | Type::Unit - | Type::Trait(_) => self.clone(), + | Type::Trait(..) => self.clone(), } } @@ -1698,6 +1744,7 @@ impl Type { len_occurs || field_occurs } Type::Struct(_, generic_args) => generic_args.iter().any(|arg| arg.occurs(target_id)), + Type::Trait(_, generic_args) => generic_args.iter().any(|arg| arg.occurs(target_id)), Type::Tuple(fields) => fields.iter().any(|field| field.occurs(target_id)), Type::NamedGeneric(binding, _) | Type::TypeVariable(binding, _) => { match &*binding.borrow() { @@ -1720,7 +1767,8 @@ impl Type { | Type::Error | Type::NotConstant | Type::Unit - | Type::Trait(_) => false, + | Type::Trait(..) => false, + } } diff --git a/crates/noirc_frontend/src/monomorphization/mod.rs b/crates/noirc_frontend/src/monomorphization/mod.rs index 7fa36b225c8..c52692f5513 100644 --- a/crates/noirc_frontend/src/monomorphization/mod.rs +++ b/crates/noirc_frontend/src/monomorphization/mod.rs @@ -652,7 +652,7 @@ impl<'interner> Monomorphizer<'interner> { ast::Type::Tuple(fields) } - HirType::Trait(def) => { + HirType::Trait(def, _args) => { unreachable!("Not sure what to do here def = {:?}", def) } diff --git a/crates/noirc_frontend/src/node_interner.rs b/crates/noirc_frontend/src/node_interner.rs index 19ec54595ab..7c0daaa31e4 100644 --- a/crates/noirc_frontend/src/node_interner.rs +++ b/crates/noirc_frontend/src/node_interner.rs @@ -332,13 +332,19 @@ impl NodeInterner { type_id, typ.trait_def.name.clone(), typ.trait_def.span, - //typ.trait_def.items, + 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, @@ -723,6 +729,7 @@ fn get_type_method_key(typ: &Type) -> Option { | Type::NotConstant | Type::Struct(_, _) | Type::FmtString(_, _) - | Type::Trait(_) => None, + | Type::Struct(_, _) + | Type::Trait(_, _) => None, } } diff --git a/crates/noirc_frontend/src/parser/parser.rs b/crates/noirc_frontend/src/parser/parser.rs index 3825e8afa25..062a785bddc 100644 --- a/crates/noirc_frontend/src/parser/parser.rs +++ b/crates/noirc_frontend/src/parser/parser.rs @@ -23,8 +23,6 @@ //! prevent other parsers from being tried afterward since there is no longer an error. Thus, they should //! be limited to cases like the above `fn` example where it is clear we shouldn't back out of the //! current parser to try alternative parsers in a `choice` expression. -use std::process::Command; - use super::{ foldl_with_span, labels::ParsingRuleLabel, parameter_name_recovery, parameter_recovery, parenthesized, then_commit, then_commit_ignore, top_level_statement_recovery, ExprParser, @@ -42,7 +40,6 @@ use crate::{ TraitImplItem, TraitItem, TypeImpl, UnaryOp, UnresolvedTypeExpression, UseTree, UseTreeKind, }; -use chumsky::chain::Chain; use chumsky::prelude::*; use iter_extended::vecmap; use noirc_abi::{AbiDistinctness, AbiVisibility}; From 7abbef3fc667ef546fd6c7ea994f12af46840885 Mon Sep 17 00:00:00 2001 From: Yordan Madzhunkov Date: Mon, 24 Jul 2023 12:58:12 +0300 Subject: [PATCH 07/27] Resolving of trait impl works in very simple case --- .../noirc_frontend/src/hir/def_collector/dc_mod.rs | 1 - crates/noirc_frontend/src/hir/def_map/item_scope.rs | 1 - crates/noirc_frontend/src/hir/resolution/import.rs | 12 ++---------- crates/noirc_frontend/src/hir/resolution/resolver.rs | 12 +----------- crates/noirc_frontend/src/node_interner.rs | 1 - 5 files changed, 3 insertions(+), 24 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 93375b7fa80..78369351ea9 100644 --- a/crates/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/crates/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -127,7 +127,6 @@ impl<'a> ModCollector<'a> { 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); - } TraitImplItem::Constant(_name, _typ, _value) => { // TODO: Implement this diff --git a/crates/noirc_frontend/src/hir/def_map/item_scope.rs b/crates/noirc_frontend/src/hir/def_map/item_scope.rs index 109102dfe65..7dcc5051a0c 100644 --- a/crates/noirc_frontend/src/hir/def_map/item_scope.rs +++ b/crates/noirc_frontend/src/hir/def_map/item_scope.rs @@ -21,7 +21,6 @@ impl ItemScope { name: Ident, mod_def: ModuleDefId, ) -> Result<(), (Ident, Ident)> { - println!("Adding name = {:?} mod_def = {:?}", &name, &mod_def); self.add_item_to_namespace(name, mod_def)?; self.defs.push(mod_def); Ok(()) diff --git a/crates/noirc_frontend/src/hir/resolution/import.rs b/crates/noirc_frontend/src/hir/resolution/import.rs index bf8a18397d5..c3d82f5d29d 100644 --- a/crates/noirc_frontend/src/hir/resolution/import.rs +++ b/crates/noirc_frontend/src/hir/resolution/import.rs @@ -84,8 +84,7 @@ pub fn resolve_path_to_ns( allow_contracts: bool, ) -> PathResolution { let import_path = &import_directive.path.segments; - println!("path_to_ns = {:?}", &import_path); - println!("directive = {:?}", &import_directive); + match import_directive.path.kind { crate::ast::PathKind::Crate => { @@ -140,7 +139,6 @@ fn resolve_name_in_module( if current_ns.is_none() { return Err(PathResolutionError::Unresolved(first_segment.clone())); } - for segment in import_path { let typ = match current_ns.take_types() { None => return Err(PathResolutionError::Unresolved(segment.clone())), @@ -160,22 +158,16 @@ fn resolve_name_in_module( current_mod = &def_maps[&new_module_id.krate].modules[new_module_id.local_id.0]; - // Check if namespace let found_ns = current_mod.find_name(segment); - println!("segment = {:?} found = {}", &segment, !found_ns.is_none()); - if segment.0.contents == "default".to_string() { - println!("\n {:?}", ¤t_mod); - } + if found_ns.is_none() { return Err(PathResolutionError::Unresolved(segment.clone())); } - println!("checkpoint 3"); // Check if it is a contract and we're calling from a non-contract context if current_mod.is_contract && !allow_contracts { return Err(PathResolutionError::ExternalContractUsed(segment.clone())); } - println!("checkpoint 4 "); current_ns = found_ns; } diff --git a/crates/noirc_frontend/src/hir/resolution/resolver.rs b/crates/noirc_frontend/src/hir/resolution/resolver.rs index 673bf6105ea..df396e99a65 100644 --- a/crates/noirc_frontend/src/hir/resolution/resolver.rs +++ b/crates/noirc_frontend/src/hir/resolution/resolver.rs @@ -1721,17 +1721,7 @@ mod test { } "#; - let errors = resolve_src_code(src, vec!["main", "Foo"]); - for err in &errors { - match err { - ResolverError::PathResolutionError(PathResolutionError::Unresolved(name)) => { - println!("\n\n name = {}\n\n", name.to_string()); - assert_eq!(name.to_string(), "default"); - } - _ => unimplemented!("expected an unresolved path"), - } - } - + let errors = resolve_src_code(src, vec!["main"]); assert!(errors.is_empty()); } diff --git a/crates/noirc_frontend/src/node_interner.rs b/crates/noirc_frontend/src/node_interner.rs index 7c0daaa31e4..4d82f4fe44e 100644 --- a/crates/noirc_frontend/src/node_interner.rs +++ b/crates/noirc_frontend/src/node_interner.rs @@ -325,7 +325,6 @@ impl NodeInterner { } pub fn push_empty_trait(&mut self, type_id: TraitId, typ: &UnresolvedTrait) { - println!("Adding trait with name = {:?}", typ.trait_def); self.traits.insert( type_id, Shared::new(TraitType::new( From e319984110d4faad5f934e25516422112aff3abc Mon Sep 17 00:00:00 2001 From: Yordan Madzhunkov Date: Mon, 24 Jul 2023 13:44:40 +0300 Subject: [PATCH 08/27] First successful program with traits --- .../nargo_cli/tests/test_data_ssa_refactor/traits/Prover.toml | 2 +- .../nargo_cli/tests/test_data_ssa_refactor/traits/src/main.nr | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/crates/nargo_cli/tests/test_data_ssa_refactor/traits/Prover.toml b/crates/nargo_cli/tests/test_data_ssa_refactor/traits/Prover.toml index 7d59cc81807..71805e71e8e 100644 --- a/crates/nargo_cli/tests/test_data_ssa_refactor/traits/Prover.toml +++ b/crates/nargo_cli/tests/test_data_ssa_refactor/traits/Prover.toml @@ -1,2 +1,2 @@ -x = "0" +x = "5" y = "1" \ No newline at end of file diff --git a/crates/nargo_cli/tests/test_data_ssa_refactor/traits/src/main.nr b/crates/nargo_cli/tests/test_data_ssa_refactor/traits/src/main.nr index c43d22330dd..2333c5da244 100644 --- a/crates/nargo_cli/tests/test_data_ssa_refactor/traits/src/main.nr +++ b/crates/nargo_cli/tests/test_data_ssa_refactor/traits/src/main.nr @@ -9,10 +9,9 @@ struct Foo { array: [Field; 2], } - impl Default for Foo { fn default(x: Field,y: Field) -> Self { - Self { bar: 0, array: [x,y] } + Self { bar: x, array: [x,y] } } } From 2bf9ed9afe7f8526ff8ed89a06feb809112b8d79 Mon Sep 17 00:00:00 2001 From: Yordan Madzhunkov Date: Mon, 24 Jul 2023 14:42:07 +0300 Subject: [PATCH 09/27] Move test and remove warnings --- .../{test_data_ssa_refactor => test_data}/traits/Nargo.toml | 0 .../{test_data_ssa_refactor => test_data}/traits/Prover.toml | 0 .../{test_data_ssa_refactor => test_data}/traits/src/main.nr | 0 crates/nargo_cli/tests/test_data/traits/target/main.json | 1 + crates/noirc_frontend/src/hir/resolution/resolver.rs | 3 +-- crates/noirc_frontend/src/hir_def/types.rs | 4 ++-- 6 files changed, 4 insertions(+), 4 deletions(-) rename crates/nargo_cli/tests/{test_data_ssa_refactor => test_data}/traits/Nargo.toml (100%) rename crates/nargo_cli/tests/{test_data_ssa_refactor => test_data}/traits/Prover.toml (100%) rename crates/nargo_cli/tests/{test_data_ssa_refactor => test_data}/traits/src/main.nr (100%) create mode 100644 crates/nargo_cli/tests/test_data/traits/target/main.json diff --git a/crates/nargo_cli/tests/test_data_ssa_refactor/traits/Nargo.toml b/crates/nargo_cli/tests/test_data/traits/Nargo.toml similarity index 100% rename from crates/nargo_cli/tests/test_data_ssa_refactor/traits/Nargo.toml rename to crates/nargo_cli/tests/test_data/traits/Nargo.toml diff --git a/crates/nargo_cli/tests/test_data_ssa_refactor/traits/Prover.toml b/crates/nargo_cli/tests/test_data/traits/Prover.toml similarity index 100% rename from crates/nargo_cli/tests/test_data_ssa_refactor/traits/Prover.toml rename to crates/nargo_cli/tests/test_data/traits/Prover.toml diff --git a/crates/nargo_cli/tests/test_data_ssa_refactor/traits/src/main.nr b/crates/nargo_cli/tests/test_data/traits/src/main.nr similarity index 100% rename from crates/nargo_cli/tests/test_data_ssa_refactor/traits/src/main.nr rename to crates/nargo_cli/tests/test_data/traits/src/main.nr diff --git a/crates/nargo_cli/tests/test_data/traits/target/main.json b/crates/nargo_cli/tests/test_data/traits/target/main.json new file mode 100644 index 00000000000..ed15fc6c618 --- /dev/null +++ b/crates/nargo_cli/tests/test_data/traits/target/main.json @@ -0,0 +1 @@ +{"backend":"acvm-backend-barretenberg","abi":{"parameters":[{"name":"x","type":{"kind":"field"},"visibility":"private"},{"name":"y","type":{"kind":"field"},"visibility":"private"}],"param_witnesses":{"x":[1],"y":[2]},"return_type":null,"return_witnesses":[]},"bytecode":[155,194,60,97,194,4,0],"proving_key":null,"verification_key":null} \ No newline at end of file diff --git a/crates/noirc_frontend/src/hir/resolution/resolver.rs b/crates/noirc_frontend/src/hir/resolution/resolver.rs index df396e99a65..e8d11e794a9 100644 --- a/crates/noirc_frontend/src/hir/resolution/resolver.rs +++ b/crates/noirc_frontend/src/hir/resolution/resolver.rs @@ -855,7 +855,7 @@ impl<'a> Resolver<'a> { } } } - Type::Trait(trait_type, generics) => { + Type::Trait(_trait_type, _generics) => { // TODO: Implement this } Type::MutableReference(element) => Self::find_numeric_generics_in_type(element, found), @@ -1424,7 +1424,6 @@ mod test { use fm::FileId; use iter_extended::vecmap; - use crate::hir::def_collector; use crate::hir::def_map::{ModuleData, ModuleId, ModuleOrigin}; use crate::hir::resolution::errors::ResolverError; use crate::hir::resolution::import::PathResolutionError; diff --git a/crates/noirc_frontend/src/hir_def/types.rs b/crates/noirc_frontend/src/hir_def/types.rs index a790abb9605..8a12aef36c3 100644 --- a/crates/noirc_frontend/src/hir_def/types.rs +++ b/crates/noirc_frontend/src/hir_def/types.rs @@ -163,7 +163,7 @@ pub struct TraitType { /// struct types are equal. pub id: TraitId, - items: Vec, + pub items: Vec, pub name: Ident, pub generics: Generics, @@ -783,7 +783,7 @@ impl Type { } }) } - Type::Trait(trait_type, generics) => { + Type::Trait(_trait_type, _generics) => { // TODO: Implement this false } From cfda8e2862f03f10026a010a2d5395f305b4850e Mon Sep 17 00:00:00 2001 From: Yordan Madzhunkov Date: Mon, 24 Jul 2023 14:44:05 +0300 Subject: [PATCH 10/27] Add test for duplication of trait implementaion --- .../fail/dup_trait_implementation.nr | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) create mode 100644 crates/nargo_cli/tests/compile_tests_data/fail/dup_trait_implementation.nr 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); +} From fffc07024f8873495d4f6a66177c8bd1d89197da Mon Sep 17 00:00:00 2001 From: Yordan Madzhunkov Date: Tue, 25 Jul 2023 12:18:22 +0300 Subject: [PATCH 11/27] Improve the error reporting of duplicates --- .../fail/dup_trait_declaration.nr | 26 +++++ .../trait_implementation_wrong_parameter.nr | 21 ++++ crates/noirc_frontend/src/ast/traits.rs | 18 ++-- .../src/hir/def_collector/dc_crate.rs | 17 ++-- .../src/hir/def_collector/dc_mod.rs | 32 +++++-- .../src/hir/def_collector/errors.rs | 96 ++++++++----------- .../src/hir/resolution/import.rs | 1 - crates/noirc_frontend/src/hir_def/types.rs | 14 +-- crates/noirc_frontend/src/node_interner.rs | 2 +- crates/noirc_frontend/src/parser/parser.rs | 48 +++++----- 10 files changed, 156 insertions(+), 119 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/trait_implementation_wrong_parameter.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/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/noirc_frontend/src/ast/traits.rs b/crates/noirc_frontend/src/ast/traits.rs index d17ca98e1f3..d615e2ec796 100644 --- a/crates/noirc_frontend/src/ast/traits.rs +++ b/crates/noirc_frontend/src/ast/traits.rs @@ -3,7 +3,7 @@ use std::fmt::Display; use iter_extended::vecmap; use noirc_errors::Span; -use crate::{Ident, NoirFunction, UnresolvedGenerics, UnresolvedType, Expression, BlockExpression}; +use crate::{BlockExpression, Expression, Ident, NoirFunction, UnresolvedGenerics, UnresolvedType}; /// AST node for trait definitions: /// `trait name { ... items ... }` @@ -119,13 +119,7 @@ impl Display for NoirTrait { impl Display for TraitItem { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { - TraitItem::Function { - name, - generics, - parameters, - return_type, - where_clause, - body } => { + TraitItem::Function { name, generics, parameters, return_type, where_clause, body } => { let generics = vecmap(generics, |generic| generic.to_string()); let parameters = vecmap(parameters, |(name, typ)| format!("{name}: {typ}")); let where_clause = vecmap(where_clause, ToString::to_string); @@ -145,7 +139,7 @@ impl Display for TraitItem { } else { write!(f, ";") } - }, + } TraitItem::Constant { name, typ, default_value } => { // TODO: Shall we use `comptime` or `const` here? write!(f, "comptime {}: {}", name, typ)?; @@ -155,7 +149,7 @@ impl Display for TraitItem { } else { write!(f, ";") } - }, + } TraitItem::Type { name } => write!(f, "type {name};"), } } @@ -192,7 +186,9 @@ impl Display for TraitImplItem { TraitImplItem::Function(function) => function.fmt(f), TraitImplItem::Type { name, alias } => write!(f, "type {name} = {alias};"), // TODO: Should we use `comptime` or `const` here? - TraitImplItem::Constant(name, typ, value) => write!(f, "comptime {}: {} = {};", name, typ, value), + TraitImplItem::Constant(name, typ, value) => { + write!(f, "comptime {}: {} = {};", name, typ, value) + } } } } 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 cdf69f2bba1..2ebf1693640 100644 --- a/crates/noirc_frontend/src/hir/def_collector/dc_crate.rs +++ b/crates/noirc_frontend/src/hir/def_collector/dc_crate.rs @@ -14,7 +14,6 @@ use crate::node_interner::{FuncId, NodeInterner, StmtId, StructId, TypeAliasId, use crate::{ ExpressionKind, Generics, Ident, LetStatement, NoirFunction, NoirStruct, NoirTypeAlias, NoirTrait, ParsedModule, Shared, Type, TypeBinding, UnresolvedGenerics, UnresolvedType, - }; use fm::FileId; use iter_extended::vecmap; @@ -375,14 +374,14 @@ fn resolve_traits( for (type_id, typ) in &traits { context.def_interner.push_empty_trait(*type_id, typ); } -/* - for (type_id, typ) in traits { - let (generics, fields) = resolve_struct_fields(context, crate_id, typ, errors); - context.def_interner.update_trait(type_id, |struct_def| { - struct_def.generics = generics; - }); - } -*/ + /* + for (type_id, typ) in traits { + let (generics, fields) = resolve_struct_fields(context, crate_id, typ, errors); + context.def_interner.update_trait(type_id, |struct_def| { + struct_def.generics = generics; + }); + } + */ } fn resolve_struct_fields( 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 78369351ea9..50499acd23a 100644 --- a/crates/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/crates/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -11,7 +11,9 @@ use super::{ dc_crate::{DefCollector, UnresolvedFunctions, UnresolvedGlobal, UnresolvedTypeAlias}, errors::DefCollectorErrorKind, }; -use crate::hir::def_map::{parse_file, LocalModuleId, ModuleData, ModuleId, ModuleOrigin}; +use crate::hir::def_map::{ + parse_file, LocalModuleId, ModuleData, ModuleDefId, ModuleId, ModuleOrigin, +}; use crate::hir::resolution::import::ImportDirective; use crate::hir::Context; @@ -116,24 +118,42 @@ impl<'a> ModCollector<'a> { } } - fn collect_trait_impls(&mut self, context: &mut Context, impls: Vec, _errors: &mut Vec,) { + fn collect_trait_impls( + &mut self, + context: &mut Context, + impls: Vec, + _errors: &mut Vec, + ) { for r#impl in impls { let mut unresolved_functions = UnresolvedFunctions { file_id: self.file_id, functions: Vec::new() }; + let trait_name = r#impl.trait_name.clone(); + let module = &self.def_collector.def_map.modules[self.module_id.0]; + let p = module.find_name(&trait_name); + match p.types { + Some((module_def_id, _visibility)) => match module_def_id { + ModuleDefId::TraitId(_trait_id) => {} + _ => {} + }, + None => {} + } + for item in r#impl.items { match item { TraitImplItem::Function(noir_function) => { let func_id = context.def_interner.push_empty_fn(); - context.def_interner.push_function_definition(noir_function.name().to_owned(), func_id); + context + .def_interner + .push_function_definition(noir_function.name().to_owned(), func_id); unresolved_functions.push_fn(self.module_id, func_id, noir_function); } TraitImplItem::Constant(_name, _typ, _value) => { // TODO: Implement this } - TraitImplItem::Type { name: _, alias: _} => { + TraitImplItem::Type { name: _, alias: _ } => { // TODO: Implement this - }, + } } } @@ -273,7 +293,7 @@ impl<'a> ModCollector<'a> { if let Err((first_def, second_def)) = result { - let err = DefCollectorErrorKind::DuplicateTypeDef { first_def, second_def }; + let err = DefCollectorErrorKind::DuplicateTraitDef { first_def, second_def }; errors.push(err.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 4b61ed8b743..166f481efe4 100644 --- a/crates/noirc_frontend/src/hir/def_collector/errors.rs +++ b/crates/noirc_frontend/src/hir/def_collector/errors.rs @@ -12,7 +12,9 @@ pub enum DefCollectorErrorKind { DuplicateFunction { first_def: Ident, second_def: Ident }, #[error("duplicate type definition found in namespace")] DuplicateTypeDef { first_def: Ident, second_def: Ident }, - #[error("duplicate function found in namespace")] + #[error("duplicate trait definition found in namespace")] + DuplicateTraitDef { first_def: Ident, second_def: Ident }, + #[error("duplicate module found in namespace")] DuplicateModuleDecl { first_def: Ident, second_def: Ident }, #[error("duplicate import")] DuplicateImport { first_def: Ident, second_def: Ident }, @@ -32,73 +34,55 @@ impl DefCollectorErrorKind { } } +fn report_duplicate( + primary_message: String, + duplicate_type: &str, + first_def: Ident, + second_def: Ident, +) -> Diagnostic { + let first_span = first_def.0.span(); + let second_span = second_def.0.span(); + let mut diag = Diagnostic::simple_error( + primary_message, + format!("first {} found here", duplicate_type), + first_span, + ); + diag.add_secondary(format!("second {} found here", duplicate_type), second_span); + diag +} + impl From for Diagnostic { fn from(error: DefCollectorErrorKind) -> Diagnostic { match error { DefCollectorErrorKind::DuplicateFunction { first_def, second_def } => { - let first_span = first_def.0.span(); - let second_span = second_def.0.span(); - let func_name = &first_def.0.contents; - - let mut diag = Diagnostic::simple_error( - format!("duplicate definitions of {func_name} function found"), - "first definition found here".to_string(), - first_span, - ); - diag.add_secondary("second definition found here".to_string(), second_span); - diag + let primary_message = + format!("duplicate definitions of {} function found", &first_def.0.contents); + report_duplicate(primary_message, "function definition", first_def, second_def) } DefCollectorErrorKind::DuplicateTypeDef { first_def, second_def } => { - let first_span = first_def.0.span(); - let second_span = second_def.0.span(); - let func_name = &first_def.0.contents; - - let mut diag = Diagnostic::simple_error( - format!("duplicate definitions of {func_name} type found"), - "first definition found here".to_string(), - first_span, - ); - diag.add_secondary("second definition found here".to_string(), second_span); - diag + let primary_message = + format!("duplicate definitions of {} type found", &first_def.0.contents); + report_duplicate(primary_message, "type definition", first_def, second_def) + } + DefCollectorErrorKind::DuplicateTraitDef { first_def, second_def } => { + let primary_message = + format!("duplicate definitions of {} trait found", &first_def.0.contents); + report_duplicate(primary_message, "trait definition", first_def, second_def) } DefCollectorErrorKind::DuplicateModuleDecl { first_def, second_def } => { - let first_span = first_def.0.span(); - let second_span = second_def.0.span(); - let mod_name = &first_def.0.contents; - - let mut diag = Diagnostic::simple_error( - format!("module {mod_name} has been declared twice"), - "first declaration found here".to_string(), - first_span, - ); - diag.add_secondary("second declaration found here".to_string(), second_span); - diag + let primary_message = + format!("module {} has been declared twice", &first_def.0.contents); + report_duplicate(primary_message, "module declaration", first_def, second_def) } DefCollectorErrorKind::DuplicateImport { first_def, second_def } => { - let first_span = first_def.0.span(); - let second_span = second_def.0.span(); - let import_name = &first_def.0.contents; - - let mut diag = Diagnostic::simple_error( - format!("the name `{import_name}` is defined multiple times"), - "first import found here".to_string(), - first_span, - ); - diag.add_secondary("second import found here".to_string(), second_span); - diag + let primary_message = + format!("module `{}` is imported multiple times", &first_def.0.contents); + report_duplicate(primary_message, "import", first_def, second_def) } DefCollectorErrorKind::DuplicateGlobal { first_def, second_def } => { - let first_span = first_def.0.span(); - let second_span = second_def.0.span(); - let import_name = &first_def.0.contents; - - let mut diag = Diagnostic::simple_error( - format!("the name `{import_name}` is defined multiple times"), - "first global declaration found here".to_string(), - first_span, - ); - diag.add_secondary("second global declaration found here".to_string(), second_span); - diag + let primary_message = + format!("the global `{}` is defined multiple times", &first_def.0.contents); + report_duplicate(primary_message, "global declaration", first_def, second_def) } DefCollectorErrorKind::UnresolvedModuleDecl { mod_name } => { let span = mod_name.0.span(); diff --git a/crates/noirc_frontend/src/hir/resolution/import.rs b/crates/noirc_frontend/src/hir/resolution/import.rs index c3d82f5d29d..c70f383f245 100644 --- a/crates/noirc_frontend/src/hir/resolution/import.rs +++ b/crates/noirc_frontend/src/hir/resolution/import.rs @@ -85,7 +85,6 @@ pub fn resolve_path_to_ns( ) -> PathResolution { let import_path = &import_directive.path.segments; - match import_directive.path.kind { crate::ast::PathKind::Crate => { // Resolve from the root of the crate diff --git a/crates/noirc_frontend/src/hir_def/types.rs b/crates/noirc_frontend/src/hir_def/types.rs index 8a12aef36c3..20624b5eae1 100644 --- a/crates/noirc_frontend/src/hir_def/types.rs +++ b/crates/noirc_frontend/src/hir_def/types.rs @@ -141,18 +141,10 @@ pub enum TraitItemType { }, /// A constant declaration in a trait. - Constant { - name: Ident, - ty: Type, - span: Span, - }, + Constant { name: Ident, ty: Type, span: Span }, /// A type declaration in a trait. - Type { - name: Ident, - ty: Type, - span: Span, - }, + Type { name: Ident, ty: Type, span: Span }, } /// Represents a trait type in the type system. Each instance of this /// rust struct will be shared across all Type::Trait variants that represent @@ -207,7 +199,7 @@ impl TraitType { span: Span, items: Vec, generics: Generics, - ) -> TraitType { + ) -> TraitType { TraitType { id, name, span, items, generics } } } diff --git a/crates/noirc_frontend/src/node_interner.rs b/crates/noirc_frontend/src/node_interner.rs index 4d82f4fe44e..04dd1b7dfb4 100644 --- a/crates/noirc_frontend/src/node_interner.rs +++ b/crates/noirc_frontend/src/node_interner.rs @@ -666,7 +666,7 @@ impl NodeInterner { Type::Struct(struct_type, _generics) => { let key = (struct_type.borrow().id, method_name); self.struct_methods.insert(key, method_id) - }, + } Type::Error => None, other => { diff --git a/crates/noirc_frontend/src/parser/parser.rs b/crates/noirc_frontend/src/parser/parser.rs index 062a785bddc..d13621c1773 100644 --- a/crates/noirc_frontend/src/parser/parser.rs +++ b/crates/noirc_frontend/src/parser/parser.rs @@ -383,8 +383,7 @@ fn trait_body() -> impl NoirParser> { } fn optional_default_value() -> impl NoirParser> { - ignore_then_commit(just(Token::Assign), expression()) - .or_not() + ignore_then_commit(just(Token::Assign), expression()).or_not() } fn trait_constant_declaration() -> impl NoirParser { @@ -394,8 +393,10 @@ fn trait_constant_declaration() -> impl NoirParser { .then_ignore(just(Token::Colon)) .then(parse_type()) .then(optional_default_value()) - .validate(|((name, typ), default_value), _span, _emit| { - TraitItem::Constant { name, typ, default_value } + .validate(|((name, typ), default_value), _span, _emit| TraitItem::Constant { + name, + typ, + default_value, }) } @@ -408,25 +409,22 @@ fn trait_function_declaration() -> impl NoirParser { .then(function_return_type().map(|(_, typ)| typ)) .then(where_clause()) .then(block(expression()).or_not()) - .validate(|(((((name, generics), parameters), return_type), where_clause), body), span, emit| { - validate_where_clause(&generics, &where_clause, span, emit); - TraitItem::Function { - name, - generics, - parameters, - return_type, - where_clause, - body, - } - }) + .validate( + |(((((name, generics), parameters), return_type), where_clause), body), span, emit| { + validate_where_clause(&generics, &where_clause, span, emit); + TraitItem::Function { name, generics, parameters, return_type, where_clause, body } + }, + ) } -fn validate_where_clause(generics: &Vec, where_clause: &Vec, span: Span, emit: &mut dyn FnMut(ParserError)) { +fn validate_where_clause( + generics: &Vec, + where_clause: &Vec, + span: Span, + emit: &mut dyn FnMut(ParserError), +) { if !where_clause.is_empty() && generics.is_empty() { - emit(ParserError::with_reason( - ParserErrorReason::WhereClauseOnNonGenericFunction, - span - )); + emit(ParserError::with_reason(ParserErrorReason::WhereClauseOnNonGenericFunction, span)); } // TODO(GenericParameterNotFoundInFunction): @@ -538,8 +536,10 @@ fn where_clause() -> impl NoirParser> { .then_ignore(just(Token::Colon)) .then(ident()) .then(generic_type_args(parse_type())) - .validate(|((typ, trait_name), trait_generics), _span, _emit| { - TraitConstraint { typ, trait_name, trait_generics } + .validate(|((typ, trait_name), trait_generics), _span, _emit| TraitConstraint { + typ, + trait_name, + trait_generics, }); keyword(Keyword::Where) @@ -1839,8 +1839,8 @@ mod test { vec![ "trait MissingBody", "trait WrongDelimiter { fn foo() -> u8, fn bar() -> u8 }", - "trait WhereClauseWithoutGenerics where A: SomeTrait { }" - ] + "trait WhereClauseWithoutGenerics where A: SomeTrait { }", + ], ); } From 27f31a11479ff207aeba244856b5954680ebf4ca Mon Sep 17 00:00:00 2001 From: Yordan Madzhunkov Date: Tue, 25 Jul 2023 14:24:31 +0300 Subject: [PATCH 12/27] Add few more errors and tests --- .../fail/impl_struct_not_trait.nr | 23 ++++++ .../fail/trait_not_exists.nr | 18 +++++ .../src/hir/def_collector/dc_mod.rs | 79 ++++++++++++------- .../src/hir/def_collector/errors.rs | 5 ++ 4 files changed, 97 insertions(+), 28 deletions(-) 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_not_exists.nr 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_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/noirc_frontend/src/hir/def_collector/dc_mod.rs b/crates/noirc_frontend/src/hir/def_collector/dc_mod.rs index 50499acd23a..9fe29d2589c 100644 --- a/crates/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/crates/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -122,44 +122,67 @@ impl<'a> ModCollector<'a> { &mut self, context: &mut Context, impls: Vec, - _errors: &mut Vec, + errors: &mut Vec, ) { for r#impl in impls { let mut unresolved_functions = UnresolvedFunctions { file_id: self.file_id, functions: Vec::new() }; - let trait_name = r#impl.trait_name.clone(); let module = &self.def_collector.def_map.modules[self.module_id.0]; - let p = module.find_name(&trait_name); - match p.types { + match module.find_name(&trait_name).types { Some((module_def_id, _visibility)) => match module_def_id { - ModuleDefId::TraitId(_trait_id) => {} - _ => {} - }, - None => {} - } - - for item in r#impl.items { - match item { - TraitImplItem::Function(noir_function) => { - 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); - } - TraitImplItem::Constant(_name, _typ, _value) => { - // TODO: Implement this - } - TraitImplItem::Type { name: _, alias: _ } => { - // TODO: Implement this + ModuleDefId::TraitId(_trait_id) => { + for item in r#impl.items { + match item { + TraitImplItem::Function(noir_function) => { + 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, + ); + } + TraitImplItem::Constant(_name, _typ, _value) => { + // TODO: Implement this + } + TraitImplItem::Type { name: _, alias: _ } => { + // TODO: Implement this + } + } + } + let key = (r#impl.object_type, self.module_id); + let methods = self.def_collector.collected_impls.entry(key).or_default(); + methods.push(( + r#impl.impl_generics, + r#impl.object_type_span, + unresolved_functions, + )); + }, + _ => { + let error = DefCollectorErrorKind::SimpleError { + 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)); } + }, + None => { + let error = DefCollectorErrorKind::SimpleError { + primary_message: format!("Trait {} not found", trait_name), + secondary_message: "".to_string(), + span: trait_name.span(), + }; + errors.push(error.into_file_diagnostic(self.file_id)); } } - - let key = (r#impl.object_type, self.module_id); - let methods = self.def_collector.collected_impls.entry(key).or_default(); - methods.push((r#impl.impl_generics, r#impl.object_type_span, 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 166f481efe4..af91fd50429 100644 --- a/crates/noirc_frontend/src/hir/def_collector/errors.rs +++ b/crates/noirc_frontend/src/hir/def_collector/errors.rs @@ -26,6 +26,8 @@ pub enum DefCollectorErrorKind { PathResolutionError(PathResolutionError), #[error("Non-struct type used in impl")] NonStructTypeInImpl { span: Span }, + #[error("Feature not implemented")] + SimpleError { primary_message: String, secondary_message: String, span: Span }, } impl DefCollectorErrorKind { @@ -100,6 +102,9 @@ impl From for Diagnostic { "Only struct types may have implementation methods".into(), span, ), + DefCollectorErrorKind::SimpleError { primary_message, secondary_message, span } => { + Diagnostic::simple_error(primary_message, secondary_message, span) + } } } } From 633d7a42c43f3709b91af078fa8ee48a17d25940 Mon Sep 17 00:00:00 2001 From: Yordan Madzhunkov Date: Wed, 26 Jul 2023 17:38:02 +0300 Subject: [PATCH 13/27] Add some initial check and test of trait implemenation --- .../src/hir/def_collector/dc_mod.rs | 136 ++++++++++++++++-- 1 file changed, 122 insertions(+), 14 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 9fe29d2589c..ff93da5b4c7 100644 --- a/crates/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/crates/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -3,8 +3,8 @@ use noirc_errors::FileDiagnostic; use crate::{ graph::CrateId, hir::def_collector::dc_crate::{UnresolvedStruct, UnresolvedTrait}, node_interner::{StructId, TraitId}, - parser::SubModule, Ident, LetStatement, NoirFunction, NoirStruct, NoirTypeAlias, ParsedModule, NoirTrait, TraitImpl, - TypeImpl, TraitImplItem, + parser::SubModule, Ident, LetStatement, NoirFunction, NoirStruct, NoirTypeAlias, ParsedModule, NoirTrait, TraitImpl, TraitConstraint, + TypeImpl, TraitImplItem,TraitItem, UnresolvedType, }; use super::{ @@ -69,6 +69,97 @@ pub fn collect_defs( collector.collect_impls(context, ast.impls); } +fn check_trait_method_implementation_generics( + _generics: &Vec, + _noir_function: &NoirFunction, + _trait_name: &String, +) -> Result<(), DefCollectorErrorKind> { + Ok(()) +} + +fn check_trait_method_implementation_parameters( + _parameters: &Vec<(Ident, UnresolvedType)>, + _noir_function: &NoirFunction, + _trait_name: &String, +) -> Result<(), DefCollectorErrorKind> { + Ok(()) +} + +fn check_trait_method_implementation_trait_constains( + _where_clause: &Vec, + _noir_function: &NoirFunction, + _trait_name: &String, +) -> Result<(), DefCollectorErrorKind> { + Ok(()) +} + +fn check_trait_method_implementation_return_type( + return_type: &UnresolvedType, + noir_function: &NoirFunction, + trait_name: &String, +) -> Result<(), DefCollectorErrorKind> { + if return_type != &noir_function.return_type() { + Err(DefCollectorErrorKind::SimpleError { + primary_message: format!( + "mismatch return type of method with name {} that implemetns 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 { + match item { + TraitItem::Function { name, generics, parameters, return_type, where_clause, body } => { + 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::SimpleError { + primary_message: format!( + "method with name {} is not part of trait {}, therefore it can't be implemented", + noir_function.name(), + r#trait.name.0.contents + ), + secondary_message: "".to_string(), + span: noir_function.name_ident().span(), + }) +} + impl<'a> ModCollector<'a> { fn collect_globals( &mut self, @@ -131,20 +222,37 @@ impl<'a> ModCollector<'a> { let module = &self.def_collector.def_map.modules[self.module_id.0]; match module.find_name(&trait_name).types { Some((module_def_id, _visibility)) => match module_def_id { - ModuleDefId::TraitId(_trait_id) => { + ModuleDefId::TraitId(trait_id) => { for item in r#impl.items { match item { TraitImplItem::Function(noir_function) => { - 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, - ); + if let Some(unresolved_trait) = + self.def_collector.collected_traits.get(&trait_id) + { + match check_trait_method_implementation( + &unresolved_trait.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, + ); + } + Err(error) => { + errors + .push(error.into_file_diagnostic(self.file_id)); + } + } + } else { + // ?? + } } TraitImplItem::Constant(_name, _typ, _value) => { // TODO: Implement this @@ -161,7 +269,7 @@ impl<'a> ModCollector<'a> { r#impl.object_type_span, unresolved_functions, )); - }, + } _ => { let error = DefCollectorErrorKind::SimpleError { primary_message: format!( From 124d33540c460c6e517f344834d675aad46923b9 Mon Sep 17 00:00:00 2001 From: Yordan Madzhunkov Date: Wed, 26 Jul 2023 17:58:38 +0300 Subject: [PATCH 14/27] Fix some warnings --- .../noirc_frontend/src/hir/def_collector/dc_mod.rs | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 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 ff93da5b4c7..55ffca56acb 100644 --- a/crates/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/crates/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -2,9 +2,12 @@ use fm::FileId; use noirc_errors::FileDiagnostic; use crate::{ - graph::CrateId, hir::def_collector::dc_crate::{UnresolvedStruct, UnresolvedTrait}, node_interner::{StructId, TraitId}, - parser::SubModule, Ident, LetStatement, NoirFunction, NoirStruct, NoirTypeAlias, ParsedModule, NoirTrait, TraitImpl, TraitConstraint, - TypeImpl, TraitImplItem,TraitItem, UnresolvedType, + graph::CrateId, + hir::def_collector::dc_crate::{UnresolvedStruct, UnresolvedTrait}, + node_interner::{StructId, TraitId}, + parser::SubModule, + Ident, LetStatement, NoirFunction, NoirStruct, NoirTypeAlias, NoirTrait, ParsedModule, TraitConstraint, + TraitImpl, TraitImplItem, TraitItem, TypeImpl, UnresolvedType, }; use super::{ @@ -74,6 +77,7 @@ fn check_trait_method_implementation_generics( _noir_function: &NoirFunction, _trait_name: &String, ) -> Result<(), DefCollectorErrorKind> { + // TODO Ok(()) } @@ -82,6 +86,7 @@ fn check_trait_method_implementation_parameters( _noir_function: &NoirFunction, _trait_name: &String, ) -> Result<(), DefCollectorErrorKind> { + // TODO Ok(()) } @@ -90,6 +95,7 @@ fn check_trait_method_implementation_trait_constains( _noir_function: &NoirFunction, _trait_name: &String, ) -> Result<(), DefCollectorErrorKind> { + // TODO Ok(()) } @@ -119,7 +125,7 @@ fn check_trait_method_implementation( ) -> Result<(), DefCollectorErrorKind> { for item in &r#trait.items { match item { - TraitItem::Function { name, generics, parameters, return_type, where_clause, body } => { + TraitItem::Function { name, generics, parameters, return_type, where_clause, body:_} => { 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( From b1f3e52510018ea3be121b343ce9e17b4c3fd3c2 Mon Sep 17 00:00:00 2001 From: Yordan Madzhunkov Date: Thu, 27 Jul 2023 13:33:32 +0300 Subject: [PATCH 15/27] Add parameter checks and reporting errors --- .../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 | 52 ++++++++++++++++--- 5 files changed, 130 insertions(+), 7 deletions(-) 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/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 55ffca56acb..b0ff8d9cd00 100644 --- a/crates/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/crates/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -1,3 +1,5 @@ +use std::fmt::format; + use fm::FileId; use noirc_errors::FileDiagnostic; @@ -77,16 +79,45 @@ fn check_trait_method_implementation_generics( _noir_function: &NoirFunction, _trait_name: &String, ) -> Result<(), DefCollectorErrorKind> { - // TODO + // TODO Ok(()) } fn check_trait_method_implementation_parameters( - _parameters: &Vec<(Ident, UnresolvedType)>, - _noir_function: &NoirFunction, - _trait_name: &String, + parameters: &Vec<(Ident, UnresolvedType)>, + noir_function: &NoirFunction, + trait_name: &String, ) -> Result<(), DefCollectorErrorKind> { - // TODO + if noir_function.def.parameters.len() != parameters.len() { + return Err(DefCollectorErrorKind::SimpleError { + primary_message: format!("Mismatch signature [Number of parameters] of method with name `{}` that implemetns trait `{}`", noir_function.name(), trait_name), + secondary_message: "".to_string(), + span: noir_function.name_ident().span(), + }); + } + let mut count = 0; + for (pattern, typ, _abi_vis) in &noir_function.def.parameters { + let (expected_name, expected_type) = ¶meters[count]; + if pattern.name_ident().0.contents != expected_name.0.contents { + // we allow different namings of parameters + } + if typ != expected_type { + return Err(DefCollectorErrorKind::SimpleError { + 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.to_string(), + ), + span: pattern.name_ident().span(), + }); + } + count = count + 1; + } Ok(()) } @@ -95,7 +126,7 @@ fn check_trait_method_implementation_trait_constains( _noir_function: &NoirFunction, _trait_name: &String, ) -> Result<(), DefCollectorErrorKind> { - // TODO + // TODO Ok(()) } @@ -125,7 +156,14 @@ fn check_trait_method_implementation( ) -> Result<(), DefCollectorErrorKind> { for item in &r#trait.items { match item { - TraitItem::Function { name, generics, parameters, return_type, where_clause, body:_} => { + TraitItem::Function { + name, + generics, + parameters, + return_type, + where_clause, + body: _, + } => { 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( From 4f9c9ddbf9d77ace38ac7cd125d81b95798cecbc Mon Sep 17 00:00:00 2001 From: Yordan Madzhunkov Date: Mon, 31 Jul 2023 15:44:51 +0300 Subject: [PATCH 16/27] Emit parser errors for trait functionalities not yet impelemented --- crates/noirc_frontend/src/parser/parser.rs | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/crates/noirc_frontend/src/parser/parser.rs b/crates/noirc_frontend/src/parser/parser.rs index d13621c1773..2fc34dd9cab 100644 --- a/crates/noirc_frontend/src/parser/parser.rs +++ b/crates/noirc_frontend/src/parser/parser.rs @@ -370,6 +370,9 @@ fn trait_definition() -> impl NoirParser { .then_ignore(just(Token::RightBrace)) .validate(|(((name, generics), where_clause), items), span, emit| { validate_where_clause(&generics, &where_clause, span, emit); + if generics.len() > 0 || where_clause.len() > 0 { + emit(ParserError::with_reason(ParserErrorReason::TraitsAreExperimental, span)); + } TopLevelStatement::Trait(NoirTrait { name, generics, where_clause, span, items }) }) } @@ -501,11 +504,15 @@ fn trait_implementation() -> impl NoirParser { .then_ignore(just(Token::LeftBrace)) .then(trait_implementation_body()) .then_ignore(just(Token::RightBrace)) - .validate(|args, _span, _emit| { + .validate(|args, span, emit| { let ((other_args, where_clause), items) = args; let (((impl_generics, trait_name), trait_generics), (object_type, object_type_span)) = other_args; + if where_clause.len() > 0 || impl_generics.len() > 0 { + emit(ParserError::with_reason(ParserErrorReason::TraitsAreExperimental, span)); + } + TopLevelStatement::TraitImpl(TraitImpl { impl_generics, trait_name, @@ -536,10 +543,9 @@ fn where_clause() -> impl NoirParser> { .then_ignore(just(Token::Colon)) .then(ident()) .then(generic_type_args(parse_type())) - .validate(|((typ, trait_name), trait_generics), _span, _emit| TraitConstraint { - typ, - trait_name, - trait_generics, + .validate(|((typ, trait_name), trait_generics), span, emit| { + emit(ParserError::with_reason(ParserErrorReason::TraitsAreExperimental, span)); + TraitConstraint { typ, trait_name, trait_generics} }); keyword(Keyword::Where) From 98f1cc55179651e5f54ab5c8a6bfa8756167b6a8 Mon Sep 17 00:00:00 2001 From: Yordan Madzhunkov Date: Mon, 31 Jul 2023 16:01:40 +0300 Subject: [PATCH 17/27] Forgot one space --- crates/noirc_frontend/src/parser/parser.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/noirc_frontend/src/parser/parser.rs b/crates/noirc_frontend/src/parser/parser.rs index 2fc34dd9cab..28184a617e3 100644 --- a/crates/noirc_frontend/src/parser/parser.rs +++ b/crates/noirc_frontend/src/parser/parser.rs @@ -545,7 +545,7 @@ fn where_clause() -> impl NoirParser> { .then(generic_type_args(parse_type())) .validate(|((typ, trait_name), trait_generics), span, emit| { emit(ParserError::with_reason(ParserErrorReason::TraitsAreExperimental, span)); - TraitConstraint { typ, trait_name, trait_generics} + TraitConstraint { typ, trait_name, trait_generics } }); keyword(Keyword::Where) From 7d34e1431738666dcd6cfeb41653ed41302bf3f8 Mon Sep 17 00:00:00 2001 From: Yordan Madzhunkov Date: Wed, 2 Aug 2023 17:56:05 +0300 Subject: [PATCH 18/27] Fix traits test nargo toml --- crates/nargo_cli/tests/test_data/traits/Nargo.toml | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/crates/nargo_cli/tests/test_data/traits/Nargo.toml b/crates/nargo_cli/tests/test_data/traits/Nargo.toml index d9434868157..629f55ef064 100644 --- a/crates/nargo_cli/tests/test_data/traits/Nargo.toml +++ b/crates/nargo_cli/tests/test_data/traits/Nargo.toml @@ -1,7 +1,6 @@ +[package] +name = "traits" +authors = [""] +compiler_version = "0.1" - [package] - authors = [""] - compiler_version = "0.1" - - [dependencies] - \ No newline at end of file +[dependencies] From 6712de5313187f2d42141654918df8462b681316 Mon Sep 17 00:00:00 2001 From: Yordan Madzhunkov Date: Wed, 2 Aug 2023 18:03:45 +0300 Subject: [PATCH 19/27] Apply cargo fmt --- crates/noirc_frontend/src/hir/def_collector/dc_crate.rs | 6 +++--- crates/noirc_frontend/src/hir/def_collector/dc_mod.rs | 6 ++---- crates/noirc_frontend/src/hir/def_map/module_data.rs | 3 +-- crates/noirc_frontend/src/hir/def_map/module_def.rs | 5 +---- crates/noirc_frontend/src/hir/resolution/resolver.rs | 4 ++-- crates/noirc_frontend/src/hir_def/types.rs | 5 ++--- crates/noirc_frontend/src/node_interner.rs | 6 +++--- 7 files changed, 14 insertions(+), 21 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 2ebf1693640..78b972a321d 100644 --- a/crates/noirc_frontend/src/hir/def_collector/dc_crate.rs +++ b/crates/noirc_frontend/src/hir/def_collector/dc_crate.rs @@ -10,10 +10,10 @@ 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, TraitId}; +use crate::node_interner::{FuncId, NodeInterner, StmtId, StructId, TraitId, TypeAliasId}; use crate::{ - ExpressionKind, Generics, Ident, LetStatement, NoirFunction, NoirStruct, NoirTypeAlias, NoirTrait, - ParsedModule, Shared, Type, TypeBinding, UnresolvedGenerics, UnresolvedType, + ExpressionKind, Generics, Ident, LetStatement, NoirFunction, NoirStruct, NoirTrait, + NoirTypeAlias, ParsedModule, Shared, Type, TypeBinding, UnresolvedGenerics, UnresolvedType, }; use fm::FileId; use iter_extended::vecmap; 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 b0ff8d9cd00..7e888c90b7f 100644 --- a/crates/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/crates/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -8,8 +8,8 @@ use crate::{ hir::def_collector::dc_crate::{UnresolvedStruct, UnresolvedTrait}, node_interner::{StructId, TraitId}, parser::SubModule, - Ident, LetStatement, NoirFunction, NoirStruct, NoirTypeAlias, NoirTrait, ParsedModule, TraitConstraint, - TraitImpl, TraitImplItem, TraitItem, TypeImpl, UnresolvedType, + Ident, LetStatement, NoirFunction, NoirStruct, NoirTrait, NoirTypeAlias, ParsedModule, + TraitConstraint, TraitImpl, TraitImplItem, TraitItem, TypeImpl, UnresolvedType, }; use super::{ @@ -465,7 +465,6 @@ impl<'a> ModCollector<'a> { // 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::DuplicateTraitDef { first_def, second_def }; @@ -479,7 +478,6 @@ impl<'a> ModCollector<'a> { trait_def: trait_definition, }; self.def_collector.collected_traits.insert(id, unresolved); - } } diff --git a/crates/noirc_frontend/src/hir/def_map/module_data.rs b/crates/noirc_frontend/src/hir/def_map/module_data.rs index 3b100a7b225..0b6c6ebd77c 100644 --- a/crates/noirc_frontend/src/hir/def_map/module_data.rs +++ b/crates/noirc_frontend/src/hir/def_map/module_data.rs @@ -3,7 +3,7 @@ use std::collections::HashMap; use fm::FileId; use crate::{ - node_interner::{FuncId, StmtId, StructId, TypeAliasId, TraitId}, + node_interner::{FuncId, StmtId, StructId, TraitId, TypeAliasId}, Ident, }; @@ -65,7 +65,6 @@ impl ModuleData { self.declare(name, ModuleDefId::TypeId(id)) } - pub fn declare_type_alias( &mut self, name: Ident, diff --git a/crates/noirc_frontend/src/hir/def_map/module_def.rs b/crates/noirc_frontend/src/hir/def_map/module_def.rs index f92955f781d..ade0fcaf7aa 100644 --- a/crates/noirc_frontend/src/hir/def_map/module_def.rs +++ b/crates/noirc_frontend/src/hir/def_map/module_def.rs @@ -1,5 +1,4 @@ - -use crate::node_interner::{FuncId, StmtId, StructId, TypeAliasId, TraitId}; +use crate::node_interner::{FuncId, StmtId, StructId, TraitId, TypeAliasId}; use super::ModuleId; @@ -122,7 +121,6 @@ impl TryFromModuleDefId for StructId { } } - impl TryFromModuleDefId for TypeAliasId { fn try_from(id: ModuleDefId) -> Option { id.as_type_alias() @@ -134,7 +132,6 @@ impl TryFromModuleDefId for TypeAliasId { fn description() -> String { "type alias".to_string() - } } diff --git a/crates/noirc_frontend/src/hir/resolution/resolver.rs b/crates/noirc_frontend/src/hir/resolution/resolver.rs index e8d11e794a9..475520560b8 100644 --- a/crates/noirc_frontend/src/hir/resolution/resolver.rs +++ b/crates/noirc_frontend/src/hir/resolution/resolver.rs @@ -35,8 +35,8 @@ use crate::{ }; use crate::{ ArrayLiteral, ContractFunctionType, Generics, LValue, NoirStruct, NoirTypeAlias, Path, Pattern, - Shared, StructType, Type, TypeAliasType, TypeBinding, TypeVariable, UnaryOp, - UnresolvedGenerics, UnresolvedType, UnresolvedTypeExpression, ERROR_IDENT, TraitType, + Shared, StructType, TraitType, Type, TypeAliasType, TypeBinding, TypeVariable, UnaryOp, + UnresolvedGenerics, UnresolvedType, UnresolvedTypeExpression, ERROR_IDENT, }; use fm::FileId; use iter_extended::vecmap; diff --git a/crates/noirc_frontend/src/hir_def/types.rs b/crates/noirc_frontend/src/hir_def/types.rs index 20624b5eae1..f93c9b4497d 100644 --- a/crates/noirc_frontend/src/hir_def/types.rs +++ b/crates/noirc_frontend/src/hir_def/types.rs @@ -1720,7 +1720,7 @@ impl Type { | Type::Constant(_) | Type::Error | Type::NotConstant - | Type::Unit + | Type::Unit | Type::Trait(..) => self.clone(), } } @@ -1758,9 +1758,8 @@ impl Type { | Type::Constant(_) | Type::Error | Type::NotConstant - | Type::Unit + | Type::Unit | Type::Trait(..) => false, - } } diff --git a/crates/noirc_frontend/src/node_interner.rs b/crates/noirc_frontend/src/node_interner.rs index 04dd1b7dfb4..c74680ef9fd 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, UnresolvedTrait}; +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; @@ -60,7 +60,7 @@ pub struct NodeInterner { // Map type aliases to the actual type. // When resolving types, check against this map to see if a type alias is defined. type_aliases: Vec, - + // Trait map. // // Each struct definition is possibly shared across multiple type nodes. @@ -727,7 +727,7 @@ fn get_type_method_key(typ: &Type) -> Option { | Type::Error | Type::NotConstant | Type::Struct(_, _) - | Type::FmtString(_, _) + | Type::FmtString(_, _) | Type::Struct(_, _) | Type::Trait(_, _) => None, } From 5f8fec00a9d5ed4884c16509fc3443e5bc779f82 Mon Sep 17 00:00:00 2001 From: Yordan Madzhunkov Date: Thu, 3 Aug 2023 15:33:04 +0300 Subject: [PATCH 20/27] Fix some clippy warnings --- .../noirc_frontend/src/hir/def_collector/dc_mod.rs | 14 +++++++------- crates/noirc_frontend/src/parser/parser.rs | 4 ++-- 2 files changed, 9 insertions(+), 9 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 7e888c90b7f..a25c0ec6d18 100644 --- a/crates/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/crates/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -1,4 +1,4 @@ -use std::fmt::format; + use fm::FileId; use noirc_errors::FileDiagnostic; @@ -111,12 +111,12 @@ fn check_trait_method_implementation_parameters( secondary_message: format!( "`{}: {}` expected", pattern.name_ident().0.contents, - expected_type.to_string(), + expected_type, ), span: pattern.name_ident().span(), }); } - count = count + 1; + count += 1; } Ok(()) } @@ -168,22 +168,22 @@ fn check_trait_method_implementation( // name matches, check for parameters, return type and where clause check_trait_method_implementation_generics( generics, - &noir_function, + noir_function, &r#trait.name.0.contents, )?; check_trait_method_implementation_parameters( parameters, - &noir_function, + noir_function, &r#trait.name.0.contents, )?; check_trait_method_implementation_trait_constains( where_clause, - &noir_function, + noir_function, &r#trait.name.0.contents, )?; check_trait_method_implementation_return_type( return_type, - &noir_function, + noir_function, &r#trait.name.0.contents, )?; return Ok(()); diff --git a/crates/noirc_frontend/src/parser/parser.rs b/crates/noirc_frontend/src/parser/parser.rs index 28184a617e3..df7c140aa04 100644 --- a/crates/noirc_frontend/src/parser/parser.rs +++ b/crates/noirc_frontend/src/parser/parser.rs @@ -370,7 +370,7 @@ fn trait_definition() -> impl NoirParser { .then_ignore(just(Token::RightBrace)) .validate(|(((name, generics), where_clause), items), span, emit| { validate_where_clause(&generics, &where_clause, span, emit); - if generics.len() > 0 || where_clause.len() > 0 { + if !generics.is_empty() || !where_clause.is_empty() { emit(ParserError::with_reason(ParserErrorReason::TraitsAreExperimental, span)); } TopLevelStatement::Trait(NoirTrait { name, generics, where_clause, span, items }) @@ -509,7 +509,7 @@ fn trait_implementation() -> impl NoirParser { let (((impl_generics, trait_name), trait_generics), (object_type, object_type_span)) = other_args; - if where_clause.len() > 0 || impl_generics.len() > 0 { + if !where_clause.is_empty() || !impl_generics.is_empty() { emit(ParserError::with_reason(ParserErrorReason::TraitsAreExperimental, span)); } From 1d423848bdbde4b8bd4d8619f43b368a4367cd5a Mon Sep 17 00:00:00 2001 From: Yordan Madzhunkov Date: Thu, 3 Aug 2023 15:45:46 +0300 Subject: [PATCH 21/27] Fix cargo clippy errors --- .../src/hir/def_collector/dc_crate.rs | 2 +- .../src/hir/def_collector/dc_mod.rs | 80 +++++++++---------- .../src/hir/resolution/resolver.rs | 3 - crates/noirc_frontend/src/hir_def/types.rs | 10 +-- crates/noirc_frontend/src/node_interner.rs | 1 - 5 files changed, 40 insertions(+), 56 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 78b972a321d..1831b7831d8 100644 --- a/crates/noirc_frontend/src/hir/def_collector/dc_crate.rs +++ b/crates/noirc_frontend/src/hir/def_collector/dc_crate.rs @@ -365,7 +365,7 @@ fn resolve_traits( context: &mut Context, traits: HashMap, _crate_id: CrateId, - _errors: &mut Vec, + _errors: &mut [FileDiagnostic], ) { // We must first go through the struct list once to ensure all IDs are pushed to // the def_interner map. This lets structs refer to each other regardless of declaration order 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 a25c0ec6d18..3d973954012 100644 --- a/crates/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/crates/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -1,5 +1,3 @@ - - use fm::FileId; use noirc_errors::FileDiagnostic; @@ -75,9 +73,9 @@ pub fn collect_defs( } fn check_trait_method_implementation_generics( - _generics: &Vec, + _generics: &[Ident], _noir_function: &NoirFunction, - _trait_name: &String, + _trait_name: &str, ) -> Result<(), DefCollectorErrorKind> { // TODO Ok(()) @@ -95,8 +93,7 @@ fn check_trait_method_implementation_parameters( span: noir_function.name_ident().span(), }); } - let mut count = 0; - for (pattern, typ, _abi_vis) in &noir_function.def.parameters { + for (count, (pattern, typ, _abi_vis)) in noir_function.def.parameters.iter().enumerate() { let (expected_name, expected_type) = ¶meters[count]; if pattern.name_ident().0.contents != expected_name.0.contents { // we allow different namings of parameters @@ -116,15 +113,14 @@ fn check_trait_method_implementation_parameters( span: pattern.name_ident().span(), }); } - count += 1; } Ok(()) } fn check_trait_method_implementation_trait_constains( - _where_clause: &Vec, + _where_clause: &[TraitConstraint], _noir_function: &NoirFunction, - _trait_name: &String, + _trait_name: &str, ) -> Result<(), DefCollectorErrorKind> { // TODO Ok(()) @@ -155,41 +151,39 @@ fn check_trait_method_implementation( noir_function: &NoirFunction, ) -> Result<(), DefCollectorErrorKind> { for item in &r#trait.items { - match item { - TraitItem::Function { - name, - generics, - parameters, - return_type, - where_clause, - body: _, - } => { - 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(()); - } + 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(()); } - _ => {} } } diff --git a/crates/noirc_frontend/src/hir/resolution/resolver.rs b/crates/noirc_frontend/src/hir/resolution/resolver.rs index 475520560b8..6617e04c788 100644 --- a/crates/noirc_frontend/src/hir/resolution/resolver.rs +++ b/crates/noirc_frontend/src/hir/resolution/resolver.rs @@ -855,9 +855,6 @@ impl<'a> Resolver<'a> { } } } - Type::Trait(_trait_type, _generics) => { - // TODO: Implement this - } Type::MutableReference(element) => Self::find_numeric_generics_in_type(element, found), Type::String(length) => { if let Type::NamedGeneric(type_variable, name) = length.as_ref() { diff --git a/crates/noirc_frontend/src/hir_def/types.rs b/crates/noirc_frontend/src/hir_def/types.rs index f93c9b4497d..29b63456900 100644 --- a/crates/noirc_frontend/src/hir_def/types.rs +++ b/crates/noirc_frontend/src/hir_def/types.rs @@ -775,10 +775,6 @@ impl Type { } }) } - Type::Trait(_trait_type, _generics) => { - // TODO: Implement this - false - } Type::MutableReference(element) => element.contains_numeric_typevar(target_id), Type::String(length) => named_generic_id_matches_target(length), Type::FmtString(length, elements) => { @@ -1720,8 +1716,7 @@ impl Type { | Type::Constant(_) | Type::Error | Type::NotConstant - | Type::Unit - | Type::Trait(..) => self.clone(), + | Type::Unit => self.clone(), } } @@ -1758,8 +1753,7 @@ impl Type { | Type::Constant(_) | Type::Error | Type::NotConstant - | Type::Unit - | Type::Trait(..) => false, + | Type::Unit => false, } } diff --git a/crates/noirc_frontend/src/node_interner.rs b/crates/noirc_frontend/src/node_interner.rs index c74680ef9fd..1505e183d64 100644 --- a/crates/noirc_frontend/src/node_interner.rs +++ b/crates/noirc_frontend/src/node_interner.rs @@ -728,7 +728,6 @@ fn get_type_method_key(typ: &Type) -> Option { | Type::NotConstant | Type::Struct(_, _) | Type::FmtString(_, _) - | Type::Struct(_, _) | Type::Trait(_, _) => None, } } From a2e202b6e487fa5441fd4073b7623eb61312ed20 Mon Sep 17 00:00:00 2001 From: Yordan Madzhunkov Date: Thu, 3 Aug 2023 18:26:52 +0300 Subject: [PATCH 22/27] Remove artifact from rebase --- .../tests/test_data_ssa_refactor/struct/Nargo.toml | 6 ------ 1 file changed, 6 deletions(-) delete mode 100644 crates/nargo_cli/tests/test_data_ssa_refactor/struct/Nargo.toml diff --git a/crates/nargo_cli/tests/test_data_ssa_refactor/struct/Nargo.toml b/crates/nargo_cli/tests/test_data_ssa_refactor/struct/Nargo.toml deleted file mode 100644 index 4590df7446a..00000000000 --- a/crates/nargo_cli/tests/test_data_ssa_refactor/struct/Nargo.toml +++ /dev/null @@ -1,6 +0,0 @@ -[package] -authors = [""] -compiler_version = "0.1" - -[dependencies] - From 31355b3bef473b5a2c34b0f682065327c94cc8e1 Mon Sep 17 00:00:00 2001 From: Yordan Madzhunkov Date: Fri, 4 Aug 2023 10:33:49 +0300 Subject: [PATCH 23/27] Resolve some core review remarks --- .../src/hir/def_collector/dc_crate.rs | 25 +++--- .../src/hir/def_collector/dc_mod.rs | 57 ++++++++----- .../src/hir/def_collector/errors.rs | 80 +++++-------------- .../src/hir/resolution/resolver.rs | 22 ----- 4 files changed, 70 insertions(+), 114 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 1831b7831d8..df246978f48 100644 --- a/crates/noirc_frontend/src/hir/def_collector/dc_crate.rs +++ b/crates/noirc_frontend/src/hir/def_collector/dc_crate.rs @@ -158,7 +158,11 @@ impl DefCollector { .import(name.clone(), ns); if let Err((first_def, second_def)) = result { - let err = DefCollectorErrorKind::DuplicateImport { first_def, second_def }; + let err = DefCollectorErrorKind::Duplicate { + typ: "import".to_string(), + first_def, + second_def, + }; errors.push(err.into_file_diagnostic(root_file_id)); } } @@ -252,8 +256,11 @@ fn collect_impls( let result = module.declare_function(method.name_ident().clone(), *method_id); if let Err((first_def, second_def)) = result { - let err = - DefCollectorErrorKind::DuplicateFunction { first_def, second_def }; + let err = DefCollectorErrorKind::Duplicate { + typ: "function".to_string(), + first_def, + second_def, + }; errors.push(err.into_file_diagnostic(unresolved.file_id)); } } @@ -367,21 +374,9 @@ fn resolve_traits( _crate_id: CrateId, _errors: &mut [FileDiagnostic], ) { - // We must first go through the struct list once to ensure all IDs are pushed to - // the def_interner map. This lets structs refer to each other regardless of declaration order - // without resolve_struct_fields non-deterministically unwrapping a value - // that isn't in the HashMap. for (type_id, typ) in &traits { context.def_interner.push_empty_trait(*type_id, typ); } - /* - for (type_id, typ) in traits { - let (generics, fields) = resolve_struct_fields(context, crate_id, typ, errors); - context.def_interner.update_trait(type_id, |struct_def| { - struct_def.generics = generics; - }); - } - */ } fn resolve_struct_fields( 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 3d973954012..ef55c6cdb7b 100644 --- a/crates/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/crates/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -87,19 +87,16 @@ fn check_trait_method_implementation_parameters( trait_name: &String, ) -> Result<(), DefCollectorErrorKind> { if noir_function.def.parameters.len() != parameters.len() { - return Err(DefCollectorErrorKind::SimpleError { + return Err(DefCollectorErrorKind::GenericError { primary_message: format!("Mismatch signature [Number of parameters] of method with name `{}` that implemetns trait `{}`", noir_function.name(), trait_name), 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 pattern.name_ident().0.contents != expected_name.0.contents { - // we allow different namings of parameters - } + let (_expected_name, expected_type) = ¶meters[count]; if typ != expected_type { - return Err(DefCollectorErrorKind::SimpleError { + return Err(DefCollectorErrorKind::GenericError { primary_message: format!( "Mismatch signature of method {} that implemtns trait {}", noir_function.name(), @@ -131,8 +128,8 @@ fn check_trait_method_implementation_return_type( noir_function: &NoirFunction, trait_name: &String, ) -> Result<(), DefCollectorErrorKind> { - if return_type != &noir_function.return_type() { - Err(DefCollectorErrorKind::SimpleError { + if !(return_type == &noir_function.return_type()) { + Err(DefCollectorErrorKind::GenericError { primary_message: format!( "mismatch return type of method with name {} that implemetns trait {}", noir_function.name(), @@ -187,7 +184,7 @@ fn check_trait_method_implementation( } } - Err(DefCollectorErrorKind::SimpleError { + Err(DefCollectorErrorKind::GenericError { primary_message: format!( "method with name {} is not part of trait {}, therefore it can't be implemented", noir_function.name(), @@ -217,7 +214,11 @@ impl<'a> ModCollector<'a> { self.def_collector.def_map.modules[self.module_id.0].declare_global(name, stmt_id); if let Err((first_def, second_def)) = result { - let err = DefCollectorErrorKind::DuplicateGlobal { first_def, second_def }; + let err = DefCollectorErrorKind::Duplicate { + typ: "global".to_string(), + first_def, + second_def, + }; errors.push(err.into_file_diagnostic(self.file_id)); } @@ -309,7 +310,7 @@ impl<'a> ModCollector<'a> { )); } _ => { - let error = DefCollectorErrorKind::SimpleError { + let error = DefCollectorErrorKind::GenericError { primary_message: format!( "{} is not a trait, therefore it can't be implemented", trait_name @@ -321,7 +322,7 @@ impl<'a> ModCollector<'a> { } }, None => { - let error = DefCollectorErrorKind::SimpleError { + let error = DefCollectorErrorKind::GenericError { primary_message: format!("Trait {} not found", trait_name), secondary_message: "".to_string(), span: trait_name.span(), @@ -362,7 +363,11 @@ impl<'a> ModCollector<'a> { .declare_function(name, func_id); if let Err((first_def, second_def)) = result { - let error = DefCollectorErrorKind::DuplicateFunction { first_def, second_def }; + let error = DefCollectorErrorKind::Duplicate { + typ: "function".to_string(), + first_def, + second_def, + }; errors.push(error.into_file_diagnostic(self.file_id)); } } @@ -392,7 +397,11 @@ impl<'a> ModCollector<'a> { self.def_collector.def_map.modules[self.module_id.0].declare_struct(name, id); if let Err((first_def, second_def)) = result { - let err = DefCollectorErrorKind::DuplicateTypeDef { first_def, second_def }; + let err = DefCollectorErrorKind::Duplicate { + typ: "type definition".to_string(), + first_def, + second_def, + }; errors.push(err.into_file_diagnostic(self.file_id)); } @@ -431,7 +440,11 @@ impl<'a> ModCollector<'a> { .declare_type_alias(name, type_alias_id); if let Err((first_def, second_def)) = result { - let err = DefCollectorErrorKind::DuplicateFunction { first_def, second_def }; + let err = DefCollectorErrorKind::Duplicate { + typ: "function".to_string(), + first_def, + second_def, + }; errors.push(err.into_file_diagnostic(self.file_id)); } @@ -461,11 +474,15 @@ impl<'a> ModCollector<'a> { 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::DuplicateTraitDef { first_def, second_def }; + let err = DefCollectorErrorKind::Duplicate { + typ: "trait definition".to_string(), + first_def, + second_def, + }; errors.push(err.into_file_diagnostic(self.file_id)); } - // And store the TypeId -> TraitType mapping somewhere it is reachable + // And store the TraitId -> TraitType mapping somewhere it is reachable let unresolved = UnresolvedTrait { file_id: self.file_id, module_id: self.module_id, @@ -579,7 +596,11 @@ impl<'a> ModCollector<'a> { if let Err((first_def, second_def)) = modules[self.module_id.0].declare_child_module(mod_name.to_owned(), mod_id) { - let err = DefCollectorErrorKind::DuplicateModuleDecl { first_def, second_def }; + let err = DefCollectorErrorKind::Duplicate { + typ: "module".to_string(), + first_def, + second_def, + }; errors.push(err.into_file_diagnostic(self.file_id)); return None; } diff --git a/crates/noirc_frontend/src/hir/def_collector/errors.rs b/crates/noirc_frontend/src/hir/def_collector/errors.rs index af91fd50429..0b0a1bcb727 100644 --- a/crates/noirc_frontend/src/hir/def_collector/errors.rs +++ b/crates/noirc_frontend/src/hir/def_collector/errors.rs @@ -8,26 +8,16 @@ use thiserror::Error; #[derive(Error, Debug)] pub enum DefCollectorErrorKind { - #[error("duplicate function found in namespace")] - DuplicateFunction { first_def: Ident, second_def: Ident }, - #[error("duplicate type definition found in namespace")] - DuplicateTypeDef { first_def: Ident, second_def: Ident }, - #[error("duplicate trait definition found in namespace")] - DuplicateTraitDef { first_def: Ident, second_def: Ident }, - #[error("duplicate module found in namespace")] - DuplicateModuleDecl { first_def: Ident, second_def: Ident }, - #[error("duplicate import")] - DuplicateImport { first_def: Ident, second_def: Ident }, - #[error("duplicate global found in namespace")] - DuplicateGlobal { first_def: Ident, second_def: Ident }, + #[error("duplicate {typ} found in namespace")] + Duplicate { typ: String, first_def: Ident, second_def: Ident }, #[error("unresolved import")] UnresolvedModuleDecl { mod_name: Ident }, #[error("path resolution error")] PathResolutionError(PathResolutionError), #[error("Non-struct type used in impl")] NonStructTypeInImpl { span: Span }, - #[error("Feature not implemented")] - SimpleError { primary_message: String, secondary_message: String, span: Span }, + #[error("Generic error")] + GenericError { primary_message: String, secondary_message: String, span: Span }, } impl DefCollectorErrorKind { @@ -36,55 +26,27 @@ impl DefCollectorErrorKind { } } -fn report_duplicate( - primary_message: String, - duplicate_type: &str, - first_def: Ident, - second_def: Ident, -) -> Diagnostic { - let first_span = first_def.0.span(); - let second_span = second_def.0.span(); - let mut diag = Diagnostic::simple_error( - primary_message, - format!("first {} found here", duplicate_type), - first_span, - ); - diag.add_secondary(format!("second {} found here", duplicate_type), second_span); - diag -} - impl From for Diagnostic { fn from(error: DefCollectorErrorKind) -> Diagnostic { match error { - DefCollectorErrorKind::DuplicateFunction { first_def, second_def } => { + DefCollectorErrorKind::Duplicate { typ, first_def, second_def } => { let primary_message = format!("duplicate definitions of {} function found", &first_def.0.contents); - report_duplicate(primary_message, "function definition", first_def, second_def) - } - DefCollectorErrorKind::DuplicateTypeDef { first_def, second_def } => { - let primary_message = - format!("duplicate definitions of {} type found", &first_def.0.contents); - report_duplicate(primary_message, "type definition", first_def, second_def) - } - DefCollectorErrorKind::DuplicateTraitDef { first_def, second_def } => { - let primary_message = - format!("duplicate definitions of {} trait found", &first_def.0.contents); - report_duplicate(primary_message, "trait definition", first_def, second_def) - } - DefCollectorErrorKind::DuplicateModuleDecl { first_def, second_def } => { - let primary_message = - format!("module {} has been declared twice", &first_def.0.contents); - report_duplicate(primary_message, "module declaration", first_def, second_def) - } - DefCollectorErrorKind::DuplicateImport { first_def, second_def } => { - let primary_message = - format!("module `{}` is imported multiple times", &first_def.0.contents); - report_duplicate(primary_message, "import", first_def, second_def) - } - DefCollectorErrorKind::DuplicateGlobal { first_def, second_def } => { - let primary_message = - format!("the global `{}` is defined multiple times", &first_def.0.contents); - report_duplicate(primary_message, "global declaration", first_def, second_def) + { + let duplicate_type: &str = &typ; + let first_span = first_def.0.span(); + let second_span = second_def.0.span(); + let mut diag = Diagnostic::simple_error( + primary_message, + format!("first {} found here", duplicate_type), + first_span, + ); + diag.add_secondary( + format!("second {} found here", duplicate_type), + second_span, + ); + diag + } } DefCollectorErrorKind::UnresolvedModuleDecl { mod_name } => { let span = mod_name.0.span(); @@ -102,7 +64,7 @@ impl From for Diagnostic { "Only struct types may have implementation methods".into(), span, ), - DefCollectorErrorKind::SimpleError { primary_message, secondary_message, span } => { + DefCollectorErrorKind::GenericError { primary_message, secondary_message, span } => { Diagnostic::simple_error(primary_message, secondary_message, span) } } diff --git a/crates/noirc_frontend/src/hir/resolution/resolver.rs b/crates/noirc_frontend/src/hir/resolution/resolver.rs index 6617e04c788..233e52c7342 100644 --- a/crates/noirc_frontend/src/hir/resolution/resolver.rs +++ b/crates/noirc_frontend/src/hir/resolution/resolver.rs @@ -1699,28 +1699,6 @@ mod test { } } - fn resolve_struct_method_call_expr() { - let src = r#" - - struct Foo { - bar: Field - } - - impl Foo { - fn default(x: Field,_y: Field) -> Self { - Self { bar: x } - } - } - - fn main(x: Field, y: Field) { - let _foo = Foo::default(x,y); - } - "#; - - let errors = resolve_src_code(src, vec!["main"]); - assert!(errors.is_empty()); - } - fn path_unresolved_error(err: ResolverError, expected_unresolved_path: &str) { match err { ResolverError::PathResolutionError(PathResolutionError::Unresolved(name)) => { From 8b0510cf4f12f7b7e77b0d8bb9d841a73ca36b01 Mon Sep 17 00:00:00 2001 From: Yordan Madzhunkov Date: Fri, 4 Aug 2023 10:44:39 +0300 Subject: [PATCH 24/27] Rename TraitType to Trait --- .../src/hir/resolution/resolver.rs | 4 ++-- crates/noirc_frontend/src/hir_def/types.rs | 16 ++++++++-------- crates/noirc_frontend/src/node_interner.rs | 8 ++++---- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/crates/noirc_frontend/src/hir/resolution/resolver.rs b/crates/noirc_frontend/src/hir/resolution/resolver.rs index 233e52c7342..7a29d828509 100644 --- a/crates/noirc_frontend/src/hir/resolution/resolver.rs +++ b/crates/noirc_frontend/src/hir/resolution/resolver.rs @@ -35,7 +35,7 @@ use crate::{ }; use crate::{ ArrayLiteral, ContractFunctionType, Generics, LValue, NoirStruct, NoirTypeAlias, Path, Pattern, - Shared, StructType, TraitType, Type, TypeAliasType, TypeBinding, TypeVariable, UnaryOp, + Shared, StructType, Trait, Type, TypeAliasType, TypeBinding, TypeVariable, UnaryOp, UnresolvedGenerics, UnresolvedType, UnresolvedTypeExpression, ERROR_IDENT, }; use fm::FileId; @@ -1228,7 +1228,7 @@ impl<'a> Resolver<'a> { self.interner.get_struct(type_id) } - pub fn get_trait(&self, type_id: TraitId) -> Shared { + pub fn get_trait(&self, type_id: TraitId) -> Shared { self.interner.get_trait(type_id) } diff --git a/crates/noirc_frontend/src/hir_def/types.rs b/crates/noirc_frontend/src/hir_def/types.rs index 29b63456900..27505e14093 100644 --- a/crates/noirc_frontend/src/hir_def/types.rs +++ b/crates/noirc_frontend/src/hir_def/types.rs @@ -54,7 +54,7 @@ pub enum Type { /// A user-defined trait type. The `Shared` field here refers to /// the shared definition for each instance of this trait type. The `Vec` /// represents the generic arguments (if any) to this trait type. - Trait(Shared, Vec), + Trait(Shared, Vec), /// A tuple type with the given list of fields in the order they appear in source code. Tuple(Vec), @@ -150,7 +150,7 @@ pub enum TraitItemType { /// rust struct will be shared across all Type::Trait variants that represent /// the same trait type. #[derive(Debug, Eq)] -pub struct TraitType { +pub struct Trait { /// A unique id representing this struct type. Used to check if two /// struct types are equal. pub id: TraitId, @@ -180,31 +180,31 @@ impl PartialEq for StructType { } } -impl std::hash::Hash for TraitType { +impl std::hash::Hash for Trait { fn hash(&self, state: &mut H) { self.id.hash(state); } } -impl PartialEq for TraitType { +impl PartialEq for Trait { fn eq(&self, other: &Self) -> bool { self.id == other.id } } -impl TraitType { +impl Trait { pub fn new( id: TraitId, name: Ident, span: Span, items: Vec, generics: Generics, - ) -> TraitType { - TraitType { id, name, span, items, generics } + ) -> Trait { + Trait { id, name, span, items, generics } } } -impl std::fmt::Display for TraitType { +impl std::fmt::Display for Trait { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { write!(f, "{}", self.name) } diff --git a/crates/noirc_frontend/src/node_interner.rs b/crates/noirc_frontend/src/node_interner.rs index 1505e183d64..83d3de84f97 100644 --- a/crates/noirc_frontend/src/node_interner.rs +++ b/crates/noirc_frontend/src/node_interner.rs @@ -11,7 +11,7 @@ use crate::hir::def_collector::dc_crate::{UnresolvedStruct, UnresolvedTrait, Unr use crate::hir::def_map::{LocalModuleId, ModuleId}; use crate::hir::StorageSlot; use crate::hir_def::stmt::HirLetStatement; -use crate::hir_def::types::{StructType, TraitType, Type}; +use crate::hir_def::types::{StructType, Trait, Type}; use crate::hir_def::{ expr::HirExpression, function::{FuncMeta, HirFunction}, @@ -66,7 +66,7 @@ pub struct NodeInterner { // Each struct definition is possibly shared across multiple type nodes. // It is also mutated through the RefCell during name resolution to append // methods from impls to the type. - traits: HashMap>, + 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 @@ -327,7 +327,7 @@ impl NodeInterner { pub fn push_empty_trait(&mut self, type_id: TraitId, typ: &UnresolvedTrait) { self.traits.insert( type_id, - Shared::new(TraitType::new( + Shared::new(Trait::new( type_id, typ.trait_def.name.clone(), typ.trait_def.span, @@ -591,7 +591,7 @@ impl NodeInterner { &self.type_aliases[id.0] } - pub fn get_trait(&self, id: TraitId) -> Shared { + pub fn get_trait(&self, id: TraitId) -> Shared { self.traits[&id].clone() } From 853f0e2f3fe66c541711bbb198cd66931761aa6f Mon Sep 17 00:00:00 2001 From: Yordan Madzhunkov Date: Fri, 4 Aug 2023 10:52:02 +0300 Subject: [PATCH 25/27] Fix comment & PR remarks --- crates/noirc_frontend/src/hir_def/types.rs | 4 ++-- crates/noirc_frontend/src/monomorphization/mod.rs | 5 +---- crates/noirc_frontend/src/node_interner.rs | 5 ++++- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/crates/noirc_frontend/src/hir_def/types.rs b/crates/noirc_frontend/src/hir_def/types.rs index 27505e14093..ab5bb19c1b1 100644 --- a/crates/noirc_frontend/src/hir_def/types.rs +++ b/crates/noirc_frontend/src/hir_def/types.rs @@ -151,8 +151,8 @@ pub enum TraitItemType { /// the same trait type. #[derive(Debug, Eq)] pub struct Trait { - /// A unique id representing this struct type. Used to check if two - /// struct types are equal. + /// A unique id representing this trait type. Used to check if two + /// struct traits are equal. pub id: TraitId, pub items: Vec, diff --git a/crates/noirc_frontend/src/monomorphization/mod.rs b/crates/noirc_frontend/src/monomorphization/mod.rs index c52692f5513..fa80c1673ce 100644 --- a/crates/noirc_frontend/src/monomorphization/mod.rs +++ b/crates/noirc_frontend/src/monomorphization/mod.rs @@ -652,10 +652,6 @@ impl<'interner> Monomorphizer<'interner> { ast::Type::Tuple(fields) } - HirType::Trait(def, _args) => { - unreachable!("Not sure what to do here def = {:?}", def) - } - HirType::Tuple(fields) => { let fields = vecmap(fields, Self::convert_type); ast::Type::Tuple(fields) @@ -675,6 +671,7 @@ impl<'interner> Monomorphizer<'interner> { HirType::Forall(_, _) | HirType::Constant(_) | HirType::NotConstant + | HirType::Trait(..) | HirType::Error => { unreachable!("Unexpected type {} found", typ) } diff --git a/crates/noirc_frontend/src/node_interner.rs b/crates/noirc_frontend/src/node_interner.rs index 83d3de84f97..ad63ca8fc74 100644 --- a/crates/noirc_frontend/src/node_interner.rs +++ b/crates/noirc_frontend/src/node_interner.rs @@ -63,9 +63,12 @@ pub struct NodeInterner { // Trait map. // - // Each struct definition is possibly shared across multiple type nodes. + // Each trait definition is possibly shared across multiple type nodes. // It is also mutated through the RefCell during name resolution to append // methods from impls to the type. + // + // 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, From 31fb5f7b45783d0875880ff439f702518b4ee87a Mon Sep 17 00:00:00 2001 From: Yordan Madzhunkov Date: Fri, 4 Aug 2023 11:01:51 +0300 Subject: [PATCH 26/27] Clean up commented error code" --- crates/noirc_frontend/src/parser/errors.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/crates/noirc_frontend/src/parser/errors.rs b/crates/noirc_frontend/src/parser/errors.rs index 38ea138b54f..abd50263d0f 100644 --- a/crates/noirc_frontend/src/parser/errors.rs +++ b/crates/noirc_frontend/src/parser/errors.rs @@ -27,9 +27,6 @@ pub enum ParserErrorReason { PatternInTraitFunctionParameter, #[error("Traits are an experimental feature that are not yet in a working state")] TraitsAreExperimental, - // TODO(GenericParameterNotFoundInFunction): Consider implementing this - // #[error("The identifier '{0}' specified in the where clause is not present in the list of generic parameters of the function")] - // GenericParameterNotFoundInFunction(String), #[error("Where clauses are allowed only on functions with generic parameters")] WhereClauseOnNonGenericFunction, } From 36f74f9ad7cbbdda797de0b99b3fdb0d7c695d1a Mon Sep 17 00:00:00 2001 From: Yordan Madzhunkov Date: Fri, 4 Aug 2023 11:15:16 +0300 Subject: [PATCH 27/27] Fix parser test and constaint keyword definition in Traits --- crates/noirc_frontend/src/parser/parser.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/crates/noirc_frontend/src/parser/parser.rs b/crates/noirc_frontend/src/parser/parser.rs index df7c140aa04..d8aa5342183 100644 --- a/crates/noirc_frontend/src/parser/parser.rs +++ b/crates/noirc_frontend/src/parser/parser.rs @@ -390,8 +390,7 @@ fn optional_default_value() -> impl NoirParser> { } fn trait_constant_declaration() -> impl NoirParser { - // TODO: Shall we use `comptime` or `const` here? - keyword(Keyword::CompTime) + keyword(Keyword::Let) .ignore_then(ident()) .then_ignore(just(Token::Colon)) .then(parse_type()) @@ -1831,12 +1830,8 @@ mod test { "trait TraitAcceptingMutableRef { fn foo(&mut self); }", "trait TraitWithTypeBoundOperation { fn identity() -> Self; }", "trait TraitWithAssociatedType { type Element; fn item(self, index: Field) -> Self::Element; }", - // TODO: Shall we use `const` instead? - "trait TraitWithAssociatedConstant { comptime Size: Field; }", - "trait TraitWithAssociatedConstantWithDefaultValue { comptime Size: Field = 10; }", - "trait GenericTrait { fn elem(&mut self, index: Field) -> T; }", - "trait GenericTraitWithConstraints where T: SomeTrait { fn elem(self, index: Field) -> T; }", - "trait TraitWithMultipleGenericParams where A: SomeTrait, B: AnotherTrait { comptime Size: Field; fn zero() -> Self; }", + "trait TraitWithAssociatedConstant { let Size: Field; }", + "trait TraitWithAssociatedConstantWithDefaultValue { let Size: Field = 10; }", ], ); @@ -1846,6 +1841,11 @@ mod test { "trait MissingBody", "trait WrongDelimiter { fn foo() -> u8, fn bar() -> u8 }", "trait WhereClauseWithoutGenerics where A: SomeTrait { }", + // TODO: when implemnt generics in traits the following 3 should pass + "trait GenericTrait { fn elem(&mut self, index: Field) -> T; }", + "trait GenericTraitWithConstraints where T: SomeTrait { fn elem(self, index: Field) -> T; }", + "trait TraitWithMultipleGenericParams where A: SomeTrait, B: AnotherTrait { comptime Size: Field; fn zero() -> Self; }", + ], ); }