Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(lsp): replace panics with errors #4209

Merged
merged 10 commits into from
Jan 31, 2024
86 changes: 65 additions & 21 deletions aztec_macros/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
}

Expand All @@ -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<AztecMacroError> for MacroError {
Expand Down Expand Up @@ -71,6 +76,11 @@ impl From<AztecMacroError> for MacroError {
secondary_message: None,
span: Some(span),
},
AztecMacroError::EventError { span, message } => MacroError {
primary_message: message,
secondary_message: None,
span: Some(span),
},
}
}
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -472,19 +485,30 @@ fn collect_crate_structs(crate_id: &CrateId, context: &HirContext) -> Vec<Struct
}

/// Substitutes the signature literal that was introduced in the selector method previously with the actual signature.
fn transform_event(struct_id: StructId, interner: &mut NodeInterner) {
fn transform_event(
struct_id: StructId,
interner: &mut NodeInterner,
) -> 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_else(|| {
let error = AztecMacroError::EventError {
span: struct_type.borrow().location.span,
message: "Selector method not found".to_owned(),
};
(error, 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"),
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 {
Expand All @@ -494,12 +518,21 @@ fn transform_event(struct_id: StructId, interner: &mut NodeInterner) {
},
_ => None,
}
.expect("Compute selector statement is not a call expression");

let first_arg_id = compute_selector_expression
.arguments
.first()
.expect("Missing argument for compute selector");
.ok_or_else(|| {
let error = AztecMacroError::EventError {
span: struct_type.borrow().location.span,
message: "Compute selector statement is not a call expression".to_owned(),
};
(error, struct_type.borrow().location.file)
})?;

let first_arg_id = compute_selector_expression.arguments.first().ok_or_else(|| {
let error = AztecMacroError::EventError {
span: struct_type.borrow().location.span,
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))
Expand All @@ -518,18 +551,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";
Expand Down
14 changes: 12 additions & 2 deletions compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
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};
Expand Down Expand Up @@ -155,6 +155,12 @@
}
}

impl From<MacroError> for CompilationError {
fn from(value: MacroError) -> Self {
CompilationError::DefinitionError(DefCollectorErrorKind::MacroError(value))
}
}

impl From<ParserError> for CompilationError {
fn from(value: ParserError) -> Self {
CompilationError::ParseError(value)
Expand Down Expand Up @@ -359,7 +365,11 @@
errors.extend(resolved_globals.errors);

for macro_processor in macro_processors {
macro_processor.process_typed_ast(&crate_id, context);
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));

Expand Down Expand Up @@ -456,7 +466,7 @@
.collect()
}

// TODO(vitkov): Move this out of here and into type_check

Check warning on line 469 in compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (vitkov)
#[allow(clippy::too_many_arguments)]
pub(crate) fn check_methods_signatures(
resolver: &mut Resolver,
Expand Down
6 changes: 5 additions & 1 deletion compiler/noirc_frontend/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,10 @@ pub mod macros_api {
) -> Result<SortedModule, (MacroError, FileId)>;
/// 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)>;
}
}
Loading