Skip to content

Commit

Permalink
fix: remove panic when generic array length is not resolvable (#4408)
Browse files Browse the repository at this point in the history
# Description

## Problem\*

Resolves #4407 

## Summary\*

We currently have no way to gracefully error during monomorphization and
so must panic if we run into any errors.

This PR then adds the `MonomorphizationError` enum with an example error
type. We've also added a `CompileError` which unifies `RuntimeError` and
`MonomorphizationError` so they can be converted into `FileDiagnostic`s


## Additional Context



## Documentation\*

Check one:
- [x] No documentation needed.
- [ ] Documentation included in this PR.
- [ ] **[Exceptional Case]** Documentation to be submitted in a separate
PR.

# PR Checklist\*

- [x] I have tested the changes locally.
- [x] I have formatted the changes with [Prettier](https://prettier.io/)
and/or `cargo fmt` on default settings.
  • Loading branch information
TomAFrench committed Feb 26, 2024
1 parent 15c5618 commit 00ab3db
Show file tree
Hide file tree
Showing 8 changed files with 246 additions and 121 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions compiler/noirc_driver/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ serde.workspace = true
fxhash.workspace = true
rust-embed.workspace = true
tracing.workspace = true
thiserror.workspace = true

aztec_macros = { path = "../../aztec_macros" }
noirc_macros = { path = "../../noirc_macros" }
27 changes: 23 additions & 4 deletions compiler/noirc_driver/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,10 @@ use noirc_frontend::graph::{CrateId, CrateName};
use noirc_frontend::hir::def_map::{Contract, CrateDefMap};
use noirc_frontend::hir::Context;
use noirc_frontend::macros_api::MacroProcessor;
use noirc_frontend::monomorphization::{monomorphize, monomorphize_debug};
use noirc_frontend::monomorphization::{monomorphize, monomorphize_debug, MonomorphizationError};
use noirc_frontend::node_interner::FuncId;
use std::path::Path;
use thiserror::Error;
use tracing::info;

mod abi_gen;
Expand Down Expand Up @@ -107,6 +108,24 @@ fn parse_expression_width(input: &str) -> Result<ExpressionWidth, std::io::Error
}
}

