From 385359bd1ba546036c1110d166b6cccbf369ab77 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Tue, 27 Aug 2024 16:30:13 -0300 Subject: [PATCH 01/12] Interpreter FailingConstraint now shows the backtrace --- compiler/noirc_errors/src/reporter.rs | 18 +++++++++++++++++- .../noirc_frontend/src/hir/comptime/errors.rs | 8 ++++++-- .../src/hir/comptime/interpreter.rs | 12 ++++++++++-- .../src/hir/comptime/interpreter/builtin.rs | 3 ++- 4 files changed, 35 insertions(+), 6 deletions(-) diff --git a/compiler/noirc_errors/src/reporter.rs b/compiler/noirc_errors/src/reporter.rs index 3ce0f268715..4ef9c58c32b 100644 --- a/compiler/noirc_errors/src/reporter.rs +++ b/compiler/noirc_errors/src/reporter.rs @@ -12,6 +12,7 @@ pub struct CustomDiagnostic { pub secondaries: Vec, notes: Vec, pub kind: DiagnosticKind, + call_stack: Option>, } #[derive(Debug, Copy, Clone, PartialEq, Eq)] @@ -35,6 +36,7 @@ impl CustomDiagnostic { secondaries: Vec::new(), notes: Vec::new(), kind: DiagnosticKind::Error, + call_stack: None, } } @@ -49,6 +51,7 @@ impl CustomDiagnostic { secondaries: vec![CustomLabel::new(secondary_message, secondary_span)], notes: Vec::new(), kind, + call_stack: None, } } @@ -101,6 +104,7 @@ impl CustomDiagnostic { secondaries: vec![CustomLabel::new(secondary_message, secondary_span)], notes: Vec::new(), kind: DiagnosticKind::Bug, + call_stack: None, } } @@ -116,6 +120,10 @@ impl CustomDiagnostic { self.secondaries.push(CustomLabel::new(message, span)); } + pub fn set_call_stack(&mut self, call_stack: Vec) { + self.call_stack = Some(call_stack); + } + pub fn is_error(&self) -> bool { matches!(self.kind, DiagnosticKind::Error) } @@ -228,7 +236,7 @@ fn convert_diagnostic( _ => Diagnostic::error(), }; - let secondary_labels = if let Some(file_id) = file { + let mut secondary_labels = if let Some(file_id) = file { cd.secondaries .iter() .map(|sl| { @@ -241,6 +249,14 @@ fn convert_diagnostic( vec![] }; + if let Some(call_stack) = &cd.call_stack { + secondary_labels.extend(call_stack.iter().map(|frame| { + let start_span = frame.span.start() as usize; + let end_span = frame.span.end() as usize; + Label::secondary(frame.file, start_span..end_span) + })); + } + let mut notes = cd.notes.clone(); notes.push(stack_trace); diff --git a/compiler/noirc_frontend/src/hir/comptime/errors.rs b/compiler/noirc_frontend/src/hir/comptime/errors.rs index fd916485eaf..40d63bc834a 100644 --- a/compiler/noirc_frontend/src/hir/comptime/errors.rs +++ b/compiler/noirc_frontend/src/hir/comptime/errors.rs @@ -56,6 +56,7 @@ pub enum InterpreterError { FailingConstraint { message: Option, location: Location, + call_stack: Vec, }, NoMethodFound { name: String, @@ -353,12 +354,15 @@ 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); + diagnostic.set_call_stack(call_stack.clone()); + diagnostic } InterpreterError::NoMethodFound { name, typ, location } => { let msg = format!("No method named `{name}` found for type `{typ}`"); diff --git a/compiler/noirc_frontend/src/hir/comptime/interpreter.rs b/compiler/noirc_frontend/src/hir/comptime/interpreter.rs index 33f8c9d8332..9dea9a28035 100644 --- a/compiler/noirc_frontend/src/hir/comptime/interpreter.rs +++ b/compiler/noirc_frontend/src/hir/comptime/interpreter.rs @@ -60,6 +60,8 @@ pub struct Interpreter<'local, 'interner> { /// 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>, + + call_stack: Vec, } #[allow(unused)] @@ -70,7 +72,9 @@ impl<'local, 'interner> Interpreter<'local, 'interner> { current_function: Option, ) -> 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::new(); + Self { elaborator, crate_id, current_function, bound_generics, in_loop, call_stack } } pub(crate) fn call_function( @@ -99,8 +103,11 @@ impl<'local, 'interner> Interpreter<'local, 'interner> { } 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(); @@ -1462,7 +1469,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.call_stack.clone(); + Err(InterpreterError::FailingConstraint { location, message, call_stack }) } value => { let location = self.elaborator.interner.expr_location(&constrain.0); diff --git a/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs b/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs index 8aa8e92408f..c43bc5adc2b 100644 --- a/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs +++ b/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs @@ -146,7 +146,8 @@ impl<'local, 'context> Interpreter<'local, 'context> { } fn failing_constraint(message: impl Into, location: Location) -> IResult { - Err(InterpreterError::FailingConstraint { message: Some(message.into()), location }) + let call_stack = Vec::new(); + Err(InterpreterError::FailingConstraint { message: Some(message.into()), location, call_stack }) } fn array_len( From 48339015ac3b0d4fd2f1164c0fdff8adba4e8c7e Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Tue, 27 Aug 2024 17:37:05 -0300 Subject: [PATCH 02/12] Pass call_stack to failing_constraint --- .../src/hir/comptime/interpreter/builtin.rs | 34 ++++++++++++++----- 1 file changed, 25 insertions(+), 9 deletions(-) diff --git a/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs b/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs index c43bc5adc2b..a5fff39d944 100644 --- a/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs +++ b/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs @@ -102,11 +102,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, &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), @@ -145,8 +145,11 @@ impl<'local, 'context> Interpreter<'local, 'context> { } } -fn failing_constraint(message: impl Into, location: Location) -> IResult { - let call_stack = Vec::new(); +fn failing_constraint( + message: impl Into, + location: Location, + call_stack: Vec, +) -> IResult { Err(InterpreterError::FailingConstraint { message: Some(message.into()), location, call_stack }) } @@ -279,6 +282,7 @@ fn slice_remove( interner: &mut NodeInterner, arguments: Vec<(Value, Location)>, location: Location, + call_stack: &Vec, ) -> IResult { let (slice, index) = check_two_arguments(arguments, location)?; @@ -286,7 +290,11 @@ fn slice_remove( 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.clone(), + ); } if index >= values.len() { @@ -294,7 +302,7 @@ fn slice_remove( "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.clone()); } let element = values.remove(index); @@ -317,13 +325,18 @@ fn slice_pop_front( interner: &mut NodeInterner, arguments: Vec<(Value, Location)>, location: Location, + call_stack: &Vec, ) -> IResult { 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.clone(), + ), } } @@ -331,13 +344,16 @@ fn slice_pop_back( interner: &mut NodeInterner, arguments: Vec<(Value, Location)>, location: Location, + call_stack: &Vec, ) -> IResult { 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.clone()) + } } } From 435db64fbb0bd901de5bd0629f65f507bf8d6195 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Tue, 27 Aug 2024 17:40:33 -0300 Subject: [PATCH 03/12] Always have an initial call stack location when setting up interpreter --- compiler/noirc_frontend/src/elaborator/comptime.rs | 9 ++++++--- compiler/noirc_frontend/src/elaborator/expressions.rs | 5 +++-- compiler/noirc_frontend/src/elaborator/mod.rs | 2 +- compiler/noirc_frontend/src/elaborator/patterns.rs | 2 +- compiler/noirc_frontend/src/elaborator/statements.rs | 3 ++- compiler/noirc_frontend/src/hir/comptime/interpreter.rs | 3 ++- compiler/noirc_frontend/src/hir/comptime/tests.rs | 2 +- 7 files changed, 16 insertions(+), 10 deletions(-) diff --git a/compiler/noirc_frontend/src/elaborator/comptime.rs b/compiler/noirc_frontend/src/elaborator/comptime.rs index 01b4585640f..e81f811834e 100644 --- a/compiler/noirc_frontend/src/elaborator/comptime.rs +++ b/compiler/noirc_frontend/src/elaborator/comptime.rs @@ -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| { @@ -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> { 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>( diff --git a/compiler/noirc_frontend/src/elaborator/expressions.rs b/compiler/noirc_frontend/src/elaborator/expressions.rs index cf0b4f4071a..346cbf956a1 100644 --- a/compiler/noirc_frontend/src/elaborator/expressions.rs +++ b/compiler/noirc_frontend/src/elaborator/expressions.rs @@ -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); @@ -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(); diff --git a/compiler/noirc_frontend/src/elaborator/mod.rs b/compiler/noirc_frontend/src/elaborator/mod.rs index 53b46536078..fbedf86e3ef 100644 --- a/compiler/noirc_frontend/src/elaborator/mod.rs +++ b/compiler/noirc_frontend/src/elaborator/mod.rs @@ -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()); diff --git a/compiler/noirc_frontend/src/elaborator/patterns.rs b/compiler/noirc_frontend/src/elaborator/patterns.rs index 06c153d4c10..d71d85e11d3 100644 --- a/compiler/noirc_frontend/src/elaborator/patterns.rs +++ b/compiler/noirc_frontend/src/elaborator/patterns.rs @@ -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); } diff --git a/compiler/noirc_frontend/src/elaborator/statements.rs b/compiler/noirc_frontend/src/elaborator/statements.rs index 0bb8641b6b3..2da87e39f38 100644 --- a/compiler/noirc_frontend/src/elaborator/statements.rs +++ b/compiler/noirc_frontend/src/elaborator/statements.rs @@ -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); diff --git a/compiler/noirc_frontend/src/hir/comptime/interpreter.rs b/compiler/noirc_frontend/src/hir/comptime/interpreter.rs index 9dea9a28035..747b00b6077 100644 --- a/compiler/noirc_frontend/src/hir/comptime/interpreter.rs +++ b/compiler/noirc_frontend/src/hir/comptime/interpreter.rs @@ -70,10 +70,11 @@ impl<'local, 'interner> Interpreter<'local, 'interner> { elaborator: &'local mut Elaborator<'interner>, crate_id: CrateId, current_function: Option, + location: Location, ) -> Self { let bound_generics = Vec::new(); let in_loop = false; - let call_stack = Vec::new(); + let call_stack = vec![location]; Self { elaborator, crate_id, current_function, bound_generics, in_loop, call_stack } } diff --git a/compiler/noirc_frontend/src/hir/comptime/tests.rs b/compiler/noirc_frontend/src/hir/comptime/tests.rs index 4c1adf9fca0..53741be804e 100644 --- a/compiler/noirc_frontend/src/hir/comptime/tests.rs +++ b/compiler/noirc_frontend/src/hir/comptime/tests.rs @@ -48,7 +48,7 @@ fn interpret_helper(src: &str) -> Result { 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) From 0817868ba911c4693c0c2032ac0d744054386fbe Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Tue, 27 Aug 2024 17:45:29 -0300 Subject: [PATCH 04/12] clippy --- .../src/hir/comptime/interpreter/builtin.rs | 32 ++++++++----------- 1 file changed, 13 insertions(+), 19 deletions(-) diff --git a/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs b/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs index a5fff39d944..fc4215e8537 100644 --- a/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs +++ b/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs @@ -148,9 +148,13 @@ impl<'local, 'context> Interpreter<'local, 'context> { fn failing_constraint( message: impl Into, location: Location, - call_stack: Vec, + call_stack: &[Location], ) -> IResult { - Err(InterpreterError::FailingConstraint { message: Some(message.into()), location, call_stack }) + Err(InterpreterError::FailingConstraint { + message: Some(message.into()), + location, + call_stack: call_stack.to_vec(), + }) } fn array_len( @@ -282,7 +286,7 @@ fn slice_remove( interner: &mut NodeInterner, arguments: Vec<(Value, Location)>, location: Location, - call_stack: &Vec, + call_stack: &[Location], ) -> IResult { let (slice, index) = check_two_arguments(arguments, location)?; @@ -290,11 +294,7 @@ fn slice_remove( let index = get_u32(index)? as usize; if values.is_empty() { - return failing_constraint( - "slice_remove called on empty slice", - location, - call_stack.clone(), - ); + return failing_constraint("slice_remove called on empty slice", location, call_stack); } if index >= values.len() { @@ -302,7 +302,7 @@ fn slice_remove( "slice_remove: index {index} is out of bounds for a slice of length {}", values.len() ); - return failing_constraint(message, location, call_stack.clone()); + return failing_constraint(message, location, call_stack); } let element = values.remove(index); @@ -325,18 +325,14 @@ fn slice_pop_front( interner: &mut NodeInterner, arguments: Vec<(Value, Location)>, location: Location, - call_stack: &Vec, + call_stack: &[Location], ) -> IResult { 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, - call_stack.clone(), - ), + None => failing_constraint("slice_pop_front called on empty slice", location, call_stack), } } @@ -344,16 +340,14 @@ fn slice_pop_back( interner: &mut NodeInterner, arguments: Vec<(Value, Location)>, location: Location, - call_stack: &Vec, + call_stack: &[Location], ) -> IResult { 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, call_stack.clone()) - } + None => failing_constraint("slice_pop_back called on empty slice", location, call_stack), } } From 9504c6bc41767e97a2d0b9b346125e7d000480aa Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Wed, 28 Aug 2024 07:26:13 -0300 Subject: [PATCH 05/12] Add optional FileId to secondary labels --- compiler/noirc_errors/src/reporter.rs | 32 +++++++------------ .../noirc_frontend/src/hir/comptime/errors.rs | 4 ++- 2 files changed, 14 insertions(+), 22 deletions(-) diff --git a/compiler/noirc_errors/src/reporter.rs b/compiler/noirc_errors/src/reporter.rs index 4ef9c58c32b..b21dc759f14 100644 --- a/compiler/noirc_errors/src/reporter.rs +++ b/compiler/noirc_errors/src/reporter.rs @@ -12,7 +12,6 @@ pub struct CustomDiagnostic { pub secondaries: Vec, notes: Vec, pub kind: DiagnosticKind, - call_stack: Option>, } #[derive(Debug, Copy, Clone, PartialEq, Eq)] @@ -36,7 +35,6 @@ impl CustomDiagnostic { secondaries: Vec::new(), notes: Vec::new(), kind: DiagnosticKind::Error, - call_stack: None, } } @@ -48,10 +46,9 @@ 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, - call_stack: None, } } @@ -101,10 +98,9 @@ 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, - call_stack: None, } } @@ -117,11 +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 set_call_stack(&mut self, call_stack: Vec) { - self.call_stack = Some(call_stack); + 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 { @@ -161,11 +157,12 @@ impl std::fmt::Display for CustomDiagnostic { pub struct CustomLabel { pub message: String, pub span: Span, + pub file: Option, } impl CustomLabel { - fn new(message: String, span: Span) -> CustomLabel { - CustomLabel { message, span } + fn new(message: String, span: Span, file: Option) -> CustomLabel { + CustomLabel { message, span, file } } } @@ -236,27 +233,20 @@ fn convert_diagnostic( _ => Diagnostic::error(), }; - let mut secondary_labels = if let Some(file_id) = file { + let secondary_labels = if let Some(file_id) = file { cd.secondaries .iter() .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 { vec![] }; - if let Some(call_stack) = &cd.call_stack { - secondary_labels.extend(call_stack.iter().map(|frame| { - let start_span = frame.span.start() as usize; - let end_span = frame.span.end() as usize; - Label::secondary(frame.file, start_span..end_span) - })); - } - let mut notes = cd.notes.clone(); notes.push(stack_trace); diff --git a/compiler/noirc_frontend/src/hir/comptime/errors.rs b/compiler/noirc_frontend/src/hir/comptime/errors.rs index 40d63bc834a..08ab6822745 100644 --- a/compiler/noirc_frontend/src/hir/comptime/errors.rs +++ b/compiler/noirc_frontend/src/hir/comptime/errors.rs @@ -361,7 +361,9 @@ impl<'a> From<&'a InterpreterError> for CustomDiagnostic { }; let mut diagnostic = CustomDiagnostic::simple_error(primary, secondary, location.span); - diagnostic.set_call_stack(call_stack.clone()); + for frame in call_stack { + diagnostic.add_secondary_with_file("".to_string(), frame.span, frame.file); + } diagnostic } InterpreterError::NoMethodFound { name, typ, location } => { From b0e5393add1b35d6c8812ad10541ca2acc3b0f64 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Wed, 28 Aug 2024 15:06:37 -0300 Subject: [PATCH 06/12] Only show at most three backtrace frames --- compiler/noirc_frontend/src/hir/comptime/errors.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/compiler/noirc_frontend/src/hir/comptime/errors.rs b/compiler/noirc_frontend/src/hir/comptime/errors.rs index 08ab6822745..2269297295a 100644 --- a/compiler/noirc_frontend/src/hir/comptime/errors.rs +++ b/compiler/noirc_frontend/src/hir/comptime/errors.rs @@ -361,9 +361,12 @@ impl<'a> From<&'a InterpreterError> for CustomDiagnostic { }; let mut diagnostic = CustomDiagnostic::simple_error(primary, secondary, location.span); - for frame in call_stack { + + // 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 } => { From 9364ddb0a7aff0a82bc58c876c08ce6c072c1441 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Wed, 28 Aug 2024 15:38:09 -0300 Subject: [PATCH 07/12] Keep interpreter call stack in the Elaborator --- compiler/noirc_frontend/src/elaborator/comptime.rs | 9 +++------ .../noirc_frontend/src/elaborator/expressions.rs | 5 ++--- compiler/noirc_frontend/src/elaborator/mod.rs | 5 ++++- compiler/noirc_frontend/src/elaborator/patterns.rs | 2 +- compiler/noirc_frontend/src/elaborator/statements.rs | 3 +-- .../noirc_frontend/src/hir/comptime/interpreter.rs | 12 ++++-------- .../src/hir/comptime/interpreter/builtin.rs | 7 ++++--- compiler/noirc_frontend/src/hir/comptime/tests.rs | 2 +- 8 files changed, 20 insertions(+), 25 deletions(-) diff --git a/compiler/noirc_frontend/src/elaborator/comptime.rs b/compiler/noirc_frontend/src/elaborator/comptime.rs index e81f811834e..01b4585640f 100644 --- a/compiler/noirc_frontend/src/elaborator/comptime.rs +++ b/compiler/noirc_frontend/src/elaborator/comptime.rs @@ -139,7 +139,7 @@ impl<'context> Elaborator<'context> { return Err((ResolverError::NonFunctionInAnnotation { span }.into(), self.file)); }; - let mut interpreter = self.setup_interpreter(location); + let mut interpreter = self.setup_interpreter(); let mut arguments = Self::handle_attribute_arguments(&mut interpreter, function, arguments, location) .map_err(|error| { @@ -345,15 +345,12 @@ impl<'context> Elaborator<'context> { } } - pub fn setup_interpreter<'local>( - &'local mut self, - location: Location, - ) -> Interpreter<'local, 'context> { + pub fn setup_interpreter<'local>(&'local mut self) -> Interpreter<'local, 'context> { let current_function = match self.current_item { Some(DependencyId::Function(function)) => Some(function), _ => None, }; - Interpreter::new(self, self.crate_id, current_function, location) + Interpreter::new(self, self.crate_id, current_function) } pub(super) fn debug_comptime T>( diff --git a/compiler/noirc_frontend/src/elaborator/expressions.rs b/compiler/noirc_frontend/src/elaborator/expressions.rs index 346cbf956a1..cf0b4f4071a 100644 --- a/compiler/noirc_frontend/src/elaborator/expressions.rs +++ b/compiler/noirc_frontend/src/elaborator/expressions.rs @@ -744,8 +744,7 @@ impl<'context> Elaborator<'context> { let (block, _typ) = self.elaborate_block_expression(block); self.check_and_pop_function_context(); - let location = Location::new(span, self.file); - let mut interpreter = self.setup_interpreter(location); + let mut interpreter = self.setup_interpreter(); let value = interpreter.evaluate_block(block); let (id, typ) = self.inline_comptime_value(value, span); @@ -830,7 +829,7 @@ impl<'context> Elaborator<'context> { }; let file = self.file; - let mut interpreter = self.setup_interpreter(location); + let mut interpreter = self.setup_interpreter(); let mut comptime_args = Vec::new(); let mut errors = Vec::new(); diff --git a/compiler/noirc_frontend/src/elaborator/mod.rs b/compiler/noirc_frontend/src/elaborator/mod.rs index fbedf86e3ef..f8895afba89 100644 --- a/compiler/noirc_frontend/src/elaborator/mod.rs +++ b/compiler/noirc_frontend/src/elaborator/mod.rs @@ -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: Vec, } #[derive(Default)] @@ -214,6 +216,7 @@ impl<'context> Elaborator<'context> { unresolved_globals: BTreeMap::new(), enable_arithmetic_generics, current_trait: None, + interpreter_call_stack: Vec::new(), } } @@ -1313,7 +1316,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(location); + let mut interpreter = self.setup_interpreter(); if let Err(error) = interpreter.evaluate_let(let_statement) { self.errors.push(error.into_compilation_error_pair()); diff --git a/compiler/noirc_frontend/src/elaborator/patterns.rs b/compiler/noirc_frontend/src/elaborator/patterns.rs index d71d85e11d3..06c153d4c10 100644 --- a/compiler/noirc_frontend/src/elaborator/patterns.rs +++ b/compiler/noirc_frontend/src/elaborator/patterns.rs @@ -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(definition.location); + let mut interpreter = self.setup_interpreter(); let value = interpreter.evaluate(id); return self.inline_comptime_value(value, span); } diff --git a/compiler/noirc_frontend/src/elaborator/statements.rs b/compiler/noirc_frontend/src/elaborator/statements.rs index 2da87e39f38..0bb8641b6b3 100644 --- a/compiler/noirc_frontend/src/elaborator/statements.rs +++ b/compiler/noirc_frontend/src/elaborator/statements.rs @@ -440,8 +440,7 @@ impl<'context> Elaborator<'context> { let span = statement.span; let (hir_statement, _typ) = self.elaborate_statement(statement); self.check_and_pop_function_context(); - let location = Location::new(span, self.file); - let mut interpreter = self.setup_interpreter(location); + let mut interpreter = self.setup_interpreter(); let value = interpreter.evaluate_statement(hir_statement); let (expr, typ) = self.inline_comptime_value(value, span); diff --git a/compiler/noirc_frontend/src/hir/comptime/interpreter.rs b/compiler/noirc_frontend/src/hir/comptime/interpreter.rs index 747b00b6077..4f63706f0dc 100644 --- a/compiler/noirc_frontend/src/hir/comptime/interpreter.rs +++ b/compiler/noirc_frontend/src/hir/comptime/interpreter.rs @@ -60,8 +60,6 @@ pub struct Interpreter<'local, 'interner> { /// 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>, - - call_stack: Vec, } #[allow(unused)] @@ -70,12 +68,10 @@ impl<'local, 'interner> Interpreter<'local, 'interner> { elaborator: &'local mut Elaborator<'interner>, crate_id: CrateId, current_function: Option, - location: Location, ) -> Self { let bound_generics = Vec::new(); let in_loop = false; - let call_stack = vec![location]; - Self { elaborator, crate_id, current_function, bound_generics, in_loop, call_stack } + Self { elaborator, crate_id, current_function, bound_generics, in_loop } } pub(crate) fn call_function( @@ -104,11 +100,11 @@ impl<'local, 'interner> Interpreter<'local, 'interner> { } self.remember_bindings(&instantiation_bindings, &impl_bindings); - self.call_stack.push(location); + self.elaborator.interpreter_call_stack.push(location); let result = self.call_function_inner(function, arguments, location); - self.call_stack.pop(); + self.elaborator.interpreter_call_stack.pop(); undo_instantiation_bindings(impl_bindings); undo_instantiation_bindings(instantiation_bindings); self.rebind_generics_from_previous_function(); @@ -1470,7 +1466,7 @@ 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()); - let call_stack = self.call_stack.clone(); + let call_stack = self.elaborator.interpreter_call_stack.clone(); Err(InterpreterError::FailingConstraint { location, message, call_stack }) } value => { diff --git a/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs b/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs index fc4215e8537..3d3ffc88fda 100644 --- a/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs +++ b/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs @@ -49,6 +49,7 @@ impl<'local, 'context> Interpreter<'local, 'context> { location: Location, ) -> IResult { 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), @@ -102,11 +103,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, &self.call_stack), - "slice_pop_front" => slice_pop_front(interner, arguments, location, &self.call_stack), + "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, &self.call_stack), + "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), diff --git a/compiler/noirc_frontend/src/hir/comptime/tests.rs b/compiler/noirc_frontend/src/hir/comptime/tests.rs index 53741be804e..4c1adf9fca0 100644 --- a/compiler/noirc_frontend/src/hir/comptime/tests.rs +++ b/compiler/noirc_frontend/src/hir/comptime/tests.rs @@ -48,7 +48,7 @@ fn interpret_helper(src: &str) -> Result { Elaborator::elaborate_and_return_self(&mut context, krate, collector.items, None, false); assert_eq!(elaborator.errors.len(), 0); - let mut interpreter = elaborator.setup_interpreter(location); + let mut interpreter = elaborator.setup_interpreter(); let no_location = Location::dummy(); interpreter.call_function(main, Vec::new(), HashMap::new(), no_location) From 39127c9761c5c960f5461033f533ad258db0c0b9 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Wed, 28 Aug 2024 15:40:50 -0300 Subject: [PATCH 08/12] Use `im::Vector` for the call stack --- compiler/noirc_frontend/src/hir/comptime/errors.rs | 2 +- compiler/noirc_frontend/src/hir/comptime/interpreter.rs | 2 +- compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/compiler/noirc_frontend/src/hir/comptime/errors.rs b/compiler/noirc_frontend/src/hir/comptime/errors.rs index 2269297295a..cfee6bcedac 100644 --- a/compiler/noirc_frontend/src/hir/comptime/errors.rs +++ b/compiler/noirc_frontend/src/hir/comptime/errors.rs @@ -56,7 +56,7 @@ pub enum InterpreterError { FailingConstraint { message: Option, location: Location, - call_stack: Vec, + call_stack: im::Vector, }, NoMethodFound { name: String, diff --git a/compiler/noirc_frontend/src/hir/comptime/interpreter.rs b/compiler/noirc_frontend/src/hir/comptime/interpreter.rs index 4f63706f0dc..b5a9632253d 100644 --- a/compiler/noirc_frontend/src/hir/comptime/interpreter.rs +++ b/compiler/noirc_frontend/src/hir/comptime/interpreter.rs @@ -1466,7 +1466,7 @@ 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()); - let call_stack = self.elaborator.interpreter_call_stack.clone(); + let call_stack = im::Vector::from(self.elaborator.interpreter_call_stack.clone()); Err(InterpreterError::FailingConstraint { location, message, call_stack }) } value => { diff --git a/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs b/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs index 3d3ffc88fda..384928fe884 100644 --- a/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs +++ b/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs @@ -154,7 +154,7 @@ fn failing_constraint( Err(InterpreterError::FailingConstraint { message: Some(message.into()), location, - call_stack: call_stack.to_vec(), + call_stack: im::Vector::from(call_stack), }) } From 350e95602719de068c07da9f35f8030311f9fb9e Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Wed, 28 Aug 2024 17:09:51 -0300 Subject: [PATCH 09/12] Use im::Vector for call stack --- compiler/noirc_frontend/src/elaborator/mod.rs | 4 ++-- .../noirc_frontend/src/hir/comptime/interpreter.rs | 4 ++-- .../src/hir/comptime/interpreter/builtin.rs | 10 +++++----- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/compiler/noirc_frontend/src/elaborator/mod.rs b/compiler/noirc_frontend/src/elaborator/mod.rs index f8895afba89..0364ed3d290 100644 --- a/compiler/noirc_frontend/src/elaborator/mod.rs +++ b/compiler/noirc_frontend/src/elaborator/mod.rs @@ -169,7 +169,7 @@ pub struct Elaborator<'context> { /// Temporary flag to enable the experimental arithmetic generics feature enable_arithmetic_generics: bool, - pub(crate) interpreter_call_stack: Vec, + pub(crate) interpreter_call_stack: im::Vector, } #[derive(Default)] @@ -216,7 +216,7 @@ impl<'context> Elaborator<'context> { unresolved_globals: BTreeMap::new(), enable_arithmetic_generics, current_trait: None, - interpreter_call_stack: Vec::new(), + interpreter_call_stack: im::Vector::new(), } } diff --git a/compiler/noirc_frontend/src/hir/comptime/interpreter.rs b/compiler/noirc_frontend/src/hir/comptime/interpreter.rs index b5a9632253d..6dae955f8bc 100644 --- a/compiler/noirc_frontend/src/hir/comptime/interpreter.rs +++ b/compiler/noirc_frontend/src/hir/comptime/interpreter.rs @@ -100,11 +100,11 @@ impl<'local, 'interner> Interpreter<'local, 'interner> { } self.remember_bindings(&instantiation_bindings, &impl_bindings); - self.elaborator.interpreter_call_stack.push(location); + self.elaborator.interpreter_call_stack.push_back(location); let result = self.call_function_inner(function, arguments, location); - self.elaborator.interpreter_call_stack.pop(); + self.elaborator.interpreter_call_stack.pop_back(); undo_instantiation_bindings(impl_bindings); undo_instantiation_bindings(instantiation_bindings); self.rebind_generics_from_previous_function(); diff --git a/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs b/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs index 384928fe884..6693796e09c 100644 --- a/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs +++ b/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs @@ -149,12 +149,12 @@ impl<'local, 'context> Interpreter<'local, 'context> { fn failing_constraint( message: impl Into, location: Location, - call_stack: &[Location], + call_stack: &im::Vector, ) -> IResult { Err(InterpreterError::FailingConstraint { message: Some(message.into()), location, - call_stack: im::Vector::from(call_stack), + call_stack: call_stack.clone(), }) } @@ -287,7 +287,7 @@ fn slice_remove( interner: &mut NodeInterner, arguments: Vec<(Value, Location)>, location: Location, - call_stack: &[Location], + call_stack: &im::Vector, ) -> IResult { let (slice, index) = check_two_arguments(arguments, location)?; @@ -326,7 +326,7 @@ fn slice_pop_front( interner: &mut NodeInterner, arguments: Vec<(Value, Location)>, location: Location, - call_stack: &[Location], + call_stack: &im::Vector, ) -> IResult { let argument = check_one_argument(arguments, location)?; @@ -341,7 +341,7 @@ fn slice_pop_back( interner: &mut NodeInterner, arguments: Vec<(Value, Location)>, location: Location, - call_stack: &[Location], + call_stack: &im::Vector, ) -> IResult { let argument = check_one_argument(arguments, location)?; From 43d1d728bf884976be4b0f77cf469ae62d34ad44 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Wed, 28 Aug 2024 17:10:35 -0300 Subject: [PATCH 10/12] clippy --- compiler/noirc_frontend/src/hir/comptime/interpreter.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/noirc_frontend/src/hir/comptime/interpreter.rs b/compiler/noirc_frontend/src/hir/comptime/interpreter.rs index 6dae955f8bc..4980045c68d 100644 --- a/compiler/noirc_frontend/src/hir/comptime/interpreter.rs +++ b/compiler/noirc_frontend/src/hir/comptime/interpreter.rs @@ -1466,7 +1466,7 @@ 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()); - let call_stack = im::Vector::from(self.elaborator.interpreter_call_stack.clone()); + let call_stack = self.elaborator.interpreter_call_stack.clone(); Err(InterpreterError::FailingConstraint { location, message, call_stack }) } value => { From 7cdf11677ea4f7f19f7614c53822d1fbc42a34e3 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Thu, 29 Aug 2024 07:33:22 -0300 Subject: [PATCH 11/12] Carry interpreter call stack throughout all the compilation --- Cargo.lock | 1 + aztec_macros/Cargo.toml | 1 + aztec_macros/src/utils/hir_utils.rs | 8 ++++++-- compiler/noirc_frontend/src/elaborator/comptime.rs | 1 + compiler/noirc_frontend/src/elaborator/mod.rs | 11 +++++++++-- compiler/noirc_frontend/src/hir/comptime/tests.rs | 11 +++++++++-- .../noirc_frontend/src/hir/def_collector/dc_crate.rs | 2 ++ 7 files changed, 29 insertions(+), 6 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index f78fbfede27..3e689a8920c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -451,6 +451,7 @@ version = "0.33.0" dependencies = [ "acvm", "convert_case 0.6.0", + "im", "iter-extended", "noirc_errors", "noirc_frontend", diff --git a/aztec_macros/Cargo.toml b/aztec_macros/Cargo.toml index c9d88e36e28..258379cd7b8 100644 --- a/aztec_macros/Cargo.toml +++ b/aztec_macros/Cargo.toml @@ -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"] } diff --git a/aztec_macros/src/utils/hir_utils.rs b/aztec_macros/src/utils/hir_utils.rs index 0a8ce371708..29f76e2611c 100644 --- a/aztec_macros/src/utils/hir_utils.rs +++ b/aztec_macros/src/utils/hir_utils.rs @@ -195,7 +195,9 @@ pub fn inject_fn( let trait_id = None; items.functions.push(UnresolvedFunctions { file_id, functions, trait_id, self_type: None }); - let mut errors = Elaborator::elaborate(context, *crate_id, items, None, false); + let interpreter_call_stack = &mut im::Vector::new(); + let mut errors = + Elaborator::elaborate(context, *crate_id, items, None, false, interpreter_call_stack); errors.retain(|(error, _)| !CustomDiagnostic::from(error).is_warning()); if !errors.is_empty() { @@ -241,7 +243,9 @@ pub fn inject_global( let mut items = CollectedItems::default(); items.globals.push(UnresolvedGlobal { file_id, module_id, global_id, stmt_def: global }); - let _errors = Elaborator::elaborate(context, *crate_id, items, None, false); + let interpreter_call_stack = &mut im::Vector::new(); + let _errors = + Elaborator::elaborate(context, *crate_id, items, None, false, interpreter_call_stack); } pub fn fully_qualified_note_path(context: &HirContext, note_id: StructId) -> Option { diff --git a/compiler/noirc_frontend/src/elaborator/comptime.rs b/compiler/noirc_frontend/src/elaborator/comptime.rs index 01b4585640f..8118c210545 100644 --- a/compiler/noirc_frontend/src/elaborator/comptime.rs +++ b/compiler/noirc_frontend/src/elaborator/comptime.rs @@ -46,6 +46,7 @@ impl<'context> Elaborator<'context> { self.crate_id, self.debug_comptime_in_file, self.enable_arithmetic_generics, + self.interpreter_call_stack, ); elaborator.function_context.push(FunctionContext::default()); diff --git a/compiler/noirc_frontend/src/elaborator/mod.rs b/compiler/noirc_frontend/src/elaborator/mod.rs index 0364ed3d290..1105d81753d 100644 --- a/compiler/noirc_frontend/src/elaborator/mod.rs +++ b/compiler/noirc_frontend/src/elaborator/mod.rs @@ -169,7 +169,7 @@ pub struct Elaborator<'context> { /// Temporary flag to enable the experimental arithmetic generics feature enable_arithmetic_generics: bool, - pub(crate) interpreter_call_stack: im::Vector, + pub(crate) interpreter_call_stack: &'context mut im::Vector, } #[derive(Default)] @@ -193,6 +193,7 @@ impl<'context> Elaborator<'context> { crate_id: CrateId, debug_comptime_in_file: Option, enable_arithmetic_generics: bool, + interpreter_call_stack: &'context mut im::Vector, ) -> Self { Self { scopes: ScopeForest::default(), @@ -216,7 +217,7 @@ impl<'context> Elaborator<'context> { unresolved_globals: BTreeMap::new(), enable_arithmetic_generics, current_trait: None, - interpreter_call_stack: im::Vector::new(), + interpreter_call_stack, } } @@ -225,6 +226,7 @@ impl<'context> Elaborator<'context> { crate_id: CrateId, debug_comptime_in_file: Option, enable_arithmetic_generics: bool, + interpreter_call_stack: &'context mut im::Vector, ) -> Self { Self::new( &mut context.def_interner, @@ -232,6 +234,7 @@ impl<'context> Elaborator<'context> { crate_id, debug_comptime_in_file, enable_arithmetic_generics, + interpreter_call_stack, ) } @@ -241,6 +244,7 @@ impl<'context> Elaborator<'context> { items: CollectedItems, debug_comptime_in_file: Option, enable_arithmetic_generics: bool, + interpreter_call_stack: &'context mut im::Vector, ) -> Vec<(CompilationError, FileId)> { Self::elaborate_and_return_self( context, @@ -248,6 +252,7 @@ impl<'context> Elaborator<'context> { items, debug_comptime_in_file, enable_arithmetic_generics, + interpreter_call_stack, ) .errors } @@ -258,12 +263,14 @@ impl<'context> Elaborator<'context> { items: CollectedItems, debug_comptime_in_file: Option, enable_arithmetic_generics: bool, + interpreter_call_stack: &'context mut im::Vector, ) -> Self { let mut this = Self::from_context( context, crate_id, debug_comptime_in_file, enable_arithmetic_generics, + interpreter_call_stack, ); this.elaborate_items(items); this.check_and_pop_function_context(); diff --git a/compiler/noirc_frontend/src/hir/comptime/tests.rs b/compiler/noirc_frontend/src/hir/comptime/tests.rs index 4c1adf9fca0..5ec88c8efe2 100644 --- a/compiler/noirc_frontend/src/hir/comptime/tests.rs +++ b/compiler/noirc_frontend/src/hir/comptime/tests.rs @@ -44,8 +44,15 @@ fn interpret_helper(src: &str) -> Result { context.def_maps.insert(krate, collector.def_map); let main = context.get_main_function(&krate).expect("Expected 'main' function"); - let mut elaborator = - Elaborator::elaborate_and_return_self(&mut context, krate, collector.items, None, false); + let interpreter_call_stack = &mut im::Vector::new(); + let mut elaborator = Elaborator::elaborate_and_return_self( + &mut context, + krate, + collector.items, + None, + false, + interpreter_call_stack, + ); assert_eq!(elaborator.errors.len(), 0); let mut interpreter = elaborator.setup_interpreter(); diff --git a/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs index a961de628a8..1865907d307 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs @@ -395,12 +395,14 @@ impl DefCollector { }) }); + let interpreter_call_stack = &mut im::Vector::new(); let mut more_errors = Elaborator::elaborate( context, crate_id, def_collector.items, debug_comptime_in_file, enable_arithmetic_generics, + interpreter_call_stack, ); errors.append(&mut more_errors); From 372d6930e8fab3e479b6748347359af09dfcce1b Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Thu, 29 Aug 2024 11:53:20 -0300 Subject: [PATCH 12/12] Don't keep a reference to the interpreter call stack --- aztec_macros/src/utils/hir_utils.rs | 8 ++------ compiler/noirc_frontend/src/elaborator/comptime.rs | 2 +- compiler/noirc_frontend/src/elaborator/mod.rs | 11 +++-------- compiler/noirc_frontend/src/hir/comptime/tests.rs | 11 ++--------- .../noirc_frontend/src/hir/def_collector/dc_crate.rs | 2 -- 5 files changed, 8 insertions(+), 26 deletions(-) diff --git a/aztec_macros/src/utils/hir_utils.rs b/aztec_macros/src/utils/hir_utils.rs index 29f76e2611c..0a8ce371708 100644 --- a/aztec_macros/src/utils/hir_utils.rs +++ b/aztec_macros/src/utils/hir_utils.rs @@ -195,9 +195,7 @@ pub fn inject_fn( let trait_id = None; items.functions.push(UnresolvedFunctions { file_id, functions, trait_id, self_type: None }); - let interpreter_call_stack = &mut im::Vector::new(); - let mut errors = - Elaborator::elaborate(context, *crate_id, items, None, false, interpreter_call_stack); + let mut errors = Elaborator::elaborate(context, *crate_id, items, None, false); errors.retain(|(error, _)| !CustomDiagnostic::from(error).is_warning()); if !errors.is_empty() { @@ -243,9 +241,7 @@ pub fn inject_global( let mut items = CollectedItems::default(); items.globals.push(UnresolvedGlobal { file_id, module_id, global_id, stmt_def: global }); - let interpreter_call_stack = &mut im::Vector::new(); - let _errors = - Elaborator::elaborate(context, *crate_id, items, None, false, interpreter_call_stack); + let _errors = Elaborator::elaborate(context, *crate_id, items, None, false); } pub fn fully_qualified_note_path(context: &HirContext, note_id: StructId) -> Option { diff --git a/compiler/noirc_frontend/src/elaborator/comptime.rs b/compiler/noirc_frontend/src/elaborator/comptime.rs index 8118c210545..12099b556b7 100644 --- a/compiler/noirc_frontend/src/elaborator/comptime.rs +++ b/compiler/noirc_frontend/src/elaborator/comptime.rs @@ -46,7 +46,7 @@ impl<'context> Elaborator<'context> { self.crate_id, self.debug_comptime_in_file, self.enable_arithmetic_generics, - self.interpreter_call_stack, + self.interpreter_call_stack.clone(), ); elaborator.function_context.push(FunctionContext::default()); diff --git a/compiler/noirc_frontend/src/elaborator/mod.rs b/compiler/noirc_frontend/src/elaborator/mod.rs index 1105d81753d..a41cc0c43df 100644 --- a/compiler/noirc_frontend/src/elaborator/mod.rs +++ b/compiler/noirc_frontend/src/elaborator/mod.rs @@ -169,7 +169,7 @@ pub struct Elaborator<'context> { /// Temporary flag to enable the experimental arithmetic generics feature enable_arithmetic_generics: bool, - pub(crate) interpreter_call_stack: &'context mut im::Vector, + pub(crate) interpreter_call_stack: im::Vector, } #[derive(Default)] @@ -193,7 +193,7 @@ impl<'context> Elaborator<'context> { crate_id: CrateId, debug_comptime_in_file: Option, enable_arithmetic_generics: bool, - interpreter_call_stack: &'context mut im::Vector, + interpreter_call_stack: im::Vector, ) -> Self { Self { scopes: ScopeForest::default(), @@ -226,7 +226,6 @@ impl<'context> Elaborator<'context> { crate_id: CrateId, debug_comptime_in_file: Option, enable_arithmetic_generics: bool, - interpreter_call_stack: &'context mut im::Vector, ) -> Self { Self::new( &mut context.def_interner, @@ -234,7 +233,7 @@ impl<'context> Elaborator<'context> { crate_id, debug_comptime_in_file, enable_arithmetic_generics, - interpreter_call_stack, + im::Vector::new(), ) } @@ -244,7 +243,6 @@ impl<'context> Elaborator<'context> { items: CollectedItems, debug_comptime_in_file: Option, enable_arithmetic_generics: bool, - interpreter_call_stack: &'context mut im::Vector, ) -> Vec<(CompilationError, FileId)> { Self::elaborate_and_return_self( context, @@ -252,7 +250,6 @@ impl<'context> Elaborator<'context> { items, debug_comptime_in_file, enable_arithmetic_generics, - interpreter_call_stack, ) .errors } @@ -263,14 +260,12 @@ impl<'context> Elaborator<'context> { items: CollectedItems, debug_comptime_in_file: Option, enable_arithmetic_generics: bool, - interpreter_call_stack: &'context mut im::Vector, ) -> Self { let mut this = Self::from_context( context, crate_id, debug_comptime_in_file, enable_arithmetic_generics, - interpreter_call_stack, ); this.elaborate_items(items); this.check_and_pop_function_context(); diff --git a/compiler/noirc_frontend/src/hir/comptime/tests.rs b/compiler/noirc_frontend/src/hir/comptime/tests.rs index 5ec88c8efe2..4c1adf9fca0 100644 --- a/compiler/noirc_frontend/src/hir/comptime/tests.rs +++ b/compiler/noirc_frontend/src/hir/comptime/tests.rs @@ -44,15 +44,8 @@ fn interpret_helper(src: &str) -> Result { context.def_maps.insert(krate, collector.def_map); let main = context.get_main_function(&krate).expect("Expected 'main' function"); - let interpreter_call_stack = &mut im::Vector::new(); - let mut elaborator = Elaborator::elaborate_and_return_self( - &mut context, - krate, - collector.items, - None, - false, - interpreter_call_stack, - ); + let mut elaborator = + Elaborator::elaborate_and_return_self(&mut context, krate, collector.items, None, false); assert_eq!(elaborator.errors.len(), 0); let mut interpreter = elaborator.setup_interpreter(); diff --git a/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs index 1865907d307..a961de628a8 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs @@ -395,14 +395,12 @@ impl DefCollector { }) }); - let interpreter_call_stack = &mut im::Vector::new(); let mut more_errors = Elaborator::elaborate( context, crate_id, def_collector.items, debug_comptime_in_file, enable_arithmetic_generics, - interpreter_call_stack, ); errors.append(&mut more_errors);