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: Correct stack trace order in comptime assertion failures #6066

Merged
merged 1 commit into from
Sep 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 1 addition & 10 deletions compiler/noirc_errors/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,20 +13,11 @@ pub use reporter::{CustomDiagnostic, DiagnosticKind};
pub struct FileDiagnostic {
pub file_id: fm::FileId,
pub diagnostic: CustomDiagnostic,

/// An optional call stack to display the full runtime call stack
/// leading up to a runtime error. If this is empty it will not be displayed.
pub call_stack: Vec<Location>,
}

impl FileDiagnostic {
pub fn new(file_id: fm::FileId, diagnostic: CustomDiagnostic) -> FileDiagnostic {
FileDiagnostic { file_id, diagnostic, call_stack: Vec::new() }
}

pub fn with_call_stack(mut self, call_stack: Vec<Location>) -> Self {
self.call_stack = call_stack;
self
FileDiagnostic { file_id, diagnostic }
}
}

Expand Down
17 changes: 14 additions & 3 deletions compiler/noirc_errors/src/reporter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ pub struct CustomDiagnostic {
pub kind: DiagnosticKind,
pub deprecated: bool,
pub unnecessary: bool,

/// An optional call stack to display the full runtime call stack
/// leading up to a runtime error. If this is empty it will not be displayed.
pub call_stack: Vec<Location>,
}

#[derive(Debug, Copy, Clone, PartialEq, Eq)]
Expand All @@ -39,6 +43,7 @@ impl CustomDiagnostic {
kind: DiagnosticKind::Error,
deprecated: false,
unnecessary: false,
call_stack: Default::default(),
}
}

Expand All @@ -55,6 +60,7 @@ impl CustomDiagnostic {
kind,
deprecated: false,
unnecessary: false,
call_stack: Default::default(),
}
}

Expand Down Expand Up @@ -109,13 +115,19 @@ impl CustomDiagnostic {
kind: DiagnosticKind::Bug,
deprecated: false,
unnecessary: false,
call_stack: Default::default(),
}
}

pub fn in_file(self, file_id: fm::FileId) -> FileDiagnostic {
FileDiagnostic::new(file_id, self)
}

pub fn with_call_stack(mut self, call_stack: Vec<Location>) -> Self {
self.call_stack = call_stack;
self
}

pub fn add_note(&mut self, message: String) {
self.notes.push(message);
}
Expand Down Expand Up @@ -204,7 +216,7 @@ impl FileDiagnostic {
files: &'files impl Files<'files, FileId = fm::FileId>,
deny_warnings: bool,
) -> bool {
report(files, &self.diagnostic, Some(self.file_id), &self.call_stack, deny_warnings)
report(files, &self.diagnostic, Some(self.file_id), deny_warnings)
}
}

Expand All @@ -213,15 +225,14 @@ pub fn report<'files>(
files: &'files impl Files<'files, FileId = fm::FileId>,
custom_diagnostic: &CustomDiagnostic,
file: Option<fm::FileId>,
call_stack: &[Location],
deny_warnings: bool,
) -> bool {
let color_choice =
if std::io::stderr().is_terminal() { ColorChoice::Auto } else { ColorChoice::Never };
let writer = StandardStream::stderr(color_choice);
let config = codespan_reporting::term::Config::default();

let stack_trace = stack_trace(files, call_stack);
let stack_trace = stack_trace(files, &custom_diagnostic.call_stack);
let diagnostic = convert_diagnostic(custom_diagnostic, file, stack_trace, deny_warnings);
term::emit(&mut writer.lock(), &config, files, &diagnostic).unwrap();

Expand Down
6 changes: 3 additions & 3 deletions compiler/noirc_evaluator/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ impl From<SsaReport> for FileDiagnostic {
let location = call_stack.last().expect("Expected RuntimeError to have a location");
let diagnostic =
Diagnostic::simple_warning(message, secondary_message, location.span);
diagnostic.in_file(file_id).with_call_stack(call_stack)
diagnostic.with_call_stack(call_stack).in_file(file_id)
}
SsaReport::Bug(bug) => {
let message = bug.to_string();
Expand All @@ -101,7 +101,7 @@ impl From<SsaReport> for FileDiagnostic {
let file_id = call_stack.last().map(|location| location.file).unwrap_or_default();
let location = call_stack.last().expect("Expected RuntimeError to have a location");
let diagnostic = Diagnostic::simple_bug(message, secondary_message, location.span);
diagnostic.in_file(file_id).with_call_stack(call_stack)
diagnostic.with_call_stack(call_stack).in_file(file_id)
}
}
}
Expand Down Expand Up @@ -178,7 +178,7 @@ impl From<RuntimeError> for FileDiagnostic {
let call_stack = vecmap(error.call_stack(), |location| *location);
let file_id = call_stack.last().map(|location| location.file).unwrap_or_default();
let diagnostic = error.into_diagnostic();
diagnostic.in_file(file_id).with_call_stack(call_stack)
diagnostic.with_call_stack(call_stack).in_file(file_id)
}
}

Expand Down
10 changes: 2 additions & 8 deletions compiler/noirc_frontend/src/hir/comptime/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -391,15 +391,9 @@
Some(msg) => (msg.clone(), "Assertion failed".into()),
None => ("Assertion failed".into(), String::new()),
};
let mut diagnostic =
CustomDiagnostic::simple_error(primary, secondary, location.span);
let diagnostic = CustomDiagnostic::simple_error(primary, secondary, location.span);

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

diagnostic
diagnostic.with_call_stack(call_stack.into_iter().copied().collect())
}
InterpreterError::NoMethodFound { name, typ, location } => {
let msg = format!("No method named `{name}` found for type `{typ}`");
Expand Down Expand Up @@ -496,7 +490,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 493 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 Expand Up @@ -612,7 +606,7 @@
}
InterpreterError::GenericNameShouldBeAnIdent { name, location } => {
let msg =
"Generic name needs to be a valid identifer (one word beginning with a letter)"

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

View workflow job for this annotation

GitHub Actions / Code

Unknown word (identifer)
.to_string();
let secondary = format!("`{name}` is not a valid identifier");
CustomDiagnostic::simple_error(msg, secondary, location.span)
Expand Down
2 changes: 1 addition & 1 deletion compiler/noirc_frontend/src/monomorphization/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ impl From<MonomorphizationError> for FileDiagnostic {
let location = error.location();
let call_stack = vec![location];
let diagnostic = error.into_diagnostic();
diagnostic.in_file(location.file).with_call_stack(call_stack)
diagnostic.with_call_stack(call_stack).in_file(location.file)
}
}

Expand Down
9 changes: 3 additions & 6 deletions tooling/nargo/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -238,11 +238,8 @@ pub fn try_to_diagnose_runtime_error(
};
// The location of the error itself will be the location at the top
// of the call stack (the last item in the Vec).
let location = source_locations.last()?;
let location = *source_locations.last()?;
let message = extract_message_from_error(&abi.error_types, nargo_err);
Some(
CustomDiagnostic::simple_error(message, String::new(), location.span)
.in_file(location.file)
.with_call_stack(source_locations),
)
let error = CustomDiagnostic::simple_error(message, String::new(), location.span);
Some(error.with_call_stack(source_locations).in_file(location.file))
}
Loading