From 1af3374e1678b588fbb7fdd9a8d30fc7142b4d5f Mon Sep 17 00:00:00 2001 From: Koby Date: Tue, 30 Jan 2024 17:31:41 +0100 Subject: [PATCH 1/7] fix(lsp): replace panics with errors --- aztec_macros/src/lib.rs | 87 ++++++++++++++++++++++-------- compiler/noirc_frontend/src/lib.rs | 6 ++- 2 files changed, 70 insertions(+), 23 deletions(-) diff --git a/aztec_macros/src/lib.rs b/aztec_macros/src/lib.rs index c9adece4eb5..6e7602358a6 100644 --- a/aztec_macros/src/lib.rs +++ b/aztec_macros/src/lib.rs @@ -26,8 +26,12 @@ impl MacroProcessor for AztecMacro { transform(ast, crate_id, context) } - fn process_typed_ast(&self, crate_id: &CrateId, context: &mut HirContext) { - transform_hir(crate_id, context) + fn process_typed_ast( + &self, + crate_id: &CrateId, + context: &mut HirContext, + ) -> Result<(), (MacroError, FileId)> { + transform_hir(crate_id, context).map_err(|(err, file_id)| (err.into(), file_id)) } } @@ -41,6 +45,7 @@ pub enum AztecMacroError { ContractHasTooManyFunctions { span: Span }, ContractConstructorMissing { span: Span }, UnsupportedFunctionArgumentType { span: Span, typ: UnresolvedTypeData }, + EventError { span: Span, message: String }, } impl From for MacroError { @@ -71,6 +76,11 @@ impl From for MacroError { secondary_message: None, span: Some(span), }, + AztecMacroError::EventError { span, message } => MacroError { + primary_message: message, + secondary_message: None, + span: Some(span), + }, } } } @@ -237,8 +247,11 @@ fn transform( // /// Completes the Hir with data gathered from type resolution -fn transform_hir(crate_id: &CrateId, context: &mut HirContext) { - transform_events(crate_id, context); +fn transform_hir( + crate_id: &CrateId, + context: &mut HirContext, +) -> Result<(), (AztecMacroError, FileId)> { + transform_events(crate_id, context) } /// Includes an import to the aztec library if it has not been included yet @@ -472,20 +485,30 @@ fn collect_crate_structs(crate_id: &CrateId, context: &HirContext) -> Vec Result<(), (AztecMacroError, FileId)> { let struct_type = interner.get_struct(struct_id); let selector_id = interner - .lookup_method(&Type::Struct(struct_type, vec![]), struct_id, "selector", false) - .expect("Selector method not found"); + .lookup_method(&Type::Struct(struct_type.clone(), vec![]), struct_id, "selector", false) + .ok_or(( + AztecMacroError::EventError { + span: struct_type.borrow().location.span, + message: "Selector method not found".to_owned(), + }, + struct_type.borrow().location.file, + ))?; let selector_function = interner.function(&selector_id); - let compute_selector_statement = interner.statement( - selector_function - .block(interner) - .statements() - .first() - .expect("Compute selector statement not found"), - ); + let compute_selector_statement = + interner.statement(selector_function.block(interner).statements().first().ok_or(( + AztecMacroError::EventError { + span: struct_type.borrow().location.span, + message: "Compute selector statement not found".to_owned(), + }, + struct_type.borrow().location.file, + ))?); let compute_selector_expression = match compute_selector_statement { HirStatement::Expression(expression_id) => match interner.expression(&expression_id) { @@ -494,12 +517,21 @@ fn transform_event(struct_id: StructId, interner: &mut NodeInterner) { }, _ => None, } - .expect("Compute selector statement is not a call expression"); + .ok_or(( + AztecMacroError::EventError { + span: struct_type.borrow().location.span, + message: "Compute selector statement is not a call expression".to_owned(), + }, + struct_type.borrow().location.file, + ))?; - let first_arg_id = compute_selector_expression - .arguments - .first() - .expect("Missing argument for compute selector"); + let first_arg_id = compute_selector_expression.arguments.first().ok_or(( + AztecMacroError::EventError { + span: struct_type.borrow().location.span, + message: "Missing argument for compute selector".to_owned(), + }, + struct_type.borrow().location.file, + ))?; match interner.expression(first_arg_id) { HirExpression::Literal(HirLiteral::Str(signature)) @@ -518,18 +550,29 @@ fn transform_event(struct_id: StructId, interner: &mut NodeInterner) { selector_literal_id, Type::String(Box::new(Type::Constant(signature.len() as u64))), ); + Ok(()) } - _ => unreachable!("Signature placeholder literal does not match"), + _ => Err(( + AztecMacroError::EventError { + span: struct_type.borrow().location.span, + message: "Signature placeholder literal does not match".to_owned(), + }, + struct_type.borrow().location.file, + )), } } -fn transform_events(crate_id: &CrateId, context: &mut HirContext) { +fn transform_events( + crate_id: &CrateId, + context: &mut HirContext, +) -> Result<(), (AztecMacroError, FileId)> { for struct_id in collect_crate_structs(crate_id, context) { let attributes = context.def_interner.struct_attributes(&struct_id); if attributes.iter().any(|attr| matches!(attr, SecondaryAttribute::Event)) { - transform_event(struct_id, &mut context.def_interner); + transform_event(struct_id, &mut context.def_interner)?; } } + Ok(()) } const SIGNATURE_PLACEHOLDER: &str = "SIGNATURE_PLACEHOLDER"; diff --git a/compiler/noirc_frontend/src/lib.rs b/compiler/noirc_frontend/src/lib.rs index 9582b80dcba..b6d4c568334 100644 --- a/compiler/noirc_frontend/src/lib.rs +++ b/compiler/noirc_frontend/src/lib.rs @@ -75,6 +75,10 @@ pub mod macros_api { ) -> Result; /// Function to manipulate the AST after type checking has been completed. /// The AST after type checking has been done is called the HIR. - fn process_typed_ast(&self, crate_id: &CrateId, context: &mut HirContext); + fn process_typed_ast( + &self, + crate_id: &CrateId, + context: &mut HirContext, + ) -> Result<(), (MacroError, FileId)>; } } From 10225c81231fb986100f702f3398e428516e1d1c Mon Sep 17 00:00:00 2001 From: Koby Date: Tue, 30 Jan 2024 18:05:11 +0100 Subject: [PATCH 2/7] chore: errors from macros collected --- .../src/hir/def_collector/dc_crate.rs | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs index a6ab6b1d825..4bf47f1ccf9 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs @@ -14,7 +14,7 @@ use crate::hir::resolution::{ use crate::hir::type_check::{type_check_func, TypeCheckError, TypeChecker}; use crate::hir::Context; -use crate::macros_api::MacroProcessor; +use crate::macros_api::{MacroError, MacroProcessor}; use crate::node_interner::{FuncId, NodeInterner, StmtId, StructId, TraitId, TypeAliasId}; use crate::parser::{ParserError, SortedModule}; @@ -155,6 +155,12 @@ impl From for CustomDiagnostic { } } +impl From for CompilationError { + fn from(value: MacroError) -> Self { + CompilationError::DefinitionError(DefCollectorErrorKind::MacroError(value)) + } +} + impl From for CompilationError { fn from(value: ParserError) -> Self { CompilationError::ParseError(value) @@ -359,7 +365,11 @@ impl DefCollector { errors.extend(resolved_globals.errors); for macro_processor in macro_processors { - macro_processor.process_typed_ast(&crate_id, context); + let _ = macro_processor.process_typed_ast(&crate_id, context).unwrap_or_else( + |(macro_err, file_id)| { + errors.push((macro_err.into(), file_id)); + }, + ); } errors.extend(type_check_globals(&mut context.def_interner, resolved_globals.globals)); From 6f9d4ba585f9aaf314c08aac80349f1bc39accce Mon Sep 17 00:00:00 2001 From: Koby Date: Tue, 30 Jan 2024 18:19:10 +0100 Subject: [PATCH 3/7] chore: clippy --- compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs index 4bf47f1ccf9..f7441750fc8 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs @@ -365,7 +365,7 @@ impl DefCollector { errors.extend(resolved_globals.errors); for macro_processor in macro_processors { - let _ = macro_processor.process_typed_ast(&crate_id, context).unwrap_or_else( + macro_processor.process_typed_ast(&crate_id, context).unwrap_or_else( |(macro_err, file_id)| { errors.push((macro_err.into(), file_id)); }, From cc254e4a051289e9f50ceb6ee3ec2b9dcc684261 Mon Sep 17 00:00:00 2001 From: Koby Hall <102518238+kobyhallx@users.noreply.github.com> Date: Tue, 30 Jan 2024 20:55:38 +0100 Subject: [PATCH 4/7] Update aztec_macros/src/lib.rs Co-authored-by: jfecher --- aztec_macros/src/lib.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/aztec_macros/src/lib.rs b/aztec_macros/src/lib.rs index 6e7602358a6..2e1960b818e 100644 --- a/aztec_macros/src/lib.rs +++ b/aztec_macros/src/lib.rs @@ -492,13 +492,13 @@ fn transform_event( let struct_type = interner.get_struct(struct_id); let selector_id = interner .lookup_method(&Type::Struct(struct_type.clone(), vec![]), struct_id, "selector", false) - .ok_or(( - AztecMacroError::EventError { + .ok_or_else(|| { + let error = AztecMacroError::EventError { span: struct_type.borrow().location.span, message: "Selector method not found".to_owned(), - }, - struct_type.borrow().location.file, - ))?; + }; + (error, struct_type.borrow().location.file) + })?; let selector_function = interner.function(&selector_id); let compute_selector_statement = From 11908653569a2d5e9085d800dccd3e95112bfef8 Mon Sep 17 00:00:00 2001 From: Koby Date: Tue, 30 Jan 2024 21:00:08 +0100 Subject: [PATCH 5/7] chore: turn to ok_or_else --- aztec_macros/src/lib.rs | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/aztec_macros/src/lib.rs b/aztec_macros/src/lib.rs index 2e1960b818e..c58046935cf 100644 --- a/aztec_macros/src/lib.rs +++ b/aztec_macros/src/lib.rs @@ -517,21 +517,21 @@ fn transform_event( }, _ => None, } - .ok_or(( - AztecMacroError::EventError { + .ok_or_else(|| { + let error = AztecMacroError::EventError { span: struct_type.borrow().location.span, message: "Compute selector statement is not a call expression".to_owned(), - }, - struct_type.borrow().location.file, - ))?; + }; + (error, struct_type.borrow().location.file) + })?; - let first_arg_id = compute_selector_expression.arguments.first().ok_or(( - AztecMacroError::EventError { + let first_arg_id = compute_selector_expression.arguments.first().ok_or_else(|| { + let error = AztecMacroError::EventError { span: struct_type.borrow().location.span, - message: "Missing argument for compute selector".to_owned(), - }, - struct_type.borrow().location.file, - ))?; + message: "Compute selector statement is not a call expression".to_owned(), + }; + (error, struct_type.borrow().location.file) + })?; match interner.expression(first_arg_id) { HirExpression::Literal(HirLiteral::Str(signature)) From 85ccddefb7caa232ec06e7fb2f781808a8280c80 Mon Sep 17 00:00:00 2001 From: jfecher Date: Tue, 30 Jan 2024 14:42:16 -0600 Subject: [PATCH 6/7] Update aztec_macros/src/lib.rs --- aztec_macros/src/lib.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/aztec_macros/src/lib.rs b/aztec_macros/src/lib.rs index c58046935cf..4cd40ff04d6 100644 --- a/aztec_macros/src/lib.rs +++ b/aztec_macros/src/lib.rs @@ -502,13 +502,13 @@ fn transform_event( let selector_function = interner.function(&selector_id); let compute_selector_statement = - interner.statement(selector_function.block(interner).statements().first().ok_or(( - AztecMacroError::EventError { + interner.statement(selector_function.block(interner).statements().first().ok_or_else(|| { + let error = AztecMacroError::EventError { span: struct_type.borrow().location.span, message: "Compute selector statement not found".to_owned(), - }, - struct_type.borrow().location.file, - ))?); + }; + (error, struct_type.borrow().location.file) + })?); let compute_selector_expression = match compute_selector_statement { HirStatement::Expression(expression_id) => match interner.expression(&expression_id) { From 170457f85e526de040615d2a56775a6ee8acec90 Mon Sep 17 00:00:00 2001 From: Koby Date: Wed, 31 Jan 2024 08:40:16 +0100 Subject: [PATCH 7/7] chore: fmt --- aztec_macros/src/lib.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/aztec_macros/src/lib.rs b/aztec_macros/src/lib.rs index 4cd40ff04d6..baabd9aa1dc 100644 --- a/aztec_macros/src/lib.rs +++ b/aztec_macros/src/lib.rs @@ -501,14 +501,15 @@ fn transform_event( })?; let selector_function = interner.function(&selector_id); - let compute_selector_statement = - interner.statement(selector_function.block(interner).statements().first().ok_or_else(|| { + let compute_selector_statement = interner.statement( + selector_function.block(interner).statements().first().ok_or_else(|| { let error = AztecMacroError::EventError { span: struct_type.borrow().location.span, message: "Compute selector statement not found".to_owned(), }; (error, struct_type.borrow().location.file) - })?); + })?, + ); let compute_selector_expression = match compute_selector_statement { HirStatement::Expression(expression_id) => match interner.expression(&expression_id) {