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

feat: show backtrace on comptime assertion failures #5842

Merged
merged 12 commits into from
Aug 29, 2024
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
9 changes: 6 additions & 3 deletions compiler/noirc_frontend/src/elaborator/comptime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ impl<'context> Elaborator<'context> {
return Err((ResolverError::NonFunctionInAnnotation { span }.into(), self.file));
};

let mut interpreter = self.setup_interpreter();
let mut interpreter = self.setup_interpreter(location);
let mut arguments =
Self::handle_attribute_arguments(&mut interpreter, function, arguments, location)
.map_err(|error| {
Expand Down Expand Up @@ -345,12 +345,15 @@ impl<'context> Elaborator<'context> {
}
}

pub fn setup_interpreter<'local>(&'local mut self) -> Interpreter<'local, 'context> {
pub fn setup_interpreter<'local>(
&'local mut self,
location: Location,
) -> Interpreter<'local, 'context> {
jfecher marked this conversation as resolved.
Show resolved Hide resolved
let current_function = match self.current_item {
Some(DependencyId::Function(function)) => Some(function),
_ => None,
};
Interpreter::new(self, self.crate_id, current_function)
Interpreter::new(self, self.crate_id, current_function, location)
}

pub(super) fn debug_comptime<T: Display, F: FnMut(&mut NodeInterner) -> T>(
Expand Down
5 changes: 3 additions & 2 deletions compiler/noirc_frontend/src/elaborator/expressions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -744,7 +744,8 @@ impl<'context> Elaborator<'context> {
let (block, _typ) = self.elaborate_block_expression(block);

self.check_and_pop_function_context();
let mut interpreter = self.setup_interpreter();
let location = Location::new(span, self.file);
let mut interpreter = self.setup_interpreter(location);
let value = interpreter.evaluate_block(block);
let (id, typ) = self.inline_comptime_value(value, span);

Expand Down Expand Up @@ -829,7 +830,7 @@ impl<'context> Elaborator<'context> {
};

let file = self.file;
let mut interpreter = self.setup_interpreter();
let mut interpreter = self.setup_interpreter(location);
let mut comptime_args = Vec::new();
let mut errors = Vec::new();

Expand Down
2 changes: 1 addition & 1 deletion compiler/noirc_frontend/src/elaborator/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1313,7 +1313,7 @@ impl<'context> Elaborator<'context> {
let global = self.interner.get_global(global_id);
let definition_id = global.definition_id;
let location = global.location;
let mut interpreter = self.setup_interpreter();
let mut interpreter = self.setup_interpreter(location);

if let Err(error) = interpreter.evaluate_let(let_statement) {
self.errors.push(error.into_compilation_error_pair());
Expand Down
2 changes: 1 addition & 1 deletion compiler/noirc_frontend/src/elaborator/patterns.rs
Original file line number Diff line number Diff line change
Expand Up @@ -505,7 +505,7 @@ impl<'context> Elaborator<'context> {
// Comptime variables must be replaced with their values
if let Some(definition) = self.interner.try_definition(definition_id) {
if definition.comptime && !self.in_comptime_context() {
let mut interpreter = self.setup_interpreter();
let mut interpreter = self.setup_interpreter(definition.location);
let value = interpreter.evaluate(id);
return self.inline_comptime_value(value, span);
}
Expand Down
3 changes: 2 additions & 1 deletion compiler/noirc_frontend/src/elaborator/statements.rs
Original file line number Diff line number Diff line change
Expand Up @@ -440,7 +440,8 @@ impl<'context> Elaborator<'context> {
let span = statement.span;
let (hir_statement, _typ) = self.elaborate_statement(statement);
self.check_and_pop_function_context();
let mut interpreter = self.setup_interpreter();
let location = Location::new(span, self.file);
let mut interpreter = self.setup_interpreter(location);
let value = interpreter.evaluate_statement(hir_statement);
let (expr, typ) = self.inline_comptime_value(value, span);

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 @@
FailingConstraint {
message: Option<String>,
location: Location,
call_stack: Vec<Location>,
},
NoMethodFound {
name: String,
Expand Down Expand Up @@ -353,12 +354,20 @@
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 Expand Up @@ -455,7 +464,7 @@
let message = format!("Failed to parse macro's token stream into {rule}");
let tokens = vecmap(tokens.iter(), ToString::to_string).join(" ");

// 10 is an aribtrary number of tokens here chosen to fit roughly onto one line

Check warning on line 467 in compiler/noirc_frontend/src/hir/comptime/errors.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (aribtrary)
let token_stream = if tokens.len() > 10 {
format!("The resulting token stream was: {tokens}")
} else {
Expand Down
13 changes: 11 additions & 2 deletions compiler/noirc_frontend/src/hir/comptime/interpreter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,8 @@
/// multiple times. Without this map, when one of these inner functions exits we would
/// unbind the generic completely instead of resetting it to its previous binding.
bound_generics: Vec<HashMap<TypeVariable, Type>>,

call_stack: Vec<Location>,
}

#[allow(unused)]
Expand All @@ -68,9 +70,12 @@
elaborator: &'local mut Elaborator<'interner>,
crate_id: CrateId,
current_function: Option<FuncId>,
location: Location,
) -> Self {
let bound_generics = Vec::new();
Self { elaborator, crate_id, current_function, bound_generics, in_loop: false }
let in_loop = false;
let call_stack = vec![location];
Self { elaborator, crate_id, current_function, bound_generics, in_loop, call_stack }
}

pub(crate) fn call_function(
Expand Down Expand Up @@ -99,8 +104,11 @@
}

self.remember_bindings(&instantiation_bindings, &impl_bindings);
self.call_stack.push(location);

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

self.call_stack.pop();
undo_instantiation_bindings(impl_bindings);
undo_instantiation_bindings(instantiation_bindings);
self.rebind_generics_from_previous_function();
Expand Down Expand Up @@ -219,7 +227,7 @@
}
} else {
let name = self.elaborator.interner.function_name(&function);
unreachable!("Non-builtin, lowlevel or oracle builtin fn '{name}'")

Check warning on line 230 in compiler/noirc_frontend/src/hir/comptime/interpreter.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (lowlevel)
}
}

Expand Down Expand Up @@ -1462,7 +1470,8 @@
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.call_stack.clone();
Err(InterpreterError::FailingConstraint { location, message, call_stack })
}
value => {
let location = self.elaborator.interner.expr_location(&constrain.0);
Expand Down
29 changes: 20 additions & 9 deletions compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,11 +102,11 @@
"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, &self.call_stack),
"slice_pop_front" => slice_pop_front(interner, arguments, location, &self.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, &self.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 @@ -145,8 +145,16 @@
}
}

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: &[Location],
) -> IResult<T> {
Err(InterpreterError::FailingConstraint {
message: Some(message.into()),
location,
call_stack: call_stack.to_vec(),
asterite marked this conversation as resolved.
Show resolved Hide resolved
})
}

fn array_len(
Expand Down Expand Up @@ -278,22 +286,23 @@
interner: &mut NodeInterner,
arguments: Vec<(Value, Location)>,
location: Location,
call_stack: &[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 @@ -316,27 +325,29 @@
interner: &mut NodeInterner,
arguments: Vec<(Value, Location)>,
location: Location,
call_stack: &[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: &[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 Expand Up @@ -413,7 +424,7 @@
let argument = check_one_argument(arguments, location)?;
let typ = parse(argument, parser::parse_type(), "a type")?;
let typ =
interpreter.elaborate_item(interpreter.current_function, |elab| elab.resolve_type(typ));

Check warning on line 427 in compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (elab)

Check warning on line 427 in compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (elab)
Ok(Value::Type(typ))
}

Expand Down
2 changes: 1 addition & 1 deletion compiler/noirc_frontend/src/hir/comptime/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ fn interpret_helper(src: &str) -> Result<Value, InterpreterError> {
Elaborator::elaborate_and_return_self(&mut context, krate, collector.items, None, false);
assert_eq!(elaborator.errors.len(), 0);

let mut interpreter = elaborator.setup_interpreter();
let mut interpreter = elaborator.setup_interpreter(location);

let no_location = Location::dummy();
interpreter.call_function(main, Vec::new(), HashMap::new(), no_location)
Expand Down
Loading