#[derive(Debug, Error)]
pub enum CompileError {
#[error(transparent)]
MonomorphizationError(#[from] MonomorphizationError),

#[error(transparent)]
RuntimeError(#[from] RuntimeError),
}

impl From<CompileError> for FileDiagnostic {
fn from(error: CompileError) -> FileDiagnostic {
match error {
CompileError::RuntimeError(err) => err.into(),
CompileError::MonomorphizationError(err) => err.into(),
}
}
}

/// Helper type used to signify where only warnings are expected in file diagnostics
pub type Warnings = Vec<FileDiagnostic>;

Expand Down Expand Up @@ -436,11 +455,11 @@ pub fn compile_no_check(
main_function: FuncId,
cached_program: Option<CompiledProgram>,
force_compile: bool,
) -> Result<CompiledProgram, RuntimeError> {
) -> Result<CompiledProgram, CompileError> {
let program = if options.instrument_debug {
monomorphize_debug(main_function, &mut context.def_interner, &context.debug_instrumenter)
monomorphize_debug(main_function, &mut context.def_interner, &context.debug_instrumenter)?
} else {
monomorphize(main_function, &mut context.def_interner)
monomorphize(main_function, &mut context.def_interner)?
};

let hash = fxhash::hash64(&program);
Expand Down
2 changes: 1 addition & 1 deletion compiler/noirc_evaluator/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ impl RuntimeError {
RuntimeError::InternalError(cause) => {
Diagnostic::simple_error(
"Internal Consistency Evaluators Errors: \n
This is likely a bug. Consider Opening an issue at https://github.com/noir-lang/noir/issues".to_owned(),
This is likely a bug. Consider opening an issue at https://github.com/noir-lang/noir/issues".to_owned(),
cause.to_string(),
noirc_errors::Span::inclusive(0, 0)
)
Expand Down
38 changes: 25 additions & 13 deletions compiler/noirc_frontend/src/monomorphization/debug.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use crate::hir_def::expr::*;
use crate::node_interner::ExprId;

use super::ast::{Expression, Ident};
use super::Monomorphizer;
use super::{MonomorphizationError, Monomorphizer};

const DEBUG_MEMBER_ASSIGN_PREFIX: &str = "__debug_member_assign_";
const DEBUG_VAR_ID_ARG_SLOT: usize = 0;
Expand Down Expand Up @@ -39,18 +39,19 @@ impl<'interner> Monomorphizer<'interner> {
&mut self,
call: &HirCallExpression,
arguments: &mut [Expression],
) {
let original_func = Box::new(self.expr(call.func));
) -> Result<(), MonomorphizationError> {
let original_func = Box::new(self.expr(call.func)?);
if let Expression::Ident(Ident { name, .. }) = original_func.as_ref() {
if name == "__debug_var_assign" {
self.patch_debug_var_assign(call, arguments);
self.patch_debug_var_assign(call, arguments)?;
} else if name == "__debug_var_drop" {
self.patch_debug_var_drop(call, arguments);
self.patch_debug_var_drop(call, arguments)?;
} else if let Some(arity) = name.strip_prefix(DEBUG_MEMBER_ASSIGN_PREFIX) {
let arity = arity.parse::<usize>().expect("failed to parse member assign arity");
self.patch_debug_member_assign(call, arguments, arity);
self.patch_debug_member_assign(call, arguments, arity)?;
}
}
Ok(())
}

/// Update instrumentation code inserted on variable assignment. We need to
Expand All @@ -59,7 +60,11 @@ impl<'interner> Monomorphizer<'interner> {
/// variable are possible if using generic functions, hence the temporary ID
/// created when injecting the instrumentation code can map to multiple IDs
/// at runtime.
fn patch_debug_var_assign(&mut self, call: &HirCallExpression, arguments: &mut [Expression]) {
fn patch_debug_var_assign(
&mut self,
call: &HirCallExpression,
arguments: &mut [Expression],
) -> Result<(), MonomorphizationError> {
let hir_arguments = vecmap(&call.arguments, |id| self.interner.expression(id));
let var_id_arg = hir_arguments.get(DEBUG_VAR_ID_ARG_SLOT);
let Some(HirExpression::Literal(HirLiteral::Integer(source_var_id, _))) = var_id_arg else {
Expand All @@ -73,13 +78,18 @@ impl<'interner> Monomorphizer<'interner> {
// then update the ID used for tracking at runtime
let var_id = self.debug_type_tracker.insert_var(source_var_id, var_type);
let interned_var_id = self.intern_var_id(var_id, &call.location);
arguments[DEBUG_VAR_ID_ARG_SLOT] = self.expr(interned_var_id);
arguments[DEBUG_VAR_ID_ARG_SLOT] = self.expr(interned_var_id)?;
Ok(())
}

/// Update instrumentation code for a variable being dropped out of scope.
/// Given the source_var_id we search for the last assigned debug var_id and
/// replace it instead.
fn patch_debug_var_drop(&mut self, call: &HirCallExpression, arguments: &mut [Expression]) {
fn patch_debug_var_drop(
&mut self,
call: &HirCallExpression,
arguments: &mut [Expression],
) -> Result<(), MonomorphizationError> {
let hir_arguments = vecmap(&call.arguments, |id| self.interner.expression(id));
let var_id_arg = hir_arguments.get(DEBUG_VAR_ID_ARG_SLOT);
let Some(HirExpression::Literal(HirLiteral::Integer(source_var_id, _))) = var_id_arg else {
Expand All @@ -92,7 +102,8 @@ impl<'interner> Monomorphizer<'interner> {
.get_var_id(source_var_id)
.unwrap_or_else(|| unreachable!("failed to find debug variable"));
let interned_var_id = self.intern_var_id(var_id, &call.location);
arguments[DEBUG_VAR_ID_ARG_SLOT] = self.expr(interned_var_id);
arguments[DEBUG_VAR_ID_ARG_SLOT] = self.expr(interned_var_id)?;
Ok(())
}

/// Update instrumentation code inserted when assigning to a member of an
Expand All @@ -106,7 +117,7 @@ impl<'interner> Monomorphizer<'interner> {
call: &HirCallExpression,
arguments: &mut [Expression],
arity: usize,
) {
) -> Result<(), MonomorphizationError> {
let hir_arguments = vecmap(&call.arguments, |id| self.interner.expression(id));
let var_id_arg = hir_arguments.get(DEBUG_VAR_ID_ARG_SLOT);
let Some(HirExpression::Literal(HirLiteral::Integer(source_var_id, _))) = var_id_arg else {
Expand Down Expand Up @@ -149,7 +160,7 @@ impl<'interner> Monomorphizer<'interner> {
call.location.span,
call.location.file,
);
arguments[DEBUG_MEMBER_FIELD_INDEX_ARG_SLOT + i] = self.expr(index_id);
arguments[DEBUG_MEMBER_FIELD_INDEX_ARG_SLOT + i] = self.expr(index_id)?;
} else {
// array/string element using constant index
cursor_type = element_type_at_index(cursor_type, index as usize);
Expand All @@ -165,7 +176,8 @@ impl<'interner> Monomorphizer<'interner> {
.get_var_id(source_var_id)
.unwrap_or_else(|| unreachable!("failed to find debug variable"));
let interned_var_id = self.intern_var_id(var_id, &call.location);
arguments[DEBUG_VAR_ID_ARG_SLOT] = self.expr(interned_var_id);
arguments[DEBUG_VAR_ID_ARG_SLOT] = self.expr(interned_var_id)?;
Ok(())
}

fn intern_var_id(&mut self, var_id: DebugVarId, location: &Location) -> ExprId {
Expand Down

0 comments on commit 00ab3db

Please sign in to comment.