diff --git a/crates/nargo/tests/test_data/generics/src/main.nr b/crates/nargo/tests/test_data/generics/src/main.nr index f9746690104..56078a304e0 100644 --- a/crates/nargo/tests/test_data/generics/src/main.nr +++ b/crates/nargo/tests/test_data/generics/src/main.nr @@ -8,10 +8,45 @@ fn foo(bar: Bar) { constrain bar.one == bar.two; } +struct BigInt { + limbs: [u32; N], +} + +impl BigInt { + // `N` is in scope of all methods in the impl + fn first(first: BigInt, second: BigInt) -> Self { + constrain first.limbs != second.limbs; + first + } + + fn second(first: BigInt, second: Self) -> Self { + constrain first.limbs != second.limbs; + second + } +} + +impl Bar { + fn get_other(self) -> Field { + self.other + } +} + fn main(x: Field, y: Field) { let bar1: Bar = Bar { one: x, two: y, other: 0 }; let bar2 = Bar { one: x, two: y, other: [0] }; foo(bar1); foo(bar2); + + // Test generic impls + let int1 = BigInt { limbs: [1] }; + let int2 = BigInt { limbs: [2] }; + let BigInt { limbs } = int1.second(int2).first(int1); + constrain limbs == int2.limbs; + + // Test impl exclusively for Bar + constrain bar1.get_other() == bar1.other; + + // Expected type error + // constrain bar2.get_other() == bar2.other; } diff --git a/crates/noirc_driver/src/lib.rs b/crates/noirc_driver/src/lib.rs index dc568419105..e0314eb50a3 100644 --- a/crates/noirc_driver/src/lib.rs +++ b/crates/noirc_driver/src/lib.rs @@ -3,7 +3,7 @@ use acvm::acir::circuit::Circuit; use acvm::Language; use fm::FileType; use noirc_abi::Abi; -use noirc_errors::{DiagnosableError, ReportedError, Reporter}; +use noirc_errors::{reporter, ReportedError}; use noirc_evaluator::create_circuit; use noirc_frontend::graph::{CrateId, CrateName, CrateType, LOCAL_CRATE}; use noirc_frontend::hir::def_map::CrateDefMap; @@ -47,14 +47,7 @@ impl Driver { pub fn file_compiles(&mut self) -> bool { let mut errs = vec![]; CrateDefMap::collect_defs(LOCAL_CRATE, &mut self.context, &mut errs); - for errors in &errs { - Reporter::with_diagnostics( - Some(errors.file_id), - &self.context.file_manager, - &errors.errors, - false, - ); - } + reporter::report_all(&self.context.file_manager, &errs, false); errs.is_empty() } @@ -135,17 +128,8 @@ impl Driver { pub fn check_crate(&mut self, allow_warnings: bool) -> Result<(), ReportedError> { let mut errs = vec![]; CrateDefMap::collect_defs(LOCAL_CRATE, &mut self.context, &mut errs); - let mut error_count = 0; - for errors in &errs { - error_count += Reporter::with_diagnostics( - Some(errors.file_id), - &self.context.file_manager, - &errors.errors, - allow_warnings, - ); - } - - Reporter::finish(error_count) + let error_count = reporter::report_all(&self.context.file_manager, &errs, allow_warnings); + reporter::finish_report(error_count) } pub fn compute_abi(&self) -> Option { @@ -207,12 +191,10 @@ impl Driver { Err(err) => { // The FileId here will be the file id of the file with the main file // Errors will be shown at the call site without a stacktrace - let file_id = err.location.map(|loc| loc.file); - let error = &[err.to_diagnostic()]; + let file = err.location.map(|loc| loc.file); let files = &self.context.file_manager; - - let error_count = Reporter::with_diagnostics(file_id, files, error, allow_warnings); - Reporter::finish(error_count)?; + let error = reporter::report(files, &err.into(), file, allow_warnings); + reporter::finish_report(error as u32)?; Err(ReportedError) } } diff --git a/crates/noirc_errors/src/lib.rs b/crates/noirc_errors/src/lib.rs index a4d412dd82b..efde37d9803 100644 --- a/crates/noirc_errors/src/lib.rs +++ b/crates/noirc_errors/src/lib.rs @@ -1,19 +1,14 @@ mod position; -mod reporter; - +pub mod reporter; pub use position::{Location, Position, Span, Spanned}; -pub use reporter::*; - -pub trait DiagnosableError { - fn to_diagnostic(&self) -> CustomDiagnostic; -} +pub use reporter::{CustomDiagnostic, DiagnosticKind}; /// Returned when the Reporter finishes after reporting errors #[derive(Copy, Clone)] pub struct ReportedError; #[derive(Debug, PartialEq, Eq)] -pub struct CollectedErrors { +pub struct FileDiagnostic { pub file_id: fm::FileId, - pub errors: Vec, + pub diagnostic: CustomDiagnostic, } diff --git a/crates/noirc_errors/src/reporter.rs b/crates/noirc_errors/src/reporter.rs index 60a5e07fcaf..dec215295ad 100644 --- a/crates/noirc_errors/src/reporter.rs +++ b/crates/noirc_errors/src/reporter.rs @@ -1,10 +1,9 @@ -use crate::{ReportedError, Span}; +use crate::{FileDiagnostic, ReportedError, Span}; use codespan_reporting::diagnostic::{Diagnostic, Label}; use codespan_reporting::term; use codespan_reporting::term::termcolor::{ Color, ColorChoice, ColorSpec, StandardStream, WriteColor, }; -use fm::FileId; use std::io::Write; #[derive(Debug, PartialEq, Eq)] @@ -57,6 +56,10 @@ impl CustomDiagnostic { } } + pub fn in_file(self, file_id: fm::FileId) -> FileDiagnostic { + FileDiagnostic { file_id, diagnostic: self } + } + pub fn add_note(&mut self, message: String) { self.notes.push(message); } @@ -64,6 +67,10 @@ impl CustomDiagnostic { pub fn add_secondary(&mut self, message: String, span: Span) { self.secondaries.push(CustomLabel::new(message, span)); } + + fn is_error(&self) -> bool { + matches!(self.kind, DiagnosticKind::Error) + } } impl std::fmt::Display for CustomDiagnostic { @@ -94,71 +101,72 @@ impl CustomLabel { } } -pub struct Reporter; - -impl Reporter { - /// Writes the given diagnostics to stderr and returns the count - /// of diagnostics that were errors. - pub fn with_diagnostics( - file_id: Option, - files: &fm::FileManager, - diagnostics: &[CustomDiagnostic], - allow_warnings: bool, - ) -> usize { - let mut error_count = 0; - - // Convert each Custom Diagnostic into a diagnostic - let diagnostics = diagnostics.iter().map(|cd| { - let diagnostic = match (cd.kind, allow_warnings) { - (DiagnosticKind::Warning, true) => Diagnostic::warning(), - _ => { - error_count += 1; - Diagnostic::error() - } - }; - - let secondary_labels = if let Some(f_id) = file_id { - cd.secondaries - .iter() - .map(|sl| { - let start_span = sl.span.start() as usize; - let end_span = sl.span.end() as usize + 1; - - Label::secondary(f_id.as_usize(), start_span..end_span) - .with_message(&sl.message) - }) - .collect() - } else { - Vec::new() - }; - diagnostic - .with_message(&cd.message) - .with_labels(secondary_labels) - .with_notes(cd.notes.clone()) - }); +/// Writes the given diagnostics to stderr and returns the count +/// of diagnostics that were errors. +pub fn report_all( + files: &fm::FileManager, + diagnostics: &[FileDiagnostic], + allow_warnings: bool, +) -> u32 { + diagnostics + .iter() + .map(|error| report(files, &error.diagnostic, Some(error.file_id), allow_warnings) as u32) + .sum() +} - let writer = StandardStream::stderr(ColorChoice::Always); - let config = codespan_reporting::term::Config::default(); +/// Report the given diagnostic, and return true if it was an error +pub fn report( + files: &fm::FileManager, + custom_diagnostic: &CustomDiagnostic, + file: Option, + allow_warnings: bool, +) -> bool { + let writer = StandardStream::stderr(ColorChoice::Always); + let config = codespan_reporting::term::Config::default(); - for diagnostic in diagnostics { - term::emit(&mut writer.lock(), &config, files.as_simple_files(), &diagnostic).unwrap(); - } + let diagnostic = convert_diagnostic(custom_diagnostic, file, allow_warnings); + term::emit(&mut writer.lock(), &config, files.as_simple_files(), &diagnostic).unwrap(); - error_count - } + !allow_warnings || custom_diagnostic.is_error() +} - pub fn finish(error_count: usize) -> Result<(), ReportedError> { - if error_count != 0 { - let writer = StandardStream::stderr(ColorChoice::Always); - let mut writer = writer.lock(); +fn convert_diagnostic( + cd: &CustomDiagnostic, + file: Option, + allow_warnings: bool, +) -> Diagnostic { + let diagnostic = match (cd.kind, allow_warnings) { + (DiagnosticKind::Warning, true) => Diagnostic::warning(), + _ => Diagnostic::error(), + }; + + let secondary_labels = if let Some(file_id) = file { + cd.secondaries + .iter() + .map(|sl| { + let start_span = sl.span.start() as usize; + let end_span = sl.span.end() as usize + 1; + Label::secondary(file_id.as_usize(), start_span..end_span).with_message(&sl.message) + }) + .collect() + } else { + vec![] + }; + + diagnostic.with_message(&cd.message).with_labels(secondary_labels).with_notes(cd.notes.clone()) +} - writer.set_color(ColorSpec::new().set_fg(Some(Color::Red))).unwrap(); - writeln!(&mut writer, "error: aborting due to {error_count} previous errors").unwrap(); - writer.reset().ok(); // Ignore any IO errors, we're exiting at this point anyway +pub fn finish_report(error_count: u32) -> Result<(), ReportedError> { + if error_count != 0 { + let writer = StandardStream::stderr(ColorChoice::Always); + let mut writer = writer.lock(); - Err(ReportedError) - } else { - Ok(()) - } + writer.set_color(ColorSpec::new().set_fg(Some(Color::Red))).unwrap(); + writeln!(&mut writer, "error: aborting due to {error_count} previous errors").unwrap(); + writer.reset().ok(); + + Err(ReportedError) + } else { + Ok(()) } } diff --git a/crates/noirc_evaluator/src/errors.rs b/crates/noirc_evaluator/src/errors.rs index 358bd3717e3..e90726a1211 100644 --- a/crates/noirc_evaluator/src/errors.rs +++ b/crates/noirc_evaluator/src/errors.rs @@ -1,4 +1,4 @@ -use noirc_errors::{CustomDiagnostic as Diagnostic, DiagnosableError, Location}; +use noirc_errors::{CustomDiagnostic as Diagnostic, Location}; use thiserror::Error; #[derive(Debug)] @@ -64,11 +64,11 @@ impl RuntimeErrorKind { } } -impl DiagnosableError for RuntimeError { - fn to_diagnostic(&self) -> Diagnostic { +impl From for Diagnostic { + fn from(error: RuntimeError) -> Diagnostic { let span = - if let Some(loc) = self.location { loc.span } else { noirc_errors::Span::new(0..0) }; - match &self.kind { + if let Some(loc) = error.location { loc.span } else { noirc_errors::Span::new(0..0) }; + match &error.kind { RuntimeErrorKind::ArrayOutOfBounds { index, bound } => Diagnostic::simple_error( "index out of bounds".to_string(), format!("out of bounds error, index is {index} but length is {bound}"), diff --git a/crates/noirc_frontend/src/ast/expression.rs b/crates/noirc_frontend/src/ast/expression.rs index db08c7dae01..15ae1d8aaa8 100644 --- a/crates/noirc_frontend/src/ast/expression.rs +++ b/crates/noirc_frontend/src/ast/expression.rs @@ -26,6 +26,10 @@ pub enum ExpressionKind { Error, } +/// A Vec of unresolved names for type variables. +/// For `fn foo(...)` this corresponds to vec!["A", "B"]. +pub type UnresolvedGenerics = Vec; + impl ExpressionKind { pub fn into_path(self) -> Option { match self { @@ -306,7 +310,7 @@ pub struct Lambda { pub struct FunctionDefinition { pub name: Ident, pub attribute: Option, // XXX: Currently we only have one attribute defined. If more attributes are needed per function, we can make this a vector and make attribute definition more expressive - pub generics: Vec, + pub generics: UnresolvedGenerics, pub parameters: Vec<(Pattern, UnresolvedType, noirc_abi::AbiVisibility)>, pub body: BlockExpression, pub span: Span, diff --git a/crates/noirc_frontend/src/ast/mod.rs b/crates/noirc_frontend/src/ast/mod.rs index 725455a8598..057fa42345d 100644 --- a/crates/noirc_frontend/src/ast/mod.rs +++ b/crates/noirc_frontend/src/ast/mod.rs @@ -18,7 +18,7 @@ use iter_extended::vecmap; /// The parser parses types as 'UnresolvedType's which /// require name resolution to resolve any type names used /// for structs within, but are otherwise identical to Types. -#[derive(Debug, PartialEq, Eq, Clone)] +#[derive(Debug, PartialEq, Eq, Clone, Hash)] pub enum UnresolvedType { FieldElement(CompTime), Array(Option, Box), // [4]Witness = Array(4, Witness) @@ -43,7 +43,7 @@ pub enum UnresolvedType { /// The precursor to TypeExpression, this is the type that the parser allows /// to be used in the length position of an array type. Only constants, variables, /// and numeric binary operators are allowed here. -#[derive(Debug, PartialEq, Eq, Clone)] +#[derive(Debug, PartialEq, Eq, Clone, Hash)] pub enum UnresolvedTypeExpression { Variable(Path), Constant(u64, Span), diff --git a/crates/noirc_frontend/src/ast/structure.rs b/crates/noirc_frontend/src/ast/structure.rs index dc01a7d5c90..47dd432e739 100644 --- a/crates/noirc_frontend/src/ast/structure.rs +++ b/crates/noirc_frontend/src/ast/structure.rs @@ -1,12 +1,14 @@ use std::fmt::Display; -use crate::{Ident, NoirFunction, Path, UnresolvedType}; +use crate::{Ident, NoirFunction, UnresolvedGenerics, UnresolvedType}; +use iter_extended::vecmap; use noirc_errors::Span; +/// Ast node for a struct #[derive(Clone, Debug, PartialEq, Eq)] pub struct NoirStruct { pub name: Ident, - pub generics: Vec, + pub generics: UnresolvedGenerics, pub fields: Vec<(Ident, UnresolvedType)>, pub span: Span, } @@ -22,15 +24,21 @@ impl NoirStruct { } } +/// Ast node for an impl #[derive(Clone, Debug)] pub struct NoirImpl { - pub type_path: Path, + pub object_type: UnresolvedType, + pub type_span: Span, + pub generics: UnresolvedGenerics, pub methods: Vec, } impl Display for NoirStruct { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - writeln!(f, "struct {} {{", self.name)?; + let generics = vecmap(&self.generics, |generic| generic.to_string()); + let generics = if generics.is_empty() { "".into() } else { generics.join(", ") }; + + writeln!(f, "struct {}{} {{", self.name, generics)?; for (name, typ) in self.fields.iter() { writeln!(f, " {name}: {typ},")?; @@ -42,7 +50,10 @@ impl Display for NoirStruct { impl Display for NoirImpl { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - writeln!(f, "impl {} {{", self.type_path)?; + let generics = vecmap(&self.generics, |generic| generic.to_string()); + let generics = if generics.is_empty() { "".into() } else { generics.join(", ") }; + + writeln!(f, "impl{} {} {{", generics, self.object_type)?; for method in self.methods.iter() { let method = method.to_string(); 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 1b4bd7991d1..1c5dc4f178f 100644 --- a/crates/noirc_frontend/src/hir/def_collector/dc_crate.rs +++ b/crates/noirc_frontend/src/hir/def_collector/dc_crate.rs @@ -12,13 +12,16 @@ use crate::hir::type_check::type_check; use crate::hir::type_check::type_check_func; use crate::hir::Context; use crate::node_interner::{FuncId, NodeInterner, StmtId, StructId}; -use crate::{Generics, Ident, LetStatement, NoirFunction, NoirStruct, ParsedModule, Path, Type}; +use crate::{ + Generics, Ident, LetStatement, NoirFunction, NoirStruct, ParsedModule, Shared, Type, + TypeBinding, UnresolvedGenerics, UnresolvedType, +}; use fm::FileId; -use noirc_errors::CollectedErrors; -use noirc_errors::DiagnosableError; -use std::collections::{BTreeMap, HashMap}; - use iter_extended::vecmap; +use noirc_errors::Span; +use noirc_errors::{CustomDiagnostic, FileDiagnostic}; +use std::collections::{BTreeMap, HashMap}; +use std::rc::Rc; /// Stores all of the unresolved functions in a particular file/mod pub struct UnresolvedFunctions { @@ -53,11 +56,15 @@ pub struct DefCollector { pub(crate) collected_functions: Vec, pub(crate) collected_types: HashMap, pub(crate) collected_globals: Vec, - /// collected impls maps the type name and the module id in which - /// the impl is defined to the functions contained in that impl - pub(crate) collected_impls: HashMap<(Path, LocalModuleId), Vec>, + pub(crate) collected_impls: ImplMap, } +/// Maps the type and the module id in which the impl is defined to the functions contained in that +/// impl along with the generics declared on the impl itself. This also contains the Span +/// of the object_type of the impl, used to issue an error if the object type fails to resolve. +type ImplMap = + HashMap<(UnresolvedType, LocalModuleId), Vec<(UnresolvedGenerics, Span, UnresolvedFunctions)>>; + impl DefCollector { fn new(def_map: CrateDefMap) -> DefCollector { DefCollector { @@ -78,7 +85,7 @@ impl DefCollector { context: &mut Context, ast: ParsedModule, root_file_id: FileId, - errors: &mut Vec, + errors: &mut Vec, ) { let crate_id = def_map.krate; @@ -122,10 +129,8 @@ impl DefCollector { for unresolved_import in unresolved.into_iter() { // File if that the import was declared let file_id = current_def_map.modules[unresolved_import.module_id.0].origin.file_id(); - let diagnostic = DefCollectorErrorKind::UnresolvedImport { import: unresolved_import } - .to_diagnostic(); - let err = CollectedErrors { file_id, errors: vec![diagnostic] }; - errors.push(err); + let error = DefCollectorErrorKind::UnresolvedImport { import: unresolved_import }; + errors.push(error.into_file_diagnostic(file_id)); } // Populate module namespaces according to the imports used @@ -139,11 +144,7 @@ impl DefCollector { if let Err((first_def, second_def)) = result { let err = DefCollectorErrorKind::DuplicateImport { first_def, second_def }; - - errors.push(CollectedErrors { - file_id: root_file_id, - errors: vec![err.to_diagnostic()], - }); + errors.push(err.into_file_diagnostic(root_file_id)); } } } @@ -161,7 +162,7 @@ impl DefCollector { collect_impls(context, crate_id, &def_collector.collected_impls, errors); // Lower each function in the crate. This is now possible since imports have been resolved - let file_func_ids = resolve_functions( + let file_func_ids = resolve_free_functions( &mut context.def_interner, crate_id, &context.def_maps, @@ -190,33 +191,29 @@ impl DefCollector { fn collect_impls( context: &mut Context, crate_id: CrateId, - collected_impls: &HashMap<(Path, LocalModuleId), Vec>, - errors: &mut Vec, + collected_impls: &ImplMap, + errors: &mut Vec, ) { let interner = &mut context.def_interner; let def_maps = &mut context.def_maps; - for ((path, module_id), methods) in collected_impls { + for ((unresolved_type, module_id), methods) in collected_impls { let path_resolver = StandardPathResolver::new(ModuleId { local_id: *module_id, krate: crate_id }); let file = def_maps[&crate_id].module_file_id(*module_id); - for unresolved in methods { - let resolver = Resolver::new(interner, &path_resolver, def_maps, file); - let (typ, more_errors) = resolver.lookup_type_for_impl(path.clone()); - if !more_errors.is_empty() { - errors.push(CollectedErrors { - file_id: unresolved.file_id, - errors: vecmap(more_errors, |err| err.into_diagnostic()), - }) - } + for (generics, span, unresolved) in methods { + let mut resolver = Resolver::new(interner, &path_resolver, def_maps, file); + resolver.add_generics(generics); + let typ = resolver.resolve_type(unresolved_type.clone()); + + extend_errors(errors, unresolved.file_id, resolver.take_errors()); - if typ != StructId::dummy_id() { + if let Some(type_module) = get_local_id_from_type(&typ) { // Grab the scope defined by the struct type. Note that impls are a case // where the scope the methods are added to is not the same as the scope // they are resolved in. - let type_module = typ.0.local_id; let scope = &mut def_maps.get_mut(&crate_id).unwrap().modules[type_module.0].scope; // .define_func_def(name, func_id); @@ -226,17 +223,33 @@ fn collect_impls( if let Err((first_def, second_def)) = result { let err = DefCollectorErrorKind::DuplicateFunction { first_def, second_def }; - errors.push(CollectedErrors { - file_id: unresolved.file_id, - errors: vec![err.to_diagnostic()], - }); + errors.push(err.into_file_diagnostic(unresolved.file_id)); } } + } else if typ != Type::Error { + let span = *span; + let error = DefCollectorErrorKind::NonStructTypeInImpl { span }; + errors.push(error.into_file_diagnostic(unresolved.file_id)) } } } } +fn get_local_id_from_type(typ: &Type) -> Option { + match typ { + Type::Struct(definition, _) => Some(definition.borrow().id.0.local_id), + _ => None, + } +} + +fn extend_errors(errors: &mut Vec, file: fm::FileId, new_errors: Errs) +where + Errs: IntoIterator, + Err: Into, +{ + errors.extend(new_errors.into_iter().map(|err| err.into().in_file(file))) +} + fn resolve_globals( context: &mut Context, globals: Vec, @@ -271,16 +284,12 @@ fn resolve_globals( fn type_check_globals( interner: &mut NodeInterner, global_ids: Vec<(FileId, StmtId)>, - all_errors: &mut Vec, + all_errors: &mut Vec, ) { for (file_id, stmt_id) in global_ids { - let mut type_check_errs = vec![]; - type_check(interner, &stmt_id, &mut type_check_errs); - let errors = vecmap(type_check_errs, |error| error.into_diagnostic()); - - if !errors.is_empty() { - all_errors.push(CollectedErrors { file_id, errors }); - } + let mut errors = vec![]; + type_check(interner, &stmt_id, &mut errors); + extend_errors(all_errors, file_id, errors); } } @@ -290,7 +299,7 @@ fn resolve_structs( context: &mut Context, structs: HashMap, crate_id: CrateId, - errors: &mut Vec, + 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 @@ -313,24 +322,18 @@ fn resolve_struct_fields( context: &mut Context, krate: CrateId, unresolved: UnresolvedStruct, - errors: &mut Vec, + all_errors: &mut Vec, ) -> (Generics, BTreeMap) { let path_resolver = StandardPathResolver::new(ModuleId { local_id: unresolved.module_id, krate }); let file = unresolved.file_id; - let (generics, fields, errs) = + let (generics, fields, errors) = Resolver::new(&mut context.def_interner, &path_resolver, &context.def_maps, file) .resolve_struct_fields(unresolved.struct_def); - if !errs.is_empty() { - errors.push(CollectedErrors { - file_id: unresolved.file_id, - errors: vecmap(errs, |err| err.into_diagnostic()), - }) - } - + extend_errors(all_errors, unresolved.file_id, errors); (generics, fields) } @@ -338,100 +341,118 @@ fn resolve_impls( interner: &mut NodeInterner, crate_id: CrateId, def_maps: &HashMap, - collected_impls: HashMap<(Path, LocalModuleId), Vec>, - errors: &mut Vec, + collected_impls: ImplMap, + errors: &mut Vec, ) -> Vec<(FileId, FuncId)> { let mut file_method_ids = Vec::new(); - for ((path, module_id), methods) in collected_impls { + for ((unresolved_type, module_id), methods) in collected_impls { let path_resolver = StandardPathResolver::new(ModuleId { local_id: module_id, krate: crate_id }); let file = def_maps[&crate_id].module_file_id(module_id); - let mut resolver = Resolver::new(interner, &path_resolver, def_maps, file); - let self_type = resolver.lookup_struct(path); - let self_type_id = self_type.as_ref().map(|typ| typ.borrow().id); - - let mut ids = - resolve_functions(interner, crate_id, def_maps, methods, self_type_id, errors); - - if let Some(typ) = self_type { - for (file_id, method_id) in &ids { - let method_name = interner.function_name(method_id).to_owned(); - let mut typ = typ.borrow_mut(); - - if let Some(first_fn) = typ.methods.insert(method_name.clone(), *method_id) { - let error = ResolverError::DuplicateDefinition { - name: method_name, - first_span: interner.function_ident(&first_fn).span(), - second_span: interner.function_ident(method_id).span(), - }; - - errors.push(CollectedErrors { - file_id: *file_id, - errors: vec![error.into_diagnostic()], - }); + for (generics, _, functions) in methods { + let mut resolver = Resolver::new(interner, &path_resolver, def_maps, file); + resolver.add_generics(&generics); + let generics = resolver.get_generics().to_vec(); + let self_type = resolver.resolve_type(unresolved_type.clone()); + + let mut file_func_ids = resolve_function_set( + interner, + crate_id, + def_maps, + functions, + Some(self_type.clone()), + 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(); + + if let Some(first_fn) = + interner.add_method(&self_type, method_name.clone(), *method_id) + { + let error = ResolverError::DuplicateDefinition { + name: method_name, + first_span: interner.function_ident(&first_fn).span(), + second_span: interner.function_ident(method_id).span(), + }; + + errors.push(error.into_file_diagnostic(*file_id)); + } } } + file_method_ids.append(&mut file_func_ids); } - - file_method_ids.append(&mut ids); } file_method_ids } -fn resolve_functions( +fn resolve_free_functions( interner: &mut NodeInterner, crate_id: CrateId, def_maps: &HashMap, collected_functions: Vec, - self_type: Option, - errors: &mut Vec, + self_type: Option, + errors: &mut Vec, ) -> Vec<(FileId, FuncId)> { - let mut file_func_ids = Vec::new(); - // Lower each function in the crate. This is now possible since imports have been resolved - for unresolved_functions in collected_functions { - let file_id = unresolved_functions.file_id; - let mut collected_errors = CollectedErrors { file_id, errors: Vec::new() }; - - for (mod_id, func_id, func) in unresolved_functions.functions { - file_func_ids.push((file_id, func_id)); - - let path_resolver = - StandardPathResolver::new(ModuleId { local_id: mod_id, krate: crate_id }); - - let mut resolver = Resolver::new(interner, &path_resolver, def_maps, file_id); - resolver.set_self_type(self_type); + collected_functions + .into_iter() + .flat_map(|unresolved_functions| { + resolve_function_set( + interner, + crate_id, + def_maps, + unresolved_functions, + self_type.clone(), + vec![], // no impl generics + errors, + ) + }) + .collect() +} - let (hir_func, func_meta, errs) = resolver.resolve_function(func, func_id); - interner.push_fn_meta(func_meta, func_id); - interner.update_fn(func_id, hir_func); - collected_errors.errors.extend(errs.into_iter().map(|err| err.into_diagnostic())); - } - if !collected_errors.errors.is_empty() { - errors.push(collected_errors); - } - } +fn resolve_function_set( + interner: &mut NodeInterner, + crate_id: CrateId, + def_maps: &HashMap, + unresolved_functions: UnresolvedFunctions, + self_type: Option, + impl_generics: Vec<(Rc, Shared, Span)>, + errors: &mut Vec, +) -> Vec<(FileId, FuncId)> { + let file_id = unresolved_functions.file_id; - file_func_ids + vecmap(unresolved_functions.functions, |(mod_id, func_id, func)| { + let path_resolver = + StandardPathResolver::new(ModuleId { local_id: mod_id, krate: crate_id }); + + let mut resolver = Resolver::new(interner, &path_resolver, def_maps, file_id); + // Must use set_generics here to ensure we re-use the same generics from when + // the impl was originally collected. Otherwise the function will be using different + // TypeVariables for the same generic, causing it to instantiate incorrectly. + resolver.set_generics(impl_generics.clone()); + resolver.set_self_type(self_type.clone()); + + let (hir_func, func_meta, errs) = resolver.resolve_function(func, func_id); + interner.push_fn_meta(func_meta, func_id); + interner.update_fn(func_id, hir_func); + extend_errors(errors, file_id, errs); + (file_id, func_id) + }) } fn type_check_functions( interner: &mut NodeInterner, file_func_ids: Vec<(FileId, FuncId)>, - errors: &mut Vec, + errors: &mut Vec, ) { - file_func_ids - .into_iter() - .map(|(file_id, func_id)| { - let errors = - vecmap(type_check_func(interner, func_id), |error| error.into_diagnostic()); - - CollectedErrors { file_id, errors } - }) - .filter(|collected| !collected.errors.is_empty()) - .for_each(|error| errors.push(error)); + for (file, func) in file_func_ids { + extend_errors(errors, file, type_check_func(interner, func)) + } } 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 256efa148b1..fb0c797d0cb 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,5 @@ use fm::FileId; -use noirc_errors::{CollectedErrors, CustomDiagnostic, DiagnosableError}; +use noirc_errors::FileDiagnostic; use crate::{ graph::CrateId, hir::def_collector::dc_crate::UnresolvedStruct, node_interner::StructId, @@ -29,7 +29,7 @@ pub fn collect_defs( module_id: LocalModuleId, crate_id: CrateId, context: &mut Context, - errors: &mut Vec, + errors: &mut Vec, ) { let mut collector = ModCollector { def_collector, file_id, module_id }; @@ -53,13 +53,9 @@ pub fn collect_defs( collector.collect_structs(ast.types, crate_id, errors); - let errors_in_same_file = collector.collect_functions(context, ast.functions); + collector.collect_functions(context, ast.functions, errors); collector.collect_impls(context, ast.impls); - - if !errors_in_same_file.is_empty() { - errors.push(CollectedErrors { file_id: collector.file_id, errors: errors_in_same_file }); - } } impl<'a> ModCollector<'a> { @@ -67,7 +63,7 @@ impl<'a> ModCollector<'a> { &mut self, context: &mut Context, globals: Vec, - errors: &mut Vec, + errors: &mut Vec, ) { for global in globals { let name = global.pattern.name_ident().clone(); @@ -83,11 +79,7 @@ impl<'a> ModCollector<'a> { if let Err((first_def, second_def)) = result { let err = DefCollectorErrorKind::DuplicateGlobal { first_def, second_def }; - - errors.push(CollectedErrors { - file_id: self.file_id, - errors: vec![err.to_diagnostic()], - }); + errors.push(err.into_file_diagnostic(self.file_id)); } self.def_collector.collected_globals.push(UnresolvedGlobal { @@ -104,15 +96,15 @@ impl<'a> ModCollector<'a> { let mut unresolved_functions = UnresolvedFunctions { file_id: self.file_id, functions: Vec::new() }; - for method in r#impl.methods.iter() { + for method in r#impl.methods { let func_id = context.def_interner.push_empty_fn(); - unresolved_functions.push_fn(self.module_id, func_id, method.clone()); 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.type_path.clone(), self.module_id); + let key = (r#impl.object_type, self.module_id); let methods = self.def_collector.collected_impls.entry(key).or_default(); - methods.push(unresolved_functions); + methods.push((r#impl.generics, r#impl.type_span, unresolved_functions)); } } @@ -120,9 +112,8 @@ impl<'a> ModCollector<'a> { &mut self, context: &mut Context, functions: Vec, - ) -> Vec { - let mut errors = vec![]; - + errors: &mut Vec, + ) { let mut unresolved_functions = UnresolvedFunctions { file_id: self.file_id, functions: Vec::new() }; @@ -148,16 +139,12 @@ impl<'a> ModCollector<'a> { .define_func_def(name, func_id); if let Err((first_def, second_def)) = result { - errors.push( - DefCollectorErrorKind::DuplicateFunction { first_def, second_def } - .to_diagnostic(), - ); + let error = DefCollectorErrorKind::DuplicateFunction { first_def, second_def }; + errors.push(error.into_file_diagnostic(self.file_id)); } } self.def_collector.collected_functions.push(unresolved_functions); - - errors } /// Collect any struct definitions declared within the ast. @@ -166,18 +153,15 @@ impl<'a> ModCollector<'a> { &mut self, types: Vec, krate: CrateId, - errors: &mut Vec, + errors: &mut Vec, ) { for struct_definition in types { let name = struct_definition.name.clone(); // Create the corresponding module for the struct namespace - let id = match self.push_child_module(&name, self.file_id, false) { - Ok(local_id) => StructId(ModuleId { krate, local_id }), - Err(mut more_errors) => { - errors.append(&mut more_errors); - continue; - } + let id = match self.push_child_module(&name, self.file_id, false, errors) { + Some(local_id) => StructId(ModuleId { krate, local_id }), + None => continue, }; // Add the struct to scope so its path can be looked up later @@ -187,11 +171,7 @@ impl<'a> ModCollector<'a> { if let Err((first_def, second_def)) = result { let err = DefCollectorErrorKind::DuplicateFunction { first_def, second_def }; - - errors.push(CollectedErrors { - file_id: self.file_id, - errors: vec![err.to_diagnostic()], - }); + errors.push(err.into_file_diagnostic(self.file_id)); } // And store the TypeId -> StructType mapping somewhere it is reachable @@ -210,20 +190,19 @@ impl<'a> ModCollector<'a> { crate_id: CrateId, submodules: Vec, file_id: FileId, - errors: &mut Vec, + errors: &mut Vec, ) { for submodule in submodules { - match self.push_child_module(&submodule.name, file_id, true) { - Err(mut more_errors) => errors.append(&mut more_errors), - Ok(child_mod_id) => collect_defs( + if let Some(child) = self.push_child_module(&submodule.name, file_id, true, errors) { + collect_defs( self.def_collector, submodule.contents, file_id, - child_mod_id, + child, crate_id, context, errors, - ), + ); } } } @@ -236,7 +215,7 @@ impl<'a> ModCollector<'a> { context: &mut Context, mod_name: &Ident, crate_id: CrateId, - errors: &mut Vec, + errors: &mut Vec, ) { let child_file_id = match context.file_manager.resolve_path(self.file_id, &mod_name.0.contents) { @@ -244,11 +223,7 @@ impl<'a> ModCollector<'a> { Err(_) => { let err = DefCollectorErrorKind::UnresolvedModuleDecl { mod_name: mod_name.clone() }; - - errors.push(CollectedErrors { - file_id: self.file_id, - errors: vec![err.to_diagnostic()], - }); + errors.push(err.into_file_diagnostic(self.file_id)); return; } }; @@ -257,9 +232,8 @@ impl<'a> ModCollector<'a> { let ast = parse_file(&mut context.file_manager, child_file_id, errors); // Add module into def collector and get a ModuleId - match self.push_child_module(mod_name, child_file_id, true) { - Err(mut more_errors) => errors.append(&mut more_errors), - Ok(child_mod_id) => collect_defs( + if let Some(child_mod_id) = self.push_child_module(mod_name, child_file_id, true, errors) { + collect_defs( self.def_collector, ast, child_file_id, @@ -267,16 +241,19 @@ impl<'a> ModCollector<'a> { crate_id, context, errors, - ), + ); } } - pub fn push_child_module( + /// Add a child module to the current def_map. + /// On error this returns None and pushes to `errors` + fn push_child_module( &mut self, mod_name: &Ident, file_id: FileId, add_to_parent_scope: bool, - ) -> Result> { + errors: &mut Vec, + ) -> Option { // Create a new default module let module_id = self.def_collector.def_map.modules.insert(ModuleData::default()); @@ -305,19 +282,16 @@ impl<'a> ModCollector<'a> { krate: self.def_collector.def_map.krate, local_id: LocalModuleId(module_id), }; - modules[self.module_id.0] - .scope - .define_module_def(mod_name.to_owned(), mod_id) - .map_err(|(first_def, second_def)| { - let err = DefCollectorErrorKind::DuplicateModuleDecl { first_def, second_def }; - - vec![CollectedErrors { - file_id: self.file_id, - errors: vec![err.to_diagnostic()], - }] - })?; + + if let Err((first_def, second_def)) = + modules[self.module_id.0].scope.define_module_def(mod_name.to_owned(), mod_id) + { + let err = DefCollectorErrorKind::DuplicateModuleDecl { first_def, second_def }; + errors.push(err.into_file_diagnostic(self.file_id)); + return None; + } } - Ok(LocalModuleId(module_id)) + Some(LocalModuleId(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 93d466077c3..6ee97061005 100644 --- a/crates/noirc_frontend/src/hir/def_collector/errors.rs +++ b/crates/noirc_frontend/src/hir/def_collector/errors.rs @@ -1,7 +1,8 @@ use crate::{hir::resolution::import::ImportDirective, Ident}; use noirc_errors::CustomDiagnostic as Diagnostic; -use noirc_errors::DiagnosableError; +use noirc_errors::FileDiagnostic; +use noirc_errors::Span; use thiserror::Error; #[derive(Error, Debug)] @@ -18,11 +19,19 @@ pub enum DefCollectorErrorKind { UnresolvedModuleDecl { mod_name: Ident }, #[error("unresolved import")] UnresolvedImport { import: ImportDirective }, + #[error("Non-struct type used in impl")] + NonStructTypeInImpl { span: Span }, } -impl DiagnosableError for DefCollectorErrorKind { - fn to_diagnostic(&self) -> Diagnostic { - match self { +impl DefCollectorErrorKind { + pub fn into_file_diagnostic(self, file: fm::FileId) -> FileDiagnostic { + Diagnostic::from(self).in_file(file) + } +} + +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(); @@ -97,6 +106,11 @@ impl DiagnosableError for DefCollectorErrorKind { span, ) } + DefCollectorErrorKind::NonStructTypeInImpl { span } => Diagnostic::simple_error( + "Non-struct type used in impl".into(), + "Only struct types may have implementation methods".into(), + span, + ), } } } diff --git a/crates/noirc_frontend/src/hir/def_map/mod.rs b/crates/noirc_frontend/src/hir/def_map/mod.rs index a1873d58530..4d6eb00b2eb 100644 --- a/crates/noirc_frontend/src/hir/def_map/mod.rs +++ b/crates/noirc_frontend/src/hir/def_map/mod.rs @@ -5,7 +5,7 @@ use crate::node_interner::FuncId; use crate::parser::{parse_program, ParsedModule}; use arena::{Arena, Index}; use fm::{FileId, FileManager}; -use noirc_errors::CollectedErrors; +use noirc_errors::FileDiagnostic; use std::collections::HashMap; mod module_def; @@ -50,7 +50,7 @@ impl CrateDefMap { pub fn collect_defs( crate_id: CrateId, context: &mut Context, - errors: &mut Vec, + errors: &mut Vec, ) { // Check if this Crate has already been compiled // XXX: There is probably a better alternative for this. @@ -118,13 +118,11 @@ impl CrateDefMap { pub fn parse_file( fm: &mut FileManager, file_id: FileId, - all_errors: &mut Vec, + all_errors: &mut Vec, ) -> ParsedModule { let file = fm.fetch_file(file_id); let (program, errors) = parse_program(file.get_source()); - if !errors.is_empty() { - all_errors.push(CollectedErrors { file_id, errors }); - }; + all_errors.extend(errors.into_iter().map(|error| error.in_file(file_id))); program } diff --git a/crates/noirc_frontend/src/hir/resolution/errors.rs b/crates/noirc_frontend/src/hir/resolution/errors.rs index 01fe6f755a9..bf1d1ddfcf4 100644 --- a/crates/noirc_frontend/src/hir/resolution/errors.rs +++ b/crates/noirc_frontend/src/hir/resolution/errors.rs @@ -1,8 +1,8 @@ -use noirc_errors::CustomDiagnostic as Diagnostic; pub use noirc_errors::Span; +use noirc_errors::{CustomDiagnostic as Diagnostic, FileDiagnostic}; use thiserror::Error; -use crate::Ident; +use crate::{Ident, Type}; #[derive(Error, Debug, Clone, PartialEq, Eq)] pub enum ResolverError { @@ -44,14 +44,26 @@ pub enum ResolverError { CapturedMutableVariable { span: Span }, #[error("Test functions are not allowed to have any parameters")] TestFunctionHasParameters { span: Span }, + #[error("Only struct types can be used in constructor expressions")] + NonStructUsedInConstructor { typ: Type, span: Span }, + #[error("Only struct types can have generics")] + NonStructWithGenerics { span: Span }, + #[error("Cannot apply generics on Self type")] + GenericsOnSelfType { span: Span }, } impl ResolverError { + pub fn into_file_diagnostic(self, file: fm::FileId) -> FileDiagnostic { + Diagnostic::from(self).in_file(file) + } +} + +impl From for Diagnostic { /// Only user errors can be transformed into a Diagnostic /// ICEs will make the compiler panic, as they could affect the /// soundness of the generated program - pub fn into_diagnostic(self) -> Diagnostic { - match self { + fn from(error: ResolverError) -> Diagnostic { + match error { ResolverError::DuplicateDefinition { name, first_span, second_span } => { let mut diag = Diagnostic::simple_error( format!("duplicate definitions of {name} found"), @@ -208,6 +220,21 @@ impl ResolverError { "Try removing the parameters or moving the test into a wrapper function".into(), span, ), + ResolverError::NonStructUsedInConstructor { typ, span } => Diagnostic::simple_error( + "Only struct types can be used in constructor expressions".into(), + format!("{} has no fields to construct it with", typ), + span, + ), + ResolverError::NonStructWithGenerics { span } => Diagnostic::simple_error( + "Only struct types can have generic arguments".into(), + "Try removing the generic arguments".into(), + span, + ), + ResolverError::GenericsOnSelfType { span } => Diagnostic::simple_error( + "Cannot apply generics to Self type".into(), + "Use an explicit type name or apply the generics at the start of the impl instead".into(), + span, + ), } } } diff --git a/crates/noirc_frontend/src/hir/resolution/resolver.rs b/crates/noirc_frontend/src/hir/resolution/resolver.rs index 5db43e09edf..fdcf2210148 100644 --- a/crates/noirc_frontend/src/hir/resolution/resolver.rs +++ b/crates/noirc_frontend/src/hir/resolution/resolver.rs @@ -40,7 +40,8 @@ use crate::{ }; use crate::{ ArrayLiteral, Generics, LValue, NoirStruct, Path, Pattern, Shared, StructType, Type, - TypeBinding, TypeVariable, UnresolvedType, UnresolvedTypeExpression, ERROR_IDENT, + TypeBinding, TypeVariable, UnresolvedGenerics, UnresolvedType, UnresolvedTypeExpression, + ERROR_IDENT, }; use fm::FileId; use iter_extended::vecmap; @@ -56,6 +57,8 @@ use crate::hir_def::{ use super::errors::ResolverError; +const SELF_TYPE_NAME: &str = "Self"; + type Scope = GenericScope; type ScopeTree = GenericScopeTree; type ScopeForest = GenericScopeForest; @@ -69,11 +72,13 @@ pub struct Resolver<'a> { file: FileId, /// Set to the current type if we're resolving an impl - self_type: Option, + self_type: Option, - /// Contains a mapping of the current struct's generics to + /// Contains a mapping of the current struct or functions's generics to /// unique type variables if we're resolving a struct. Empty otherwise. - generics: HashMap, (TypeVariable, Span)>, + /// This is a Vec rather than a map to preserve the order a functions generics + /// were declared in. + generics: Vec<(Rc, TypeVariable, Span)>, /// Lambdas share the function scope of the function they're defined in, /// so to identify whether they use any variables from the parent function @@ -96,14 +101,14 @@ impl<'a> Resolver<'a> { scopes: ScopeForest::new(), interner, self_type: None, - generics: HashMap::new(), + generics: Vec::new(), errors: Vec::new(), lambda_index: 0, file, } } - pub fn set_self_type(&mut self, self_type: Option) { + pub fn set_self_type(&mut self, self_type: Option) { self.self_type = self_type; } @@ -130,7 +135,7 @@ impl<'a> Resolver<'a> { // Check whether the function has globals in the local module and add them to the scope self.resolve_local_globals(); - self.add_generics(func.def.generics.clone()); + self.add_generics(&func.def.generics); let (hir_func, func_meta) = self.intern_function(func, func_id); let func_scope_tree = self.scopes.end_function(); @@ -181,6 +186,7 @@ impl<'a> Resolver<'a> { &mut self, name: Ident, mutable: bool, + allow_shadowing: bool, definition: DefinitionKind, ) -> HirIdent { if definition.is_global() { @@ -194,13 +200,17 @@ impl<'a> Resolver<'a> { let scope = self.scopes.get_mut_scope(); let old_value = scope.add_key_value(name.0.contents.clone(), resolver_meta); - if let Some(old_value) = old_value { - self.push_err(ResolverError::DuplicateDefinition { - name: name.0.contents, - first_span: old_value.ident.location.span, - second_span: location.span, - }); + + if !allow_shadowing { + if let Some(old_value) = old_value { + self.push_err(ResolverError::DuplicateDefinition { + name: name.0.contents, + first_span: old_value.ident.location.span, + second_span: location.span, + }); + } } + ident } @@ -313,24 +323,7 @@ impl<'a> Resolver<'a> { UnresolvedType::Unit => Type::Unit, UnresolvedType::Unspecified => Type::Error, UnresolvedType::Error => Type::Error, - UnresolvedType::Named(path, args) => { - // Check if the path is a type variable first. We currently disallow generics on type - // variables since we do not support higher-kinded types. - if args.is_empty() && path.segments.len() == 1 { - let name = &path.last_segment().0.contents; - if let Some((name, (var, _))) = self.generics.get_key_value(name) { - return Type::NamedGeneric(var.clone(), name.clone()); - } - } - - match self.lookup_struct(path) { - Some(definition) => { - let args = vecmap(args, |arg| self.resolve_type_inner(arg, new_variables)); - Type::Struct(definition, args) - } - None => Type::Error, - } - } + UnresolvedType::Named(path, args) => self.resolve_named_type(path, args, new_variables), UnresolvedType::Tuple(fields) => { Type::Tuple(vecmap(fields, |field| self.resolve_type_inner(field, new_variables))) } @@ -342,6 +335,46 @@ impl<'a> Resolver<'a> { } } + fn find_generic(&self, target_name: &str) -> Option<&(Rc, TypeVariable, Span)> { + self.generics.iter().find(|(name, _, _)| name.as_ref() == target_name) + } + + fn resolve_named_type( + &mut self, + path: Path, + args: Vec, + new_variables: &mut Generics, + ) -> Type { + // Check if the path is a type variable first. We currently disallow generics on type + // variables since we do not support higher-kinded types. + if path.segments.len() == 1 { + let name = &path.last_segment().0.contents; + + if args.is_empty() { + if let Some((name, var, _)) = self.find_generic(name) { + return Type::NamedGeneric(var.clone(), name.clone()); + } + } + + if name == SELF_TYPE_NAME { + if let Some(self_type) = self.self_type.clone() { + if !args.is_empty() { + self.push_err(ResolverError::GenericsOnSelfType { span: path.span() }); + } + return self_type; + } + } + } + + match self.lookup_struct_or_error(path) { + Some(struct_type) => { + let args = vecmap(args, |arg| self.resolve_type_inner(arg, new_variables)); + Type::Struct(struct_type, args) + } + None => Type::Error, + } + } + fn resolve_array_size( &mut self, length: Option, @@ -367,7 +400,7 @@ impl<'a> Resolver<'a> { UnresolvedTypeExpression::Variable(path) => { if path.segments.len() == 1 { let name = &path.last_segment().0.contents; - if let Some((name, (var, _))) = self.generics.get_key_value(name) { + if let Some((name, var, _)) = self.find_generic(name) { return Type::NamedGeneric(var.clone(), name.clone()); } } @@ -424,10 +457,28 @@ impl<'a> Resolver<'a> { } /// Translates an UnresolvedType to a Type - fn resolve_type(&mut self, typ: UnresolvedType) -> Type { + pub fn resolve_type(&mut self, typ: UnresolvedType) -> Type { self.resolve_type_inner(typ, &mut vec![]) } + pub fn take_errors(self) -> Vec { + self.errors + } + + /// Return the current generics. + /// Needed to keep referring to the same type variables across many + /// methods in a single impl. + pub fn get_generics(&self) -> &[(Rc, TypeVariable, Span)] { + &self.generics + } + + /// Set the current generics that are in scope. + /// Unlike add_generics, this function will not create any new type variables, + /// opting to reuse the existing ones it is directly given. + pub fn set_generics(&mut self, generics: Vec<(Rc, TypeVariable, Span)>) { + self.generics = generics; + } + /// Translates a (possibly Unspecified) UnresolvedType to a Type. /// Any UnresolvedType::Unspecified encountered are replaced with fresh type variables. fn resolve_inferred_type(&mut self, typ: UnresolvedType) -> Type { @@ -437,7 +488,9 @@ impl<'a> Resolver<'a> { } } - fn add_generics(&mut self, generics: Vec) -> Generics { + /// Add the given generics to scope. + /// Each generic will have a fresh Shared associated with it. + pub fn add_generics(&mut self, generics: &UnresolvedGenerics) -> Generics { vecmap(generics, |generic| { // Map the generic to a fresh type variable let id = self.interner.next_type_variable_id(); @@ -446,14 +499,18 @@ impl<'a> Resolver<'a> { // Check for name collisions of this generic let name = Rc::new(generic.0.contents.clone()); - if let Some(old) = self.generics.insert(name, (typevar.clone(), span)) { + + if let Some((_, _, first_span)) = self.find_generic(&name) { let span = generic.0.span(); self.errors.push(ResolverError::DuplicateDefinition { - name: generic.0.contents, - first_span: old.1, + name: generic.0.contents.clone(), + first_span: *first_span, second_span: span, }) + } else { + self.generics.push((name, typevar.clone(), span)); } + (id, typevar) }) } @@ -462,7 +519,7 @@ impl<'a> Resolver<'a> { mut self, unresolved: NoirStruct, ) -> (Generics, BTreeMap, Vec) { - let generics = self.add_generics(unresolved.generics); + let generics = self.add_generics(&unresolved.generics); // Check whether the struct definition has globals in the local module and add them to the scope self.resolve_local_globals(); @@ -488,6 +545,8 @@ impl<'a> Resolver<'a> { /// Extract metadata from a NoirFunction /// to be used in analysis and intern the function parameters + /// Prerequisite: self.add_generics() has already been called with the given + /// function's generics, including any generics from the impl, if any. fn extract_meta(&mut self, func: &NoirFunction, func_id: FuncId) -> FuncMeta { let location = Location::new(func.name_ident().span(), self.file); let id = self.interner.function_definition_id(func_id); @@ -495,16 +554,10 @@ impl<'a> Resolver<'a> { let attributes = func.attribute().cloned(); - assert_eq!(self.generics.len(), func.def.generics.len()); - let mut generics = vecmap(&func.def.generics, |generic| { - // Always expect self.generics to contain all the generics of this function - let typevar = self.generics.get(&generic.0.contents).unwrap(); - match &*typevar.0.borrow() { - TypeBinding::Unbound(id) => (*id, typevar.0.clone()), - TypeBinding::Bound(binding) => unreachable!( - "Expected {} to be unbound, but it is bound to {}", - generic, binding - ), + let mut generics = vecmap(&self.generics, |(name, typevar, _)| match &*typevar.borrow() { + TypeBinding::Unbound(id) => (*id, typevar.clone()), + TypeBinding::Bound(binding) => { + unreachable!("Expected {} to be unbound, but it is bound to {}", name, binding) } }); @@ -677,8 +730,12 @@ impl<'a> Resolver<'a> { // TODO: For loop variables are currently mutable by default since we haven't // yet implemented syntax for them to be optionally mutable. let (identifier, block_id) = self.in_new_scope(|this| { - let decl = - this.add_variable_decl(identifier, false, DefinitionKind::Local(None)); + let decl = this.add_variable_decl( + identifier, + false, + false, + DefinitionKind::Local(None), + ); (decl, this.resolve_expression(block)) }); @@ -710,21 +767,24 @@ impl<'a> Resolver<'a> { ExpressionKind::Constructor(constructor) => { let span = constructor.type_name.span(); - if let Some(typ) = self.lookup_struct(constructor.type_name) { - let type_id = typ.borrow().id; - - HirExpression::Constructor(HirConstructorExpression { - type_id, - fields: self.resolve_constructor_fields( - type_id, - constructor.fields, - span, - Resolver::resolve_expression, - ), - r#type: typ, - }) - } else { - HirExpression::Error + match self.lookup_type_or_error(constructor.type_name) { + Some(Type::Struct(r#type, struct_generics)) => { + let typ = r#type.clone(); + let fields = constructor.fields; + let resolve_expr = Resolver::resolve_expression; + let fields = + self.resolve_constructor_fields(typ, fields, span, resolve_expr); + HirExpression::Constructor(HirConstructorExpression { + fields, + r#type, + struct_generics, + }) + } + Some(typ) => { + self.push_err(ResolverError::NonStructUsedInConstructor { typ, span }); + HirExpression::Error + } + None => HirExpression::Error, } } ExpressionKind::MemberAccess(access) => { @@ -782,7 +842,7 @@ impl<'a> Resolver<'a> { (Some(_), DefinitionKind::Local(_)) => DefinitionKind::Local(None), (_, other) => other, }; - let id = self.add_variable_decl(name, mutable.is_some(), definition); + let id = self.add_variable_decl(name, mutable.is_some(), false, definition); HirPattern::Identifier(id) } Pattern::Mutable(pattern, span) => { @@ -800,14 +860,33 @@ impl<'a> Resolver<'a> { HirPattern::Tuple(fields, span) } Pattern::Struct(name, fields, span) => { - let struct_id = self.lookup_type(name); - let struct_type = self.get_struct(struct_id); + let error_identifier = |this: &mut Self| { + // Must create a name here to return a HirPattern::Identifier. Allowing + // shadowing here lets us avoid further errors if we define ERROR_IDENT + // multiple times. + let name = ERROR_IDENT.into(); + let identifier = this.add_variable_decl(name, false, true, definition); + HirPattern::Identifier(identifier) + }; + + let (struct_type, generics) = match self.lookup_type_or_error(name) { + Some(Type::Struct(struct_type, generics)) => (struct_type, generics), + None => return error_identifier(self), + Some(typ) => { + self.push_err(ResolverError::NonStructUsedInConstructor { typ, span }); + return error_identifier(self); + } + }; + let resolve_field = |this: &mut Self, pattern| { this.resolve_pattern_mutable(pattern, mutable, definition) }; - let fields = - self.resolve_constructor_fields(struct_id, fields, span, resolve_field); - HirPattern::Struct(struct_type, fields, span) + + let typ = struct_type.clone(); + let fields = self.resolve_constructor_fields(typ, fields, span, resolve_field); + + let typ = Type::Struct(struct_type, generics); + HirPattern::Struct(typ, fields, span) } } } @@ -820,14 +899,14 @@ impl<'a> Resolver<'a> { /// and constructor patterns. fn resolve_constructor_fields( &mut self, - type_id: StructId, + struct_type: Shared, fields: Vec<(Ident, T)>, span: Span, mut resolve_function: impl FnMut(&mut Self, T) -> U, ) -> Vec<(Ident, U)> { let mut ret = Vec::with_capacity(fields.len()); let mut seen_fields = HashSet::new(); - let mut unseen_fields = self.get_field_names_of_type(type_id); + let mut unseen_fields = self.get_field_names_of_type(&struct_type); for (field, expr) in fields { let resolved = resolve_function(self, expr); @@ -842,7 +921,7 @@ impl<'a> Resolver<'a> { // field not required by struct self.push_err(ResolverError::NoSuchField { field: field.clone(), - struct_definition: self.get_struct(type_id).borrow().name.clone(), + struct_definition: struct_type.borrow().name.clone(), }); } @@ -853,7 +932,7 @@ impl<'a> Resolver<'a> { self.push_err(ResolverError::MissingFields { span, missing_fields: unseen_fields.into_iter().map(|field| field.to_string()).collect(), - struct_definition: self.get_struct(type_id).borrow().name.clone(), + struct_definition: struct_type.borrow().name.clone(), }); } @@ -864,10 +943,8 @@ impl<'a> Resolver<'a> { self.interner.get_struct(type_id) } - fn get_field_names_of_type(&self, type_id: StructId) -> BTreeSet { - let typ = self.get_struct(type_id); - let typ = typ.borrow(); - typ.field_names() + fn get_field_names_of_type(&self, typ: &Shared) -> BTreeSet { + typ.borrow().field_names() } fn lookup(&mut self, path: Path) -> Result { @@ -914,32 +991,40 @@ impl<'a> Resolver<'a> { Err(ResolverError::Expected { span, expected, got }) } - fn lookup_type(&mut self, path: Path) -> StructId { + /// Lookup a given struct type by name. + fn lookup_struct_or_error(&mut self, path: Path) -> Option> { + match self.lookup(path) { + Ok(struct_id) => Some(self.get_struct(struct_id)), + Err(error) => { + self.push_err(error); + None + } + } + } + + /// Looks up a given type by name. + /// This will also instantiate any struct types found. + fn lookup_type_or_error(&mut self, path: Path) -> Option { let ident = path.as_ident(); - if ident.map_or(false, |i| i == "Self") { - if let Some(id) = &self.self_type { - return *id; + if ident.map_or(false, |i| i == SELF_TYPE_NAME) { + if let Some(typ) = &self.self_type { + return Some(typ.clone()); } } match self.lookup(path) { - Ok(id) => id, + Ok(struct_id) => { + let struct_type = self.get_struct(struct_id); + let generics = struct_type.borrow().instantiate(self.interner); + Some(Type::Struct(struct_type, generics)) + } Err(error) => { self.push_err(error); - StructId::dummy_id() + None } } } - pub fn lookup_struct(&mut self, path: Path) -> Option> { - let id = self.lookup_type(path); - (id != StructId::dummy_id()).then(|| self.get_struct(id)) - } - - pub fn lookup_type_for_impl(mut self, path: Path) -> (StructId, Vec) { - (self.lookup_type(path), self.errors) - } - fn resolve_path(&mut self, path: Path) -> Result { let span = path.span(); let name = path.as_string(); diff --git a/crates/noirc_frontend/src/hir/type_check/errors.rs b/crates/noirc_frontend/src/hir/type_check/errors.rs index 82783940ad0..33e98e3d6af 100644 --- a/crates/noirc_frontend/src/hir/type_check/errors.rs +++ b/crates/noirc_frontend/src/hir/type_check/errors.rs @@ -35,15 +35,21 @@ pub enum TypeCheckError { } impl TypeCheckError { - pub fn into_diagnostic(self) -> Diagnostic { - match self { + pub fn add_context(self, ctx: &'static str) -> Self { + TypeCheckError::Context { err: Box::new(self), ctx } + } +} + +impl From for Diagnostic { + fn from(error: TypeCheckError) -> Diagnostic { + match error { TypeCheckError::TypeCannotBeUsed { typ, place, span } => Diagnostic::simple_error( format!("The type {} cannot be used in a {}", &typ, place), String::new(), span, ), TypeCheckError::Context { err, ctx } => { - let mut diag = err.into_diagnostic(); + let mut diag = Diagnostic::from(*err); diag.add_note(ctx.to_owned()); diag } @@ -92,8 +98,4 @@ impl TypeCheckError { ), } } - - pub fn add_context(self, ctx: &'static str) -> Self { - TypeCheckError::Context { err: Box::new(self), ctx } - } } diff --git a/crates/noirc_frontend/src/hir/type_check/expr.rs b/crates/noirc_frontend/src/hir/type_check/expr.rs index d88063a9997..4d43a8970d9 100644 --- a/crates/noirc_frontend/src/hir/type_check/expr.rs +++ b/crates/noirc_frontend/src/hir/type_check/expr.rs @@ -216,7 +216,7 @@ pub(crate) fn type_check_expression( } HirExpression::If(if_expr) => check_if_expr(&if_expr, expr_id, interner, errors), HirExpression::Constructor(constructor) => { - check_constructor(&constructor, expr_id, interner, errors) + check_constructor(constructor, expr_id, interner, errors) } HirExpression::MemberAccess(access) => { check_member_access(access, interner, *expr_id, errors) @@ -353,21 +353,16 @@ fn lookup_method( errors: &mut Vec, ) -> Option { match &object_type { - Type::Struct(typ, _args) => { - let typ = typ.borrow(); - match typ.methods.get(method_name) { - Some(method_id) => Some(*method_id), - None => { - errors.push(TypeCheckError::Unstructured { - span: interner.expr_span(expr_id), - msg: format!( - "No method named '{method_name}' found for type '{object_type}'", - ), - }); - None - } + Type::Struct(typ, _args) => match interner.lookup_method(typ.borrow().id, method_name) { + Some(method_id) => Some(method_id), + None => { + errors.push(TypeCheckError::Unstructured { + span: interner.expr_span(expr_id), + msg: format!("No method named '{method_name}' found for type '{object_type}'",), + }); + None } - } + }, // If we fail to resolve the object to a struct type, we have no way of type // checking its arguments as we can't even resolve the name of the function Type::Error => None, @@ -412,8 +407,10 @@ fn type_check_method_call( } let (function_type, instantiation_bindings) = func_meta.typ.instantiate(interner); + interner.store_instantiation_bindings(*function_ident_id, instantiation_bindings); interner.push_expr_type(function_ident_id, function_type.clone()); + bind_function_type(function_type, arguments, span, interner, errors) } } @@ -645,12 +642,13 @@ fn check_if_expr( } fn check_constructor( - constructor: &expr::HirConstructorExpression, + constructor: expr::HirConstructorExpression, expr_id: &ExprId, interner: &mut NodeInterner, errors: &mut Vec, ) -> Type { - let typ = &constructor.r#type; + let typ = constructor.r#type; + let generics = constructor.struct_generics; // Sanity check, this should be caught during name resolution anyway assert_eq!(constructor.fields.len(), typ.borrow().num_fields()); @@ -658,15 +656,14 @@ fn check_constructor( // Sort argument types by name so we can zip with the struct type in the same ordering. // Note that we use a Vec to store the original arguments (rather than a BTreeMap) to // preserve the evaluation order of the source code. - let mut args = constructor.fields.clone(); + let mut args = constructor.fields; args.sort_by_key(|arg| arg.0.clone()); - let typ_ref = typ.borrow(); - let (generics, fields) = typ_ref.instantiate(interner); + let fields = typ.borrow().get_fields(&generics); for ((param_name, param_type), (arg_ident, arg)) in fields.into_iter().zip(args) { // Sanity check to ensure we're matching against the same field - assert_eq!(param_name, &arg_ident.0.contents); + assert_eq!(param_name, arg_ident.0.contents); let arg_type = type_check_expression(interner, &arg, errors); @@ -678,7 +675,7 @@ fn check_constructor( }); } - Type::Struct(typ.clone(), generics) + Type::Struct(typ, generics) } pub fn check_member_access( diff --git a/crates/noirc_frontend/src/hir/type_check/stmt.rs b/crates/noirc_frontend/src/hir/type_check/stmt.rs index ac4c18139dd..f79df00dbec 100644 --- a/crates/noirc_frontend/src/hir/type_check/stmt.rs +++ b/crates/noirc_frontend/src/hir/type_check/stmt.rs @@ -77,27 +77,25 @@ pub fn bind_pattern( }); } }, - HirPattern::Struct(struct_type, fields, span) => match typ { - Type::Struct(inner, args) if &inner == struct_type => { - let mut pattern_fields = fields.clone(); + HirPattern::Struct(struct_type, fields, span) => { + struct_type.unify(&typ, *span, errors, || TypeCheckError::TypeMismatch { + expected_typ: typ.to_string(), + expr_typ: struct_type.to_string(), + expr_span: *span, + }); + if let Type::Struct(struct_type, generics) = struct_type { + let mut pattern_fields = fields.clone(); pattern_fields.sort_by_key(|(ident, _)| ident.clone()); + let struct_type = struct_type.borrow(); - for pattern_field in pattern_fields { + for (field_name, field_pattern) in pattern_fields { let type_field = - inner.borrow().get_field(&pattern_field.0 .0.contents, &args).unwrap().0; - bind_pattern(interner, &pattern_field.1, type_field, errors); + struct_type.get_field(&field_name.0.contents, generics).unwrap().0; + bind_pattern(interner, &field_pattern, type_field, errors); } } - Type::Error => (), - other => { - errors.push(TypeCheckError::TypeMismatch { - expected_typ: other.to_string(), - expr_typ: other.to_string(), - expr_span: *span, - }); - } - }, + } } } diff --git a/crates/noirc_frontend/src/hir_def/expr.rs b/crates/noirc_frontend/src/hir_def/expr.rs index 3cc5db4565d..5ad5a3a6fea 100644 --- a/crates/noirc_frontend/src/hir_def/expr.rs +++ b/crates/noirc_frontend/src/hir_def/expr.rs @@ -2,7 +2,7 @@ use acvm::FieldElement; use fm::FileId; use noirc_errors::Location; -use crate::node_interner::{DefinitionId, ExprId, FuncId, NodeInterner, StmtId, StructId}; +use crate::node_interner::{DefinitionId, ExprId, FuncId, NodeInterner, StmtId}; use crate::{BinaryOp, BinaryOpKind, Ident, Shared, UnaryOp}; use super::stmt::HirPattern; @@ -149,8 +149,8 @@ impl HirMethodCallExpression { #[derive(Debug, Clone)] pub struct HirConstructorExpression { - pub type_id: StructId, pub r#type: Shared, + pub struct_generics: Vec, // NOTE: It is tempting to make this a BTreeSet to force ordering of field // names (and thus remove the need to normalize them during type checking) diff --git a/crates/noirc_frontend/src/hir_def/stmt.rs b/crates/noirc_frontend/src/hir_def/stmt.rs index 986d417027f..278126d224d 100644 --- a/crates/noirc_frontend/src/hir_def/stmt.rs +++ b/crates/noirc_frontend/src/hir_def/stmt.rs @@ -1,6 +1,6 @@ use super::expr::HirIdent; use crate::node_interner::ExprId; -use crate::{Ident, Shared, StructType, Type}; +use crate::{Ident, Type}; use fm::FileId; use noirc_errors::Span; @@ -51,7 +51,7 @@ pub enum HirPattern { Identifier(HirIdent), Mutable(Box, Span), Tuple(Vec, Span), - Struct(Shared, Vec<(Ident, HirPattern)>, Span), + Struct(Type, Vec<(Ident, HirPattern)>, Span), } impl HirPattern { diff --git a/crates/noirc_frontend/src/hir_def/types.rs b/crates/noirc_frontend/src/hir_def/types.rs index b99cb917df4..10b70ad2db1 100644 --- a/crates/noirc_frontend/src/hir_def/types.rs +++ b/crates/noirc_frontend/src/hir_def/types.rs @@ -9,10 +9,7 @@ use iter_extended::{btree_map, vecmap}; use noirc_abi::AbiType; use noirc_errors::Span; -use crate::{ - node_interner::{FuncId, StructId}, - Ident, Signedness, -}; +use crate::{node_interner::StructId, Ident, Signedness}; /// A shared, mutable reference to some T. /// Wrapper is required for Hash impl of RefCell. @@ -75,7 +72,6 @@ pub struct StructType { fields: BTreeMap, pub generics: Generics, - pub methods: HashMap, pub span: Span, } @@ -101,7 +97,7 @@ impl StructType { fields: BTreeMap, generics: Generics, ) -> StructType { - StructType { id, fields, name, span, generics, methods: HashMap::new() } + StructType { id, fields, name, span, generics } } pub fn set_fields(&mut self, fields: BTreeMap) { @@ -155,30 +151,9 @@ impl StructType { } /// Instantiate this struct type, returning a Vec of the new generic args (in - /// the same order as self.generics) and a map of each instantiated field - pub fn instantiate<'a>( - &'a self, - interner: &mut NodeInterner, - ) -> (Vec, BTreeMap<&'a str, Type>) { - let (generics, substitutions) = self - .generics - .iter() - .map(|(old_id, old_var)| { - let new = interner.next_type_variable(); - (new.clone(), (*old_id, (old_var.clone(), new))) - }) - .unzip(); - - let fields = self - .fields - .iter() - .map(|(name, typ)| { - let typ = typ.substitute(&substitutions); - (name.0.contents.as_str(), typ) - }) - .collect(); - - (generics, fields) + /// the same order as self.generics) + pub fn instantiate(&self, interner: &mut NodeInterner) -> Vec { + vecmap(&self.generics, |_| interner.next_type_variable()) } } diff --git a/crates/noirc_frontend/src/lexer/errors.rs b/crates/noirc_frontend/src/lexer/errors.rs index 86398de3f03..189bfdb4bb2 100644 --- a/crates/noirc_frontend/src/lexer/errors.rs +++ b/crates/noirc_frontend/src/lexer/errors.rs @@ -2,7 +2,7 @@ use crate::token::SpannedToken; use super::token::Token; use noirc_errors::CustomDiagnostic as Diagnostic; -use noirc_errors::{DiagnosableError, Span}; +use noirc_errors::Span; use thiserror::Error; #[derive(Error, Clone, Debug, PartialEq, Eq)] @@ -77,9 +77,9 @@ impl LexerErrorKind { } } -impl DiagnosableError for LexerErrorKind { - fn to_diagnostic(&self) -> Diagnostic { - let (primary, secondary, span) = self.parts(); +impl From for Diagnostic { + fn from(error: LexerErrorKind) -> Diagnostic { + let (primary, secondary, span) = error.parts(); Diagnostic::simple_error(primary, secondary, span) } } diff --git a/crates/noirc_frontend/src/node_interner.rs b/crates/noirc_frontend/src/node_interner.rs index 16bfa93332c..b1287034246 100644 --- a/crates/noirc_frontend/src/node_interner.rs +++ b/crates/noirc_frontend/src/node_interner.rs @@ -169,6 +169,11 @@ pub struct NodeInterner { language: Language, delayed_type_checks: Vec, + + // A map from a struct type and method name to a function id for the method + // along with any generic on the struct it may require. E.g. if the impl is + // only for `impl Foo` rather than all Foo, the generics will be `vec![String]`. + struct_methods: HashMap<(StructId, String), (Vec, FuncId)>, } type TypeCheckFn = Box Result<(), TypeCheckError>>; @@ -236,6 +241,7 @@ impl Default for NodeInterner { globals: HashMap::new(), language: Language::R1CS, delayed_type_checks: vec![], + struct_methods: HashMap::new(), }; // An empty block expression is used often, we add this into the `node` on startup @@ -576,4 +582,27 @@ impl NodeInterner { is_test.then_some(*id) }) } + + /// Add a method to a type. + /// This will panic for non-struct types currently as we do not support methods + /// for primitives. We could allow this in the future however. + pub fn add_method( + &mut self, + self_type: &Type, + method_name: String, + method_id: FuncId, + ) -> Option { + match self_type { + Type::Struct(struct_type, generics) => { + let key = (struct_type.borrow().id, method_name); + self.struct_methods.insert(key, (generics.clone(), method_id)).map(|(_, id)| id) + } + other => unreachable!("Tried adding method to non-struct type '{}'", other), + } + } + + /// Search by name for a method on the given struct + pub fn lookup_method(&self, id: StructId, method_name: &str) -> Option { + self.struct_methods.get(&(id, method_name.to_owned())).map(|(_, id)| *id) + } } diff --git a/crates/noirc_frontend/src/parser/errors.rs b/crates/noirc_frontend/src/parser/errors.rs index 9dd73e28531..7f19ef7f062 100644 --- a/crates/noirc_frontend/src/parser/errors.rs +++ b/crates/noirc_frontend/src/parser/errors.rs @@ -5,7 +5,6 @@ use crate::BinaryOp; use iter_extended::vecmap; use noirc_errors::CustomDiagnostic as Diagnostic; -use noirc_errors::DiagnosableError; use noirc_errors::Span; #[derive(Debug, Clone, PartialEq, Eq)] @@ -82,13 +81,13 @@ impl std::fmt::Display for ParserError { } } -impl DiagnosableError for ParserError { - fn to_diagnostic(&self) -> Diagnostic { - match &self.reason { - Some(reason) => Diagnostic::simple_error(reason.clone(), String::new(), self.span), +impl From for Diagnostic { + fn from(error: ParserError) -> Diagnostic { + match &error.reason { + Some(reason) => Diagnostic::simple_error(reason.clone(), String::new(), error.span), None => { - let primary = self.to_string(); - Diagnostic::simple_error(primary, String::new(), self.span) + let primary = error.to_string(); + Diagnostic::simple_error(primary, String::new(), error.span) } } } diff --git a/crates/noirc_frontend/src/parser/parser.rs b/crates/noirc_frontend/src/parser/parser.rs index 686220c523c..56e21f59f3e 100644 --- a/crates/noirc_frontend/src/parser/parser.rs +++ b/crates/noirc_frontend/src/parser/parser.rs @@ -16,15 +16,15 @@ use crate::{ use chumsky::prelude::*; use iter_extended::vecmap; use noirc_abi::AbiVisibility; -use noirc_errors::{CustomDiagnostic, DiagnosableError, Span, Spanned}; +use noirc_errors::{CustomDiagnostic, Span, Spanned}; pub fn parse_program(source_program: &str) -> (ParsedModule, Vec) { let lexer = Lexer::new(source_program); let (tokens, lexing_errors) = lexer.lex(); - let mut errors = vecmap(&lexing_errors, DiagnosableError::to_diagnostic); + let mut errors = vecmap(lexing_errors, Into::into); let (module, parsing_errors) = program().parse_recovery_verbose(tokens); - errors.extend(parsing_errors.iter().map(DiagnosableError::to_diagnostic)); + errors.extend(parsing_errors.into_iter().map(Into::into)); (module.unwrap(), errors) } @@ -227,11 +227,14 @@ fn self_parameter() -> impl NoirParser<(Pattern, UnresolvedType, AbiVisibility)> fn implementation() -> impl NoirParser { keyword(Keyword::Impl) - .ignore_then(path()) + .ignore_then(generics()) + .then(parse_type().map_with_span(|typ, span| (typ, span))) .then_ignore(just(Token::LeftBrace)) .then(function_definition(true).repeated()) .then_ignore(just(Token::RightBrace)) - .map(|(type_path, methods)| TopLevelStatement::Impl(NoirImpl { type_path, methods })) + .map(|((generics, (object_type, type_span)), methods)| { + TopLevelStatement::Impl(NoirImpl { generics, object_type, type_span, methods }) + }) } fn block_expr<'a, P>(expr_parser: P) -> impl NoirParser + 'a @@ -898,7 +901,7 @@ fn literal() -> impl NoirParser { #[cfg(test)] mod test { - use noirc_errors::{CustomDiagnostic, DiagnosableError}; + use noirc_errors::CustomDiagnostic; use super::*; use crate::{ArrayLiteral, Literal}; @@ -910,12 +913,12 @@ mod test { let lexer = Lexer::new(program); let (tokens, lexer_errors) = lexer.lex(); if !lexer_errors.is_empty() { - return Err(vecmap(&lexer_errors, DiagnosableError::to_diagnostic)); + return Err(vecmap(lexer_errors, Into::into)); } parser .then_ignore(just(Token::EOF)) .parse(tokens) - .map_err(|errors| vecmap(&errors, DiagnosableError::to_diagnostic)) + .map_err(|errors| vecmap(errors, Into::into)) } fn parse_recover(parser: P, program: &str) -> (Option, Vec) @@ -926,8 +929,8 @@ mod test { let (tokens, lexer_errors) = lexer.lex(); let (opt, errs) = parser.then_ignore(force(just(Token::EOF))).parse_recovery(tokens); - let mut errors = vecmap(&lexer_errors, DiagnosableError::to_diagnostic); - errors.extend(errs.iter().map(DiagnosableError::to_diagnostic)); + let mut errors = vecmap(lexer_errors, Into::into); + errors.extend(errs.into_iter().map(Into::into)); (opt, errors) }