Skip to content

Commit

Permalink
feat: show backtrace on comptime assertion failures (#5842)
Browse files Browse the repository at this point in the history
# Description

## Problem

Resolves #5828

## Summary

`CustomDiagnostic` didn't have a `call_stack` property because so far
this wasn't needed. I only added it for `FailingConstraint`, which is,
_I think_, the only case where a backtrace might be needed. In all other
cases (for example "wrong number of arguments" it might be clear why
it's failing without needing a backtrace).

Here's an example output:

```
error: Assertion failed
   ┌─ std/option.nr:33:16
   │
33 │         assert(self._is_some);
   │                -------------
   │
   ┌─ src/main.nr:8:9
   │
 8 │         foo();
   │         -----
   │
   ┌─ src/other.nr:2:5
   │
 2 │     bar();
   │     -----
   ·
 7 │     let _ = expr.as_integer().unwrap();
   │             --------------------------
   │

Aborting due to 1 previous error
```

## Additional Context

None.

## Documentation

Check one:
- [x] No documentation needed.
- [ ] Documentation included in this PR.
- [ ] **[For Experimental Features]** 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
asterite authored Aug 29, 2024
1 parent 4e4ad26 commit cfd68d4
Show file tree
Hide file tree
Showing 8 changed files with 59 additions and 19 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 aztec_macros/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -18,5 +18,6 @@ noirc_frontend.workspace = true
noirc_errors.workspace = true
iter-extended.workspace = true
convert_case = "0.6.0"
im.workspace = true
regex = "1.10"
tiny-keccak = { version = "2.0.0", features = ["keccak"] }
18 changes: 12 additions & 6 deletions compiler/noirc_errors/src/reporter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ impl CustomDiagnostic {
) -> CustomDiagnostic {
CustomDiagnostic {
message: primary_message,
secondaries: vec![CustomLabel::new(secondary_message, secondary_span)],
secondaries: vec![CustomLabel::new(secondary_message, secondary_span, None)],
notes: Vec::new(),
kind,
}
Expand Down Expand Up @@ -98,7 +98,7 @@ impl CustomDiagnostic {
) -> CustomDiagnostic {
CustomDiagnostic {
message: primary_message,
secondaries: vec![CustomLabel::new(secondary_message, secondary_span)],
secondaries: vec![CustomLabel::new(secondary_message, secondary_span, None)],
notes: Vec::new(),
kind: DiagnosticKind::Bug,
}
Expand All @@ -113,7 +113,11 @@ impl CustomDiagnostic {
}

pub fn add_secondary(&mut self, message: String, span: Span) {
self.secondaries.push(CustomLabel::new(message, span));
self.secondaries.push(CustomLabel::new(message, span, None));
}

pub fn add_secondary_with_file(&mut self, message: String, span: Span, file: fm::FileId) {
self.secondaries.push(CustomLabel::new(message, span, Some(file)));
}

pub fn is_error(&self) -> bool {
Expand Down Expand Up @@ -153,11 +157,12 @@ impl std::fmt::Display for CustomDiagnostic {
pub struct CustomLabel {
pub message: String,
pub span: Span,
pub file: Option<fm::FileId>,
}

impl CustomLabel {
fn new(message: String, span: Span) -> CustomLabel {
CustomLabel { message, span }
fn new(message: String, span: Span, file: Option<fm::FileId>) -> CustomLabel {
CustomLabel { message, span, file }
}
}

Expand Down Expand Up @@ -234,7 +239,8 @@ fn convert_diagnostic(
.map(|sl| {
let start_span = sl.span.start() as usize;
let end_span = sl.span.end() as usize;
Label::secondary(file_id, start_span..end_span).with_message(&sl.message)
let file = sl.file.unwrap_or(file_id);
Label::secondary(file, start_span..end_span).with_message(&sl.message)
})
.collect()
} else {
Expand Down
1 change: 1 addition & 0 deletions compiler/noirc_frontend/src/elaborator/comptime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ impl<'context> Elaborator<'context> {
self.crate_id,
self.debug_comptime_in_file,
self.enable_arithmetic_generics,
self.interpreter_call_stack.clone(),
);

elaborator.function_context.push(FunctionContext::default());
Expand Down
5 changes: 5 additions & 0 deletions compiler/noirc_frontend/src/elaborator/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,8 @@ pub struct Elaborator<'context> {

/// Temporary flag to enable the experimental arithmetic generics feature
enable_arithmetic_generics: bool,

pub(crate) interpreter_call_stack: im::Vector<Location>,
}

#[derive(Default)]
Expand All @@ -191,6 +193,7 @@ impl<'context> Elaborator<'context> {
crate_id: CrateId,
debug_comptime_in_file: Option<FileId>,
enable_arithmetic_generics: bool,
interpreter_call_stack: im::Vector<Location>,
) -> Self {
Self {
scopes: ScopeForest::default(),
Expand All @@ -214,6 +217,7 @@ impl<'context> Elaborator<'context> {
unresolved_globals: BTreeMap::new(),
enable_arithmetic_generics,
current_trait: None,
interpreter_call_stack,
}
}

Expand All @@ -229,6 +233,7 @@ impl<'context> Elaborator<'context> {
crate_id,
debug_comptime_in_file,
enable_arithmetic_generics,
im::Vector::new(),
)
}

Expand Down
13 changes: 11 additions & 2 deletions compiler/noirc_frontend/src/hir/comptime/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ pub enum InterpreterError {
FailingConstraint {
message: Option<String>,
location: Location,
call_stack: im::Vector<Location>,
},
NoMethodFound {
name: String,
Expand Down Expand Up @@ -353,12 +354,20 @@ impl<'a> From<&'a InterpreterError> for CustomDiagnostic {
let msg = format!("Expected a `bool` but found `{typ}`");
CustomDiagnostic::simple_error(msg, String::new(), location.span)
}
InterpreterError::FailingConstraint { message, location } => {
InterpreterError::FailingConstraint { message, location, call_stack } => {
let (primary, secondary) = match message {
Some(msg) => (msg.clone(), "Assertion failed".into()),
None => ("Assertion failed".into(), String::new()),
};
CustomDiagnostic::simple_error(primary, secondary, location.span)
let mut diagnostic =
CustomDiagnostic::simple_error(primary, secondary, location.span);

// Only take at most 3 frames starting from the top of the stack to avoid producing too much output
for frame in call_stack.iter().rev().take(3) {
diagnostic.add_secondary_with_file("".to_string(), frame.span, frame.file);
}

diagnostic
}
InterpreterError::NoMethodFound { name, typ, location } => {
let msg = format!("No method named `{name}` found for type `{typ}`");
Expand Down
9 changes: 7 additions & 2 deletions compiler/noirc_frontend/src/hir/comptime/interpreter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,8 @@ impl<'local, 'interner> Interpreter<'local, 'interner> {
current_function: Option<FuncId>,
) -> Self {
let bound_generics = Vec::new();
Self { elaborator, crate_id, current_function, bound_generics, in_loop: false }
let in_loop = false;
Self { elaborator, crate_id, current_function, bound_generics, in_loop }
}

pub(crate) fn call_function(
Expand Down Expand Up @@ -99,8 +100,11 @@ impl<'local, 'interner> Interpreter<'local, 'interner> {
}

self.remember_bindings(&instantiation_bindings, &impl_bindings);
self.elaborator.interpreter_call_stack.push_back(location);

let result = self.call_function_inner(function, arguments, location);

self.elaborator.interpreter_call_stack.pop_back();
undo_instantiation_bindings(impl_bindings);
undo_instantiation_bindings(instantiation_bindings);
self.rebind_generics_from_previous_function();
Expand Down Expand Up @@ -1462,7 +1466,8 @@ impl<'local, 'interner> Interpreter<'local, 'interner> {
let message = constrain.2.and_then(|expr| self.evaluate(expr).ok());
let message =
message.map(|value| value.display(self.elaborator.interner).to_string());
Err(InterpreterError::FailingConstraint { location, message })
let call_stack = self.elaborator.interpreter_call_stack.clone();
Err(InterpreterError::FailingConstraint { location, message, call_stack })
}
value => {
let location = self.elaborator.interner.expr_location(&constrain.0);
Expand Down
30 changes: 21 additions & 9 deletions compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ impl<'local, 'context> Interpreter<'local, 'context> {
location: Location,
) -> IResult<Value> {
let interner = &mut self.elaborator.interner;
let call_stack = &self.elaborator.interpreter_call_stack;
match name {
"array_as_str_unchecked" => array_as_str_unchecked(interner, arguments, location),
"array_len" => array_len(interner, arguments, location),
Expand Down Expand Up @@ -110,11 +111,11 @@ impl<'local, 'context> Interpreter<'local, 'context> {
"quoted_as_type" => quoted_as_type(self, arguments, location),
"quoted_eq" => quoted_eq(arguments, location),
"slice_insert" => slice_insert(interner, arguments, location),
"slice_pop_back" => slice_pop_back(interner, arguments, location),
"slice_pop_front" => slice_pop_front(interner, arguments, location),
"slice_pop_back" => slice_pop_back(interner, arguments, location, call_stack),
"slice_pop_front" => slice_pop_front(interner, arguments, location, call_stack),
"slice_push_back" => slice_push_back(interner, arguments, location),
"slice_push_front" => slice_push_front(interner, arguments, location),
"slice_remove" => slice_remove(interner, arguments, location),
"slice_remove" => slice_remove(interner, arguments, location, call_stack),
"struct_def_as_type" => struct_def_as_type(interner, arguments, location),
"struct_def_fields" => struct_def_fields(interner, arguments, location),
"struct_def_generics" => struct_def_generics(interner, arguments, location),
Expand Down Expand Up @@ -154,8 +155,16 @@ impl<'local, 'context> Interpreter<'local, 'context> {
}
}

fn failing_constraint<T>(message: impl Into<String>, location: Location) -> IResult<T> {
Err(InterpreterError::FailingConstraint { message: Some(message.into()), location })
fn failing_constraint<T>(
message: impl Into<String>,
location: Location,
call_stack: &im::Vector<Location>,
) -> IResult<T> {
Err(InterpreterError::FailingConstraint {
message: Some(message.into()),
location,
call_stack: call_stack.clone(),
})
}

fn array_len(
Expand Down Expand Up @@ -287,22 +296,23 @@ fn slice_remove(
interner: &mut NodeInterner,
arguments: Vec<(Value, Location)>,
location: Location,
call_stack: &im::Vector<Location>,
) -> IResult<Value> {
let (slice, index) = check_two_arguments(arguments, location)?;

let (mut values, typ) = get_slice(interner, slice)?;
let index = get_u32(index)? as usize;

if values.is_empty() {
return failing_constraint("slice_remove called on empty slice", location);
return failing_constraint("slice_remove called on empty slice", location, call_stack);
}

if index >= values.len() {
let message = format!(
"slice_remove: index {index} is out of bounds for a slice of length {}",
values.len()
);
return failing_constraint(message, location);
return failing_constraint(message, location, call_stack);
}

let element = values.remove(index);
Expand All @@ -325,27 +335,29 @@ fn slice_pop_front(
interner: &mut NodeInterner,
arguments: Vec<(Value, Location)>,
location: Location,
call_stack: &im::Vector<Location>,
) -> IResult<Value> {
let argument = check_one_argument(arguments, location)?;

let (mut values, typ) = get_slice(interner, argument)?;
match values.pop_front() {
Some(element) => Ok(Value::Tuple(vec![element, Value::Slice(values, typ)])),
None => failing_constraint("slice_pop_front called on empty slice", location),
None => failing_constraint("slice_pop_front called on empty slice", location, call_stack),
}
}

fn slice_pop_back(
interner: &mut NodeInterner,
arguments: Vec<(Value, Location)>,
location: Location,
call_stack: &im::Vector<Location>,
) -> IResult<Value> {
let argument = check_one_argument(arguments, location)?;

let (mut values, typ) = get_slice(interner, argument)?;
match values.pop_back() {
Some(element) => Ok(Value::Tuple(vec![Value::Slice(values, typ), element])),
None => failing_constraint("slice_pop_back called on empty slice", location),
None => failing_constraint("slice_pop_back called on empty slice", location, call_stack),
}
}

Expand Down

0 comments on commit cfd68d4

Please sign in to comment.