From c55b1b24f9f47fe0f2c2baccdef4d0380bdd7c43 Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Tue, 17 Sep 2024 13:54:28 -0500 Subject: [PATCH] Fix stack trace ordering --- compiler/noirc_errors/src/lib.rs | 11 +---------- compiler/noirc_errors/src/reporter.rs | 17 ++++++++++++++--- compiler/noirc_evaluator/src/errors.rs | 6 +++--- .../noirc_frontend/src/hir/comptime/errors.rs | 10 ++-------- .../src/monomorphization/errors.rs | 2 +- tooling/nargo/src/errors.rs | 9 +++------ 6 files changed, 24 insertions(+), 31 deletions(-) diff --git a/compiler/noirc_errors/src/lib.rs b/compiler/noirc_errors/src/lib.rs index 6eaa96e0cb1..bcdc0684099 100644 --- a/compiler/noirc_errors/src/lib.rs +++ b/compiler/noirc_errors/src/lib.rs @@ -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, } 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) -> Self { - self.call_stack = call_stack; - self + FileDiagnostic { file_id, diagnostic } } } diff --git a/compiler/noirc_errors/src/reporter.rs b/compiler/noirc_errors/src/reporter.rs index 0886f83af9d..76e308969b4 100644 --- a/compiler/noirc_errors/src/reporter.rs +++ b/compiler/noirc_errors/src/reporter.rs @@ -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, } #[derive(Debug, Copy, Clone, PartialEq, Eq)] @@ -39,6 +43,7 @@ impl CustomDiagnostic { kind: DiagnosticKind::Error, deprecated: false, unnecessary: false, + call_stack: Default::default(), } } @@ -55,6 +60,7 @@ impl CustomDiagnostic { kind, deprecated: false, unnecessary: false, + call_stack: Default::default(), } } @@ -109,6 +115,7 @@ impl CustomDiagnostic { kind: DiagnosticKind::Bug, deprecated: false, unnecessary: false, + call_stack: Default::default(), } } @@ -116,6 +123,11 @@ impl CustomDiagnostic { FileDiagnostic::new(file_id, self) } + pub fn with_call_stack(mut self, call_stack: Vec) -> Self { + self.call_stack = call_stack; + self + } + pub fn add_note(&mut self, message: String) { self.notes.push(message); } @@ -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) } } @@ -213,7 +225,6 @@ pub fn report<'files>( files: &'files impl Files<'files, FileId = fm::FileId>, custom_diagnostic: &CustomDiagnostic, file: Option, - call_stack: &[Location], deny_warnings: bool, ) -> bool { let color_choice = @@ -221,7 +232,7 @@ pub fn report<'files>( 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(); diff --git a/compiler/noirc_evaluator/src/errors.rs b/compiler/noirc_evaluator/src/errors.rs index c4ba08f9acd..994e97eabb8 100644 --- a/compiler/noirc_evaluator/src/errors.rs +++ b/compiler/noirc_evaluator/src/errors.rs @@ -87,7 +87,7 @@ impl From 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(); @@ -101,7 +101,7 @@ impl From 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) } } } @@ -178,7 +178,7 @@ impl From 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) } } diff --git a/compiler/noirc_frontend/src/hir/comptime/errors.rs b/compiler/noirc_frontend/src/hir/comptime/errors.rs index e97d0ae7d5e..0cb2b37e273 100644 --- a/compiler/noirc_frontend/src/hir/comptime/errors.rs +++ b/compiler/noirc_frontend/src/hir/comptime/errors.rs @@ -391,15 +391,9 @@ impl<'a> From<&'a InterpreterError> for CustomDiagnostic { 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}`"); diff --git a/compiler/noirc_frontend/src/monomorphization/errors.rs b/compiler/noirc_frontend/src/monomorphization/errors.rs index 7f4172017e2..e2206928083 100644 --- a/compiler/noirc_frontend/src/monomorphization/errors.rs +++ b/compiler/noirc_frontend/src/monomorphization/errors.rs @@ -30,7 +30,7 @@ impl From 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) } } diff --git a/tooling/nargo/src/errors.rs b/tooling/nargo/src/errors.rs index 4a72bb1ec78..313f29517d6 100644 --- a/tooling/nargo/src/errors.rs +++ b/tooling/nargo/src/errors.rs @@ -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)) }