From de1f687e0c9cc50fcd57e3b39483faf988a399ce Mon Sep 17 00:00:00 2001 From: Ana Perez Ghiglia Date: Tue, 4 Jun 2024 14:35:02 -0600 Subject: [PATCH 1/6] smanilov solution --- tooling/debugger/src/context.rs | 11 +++++ tooling/debugger/src/repl.rs | 74 +++++++++++++++++++++++++++++++++ 2 files changed, 85 insertions(+) diff --git a/tooling/debugger/src/context.rs b/tooling/debugger/src/context.rs index 9dc5c758c6f..243feb48b19 100644 --- a/tooling/debugger/src/context.rs +++ b/tooling/debugger/src/context.rs @@ -178,6 +178,17 @@ impl<'a, B: BlackBoxFunctionSolver> DebugContext<'a, B> { .filter(|v: &Vec| !v.is_empty()) } + /// Returns the `FileId` of the file associated with the innermost function on the call stack. + pub(super) fn get_current_file(&mut self) -> Option { + match self.get_current_source_location() { + Some(locations) => match locations.last() { + Some(location) => Some(location.file), + None => None, + }, + None => None, + } + } + /// Returns the (possible) stack of source locations corresponding to the /// given opcode location. Due to compiler inlining it's possible for this /// function to return multiple source locations. An empty vector means that diff --git a/tooling/debugger/src/repl.rs b/tooling/debugger/src/repl.rs index 5aef12ad8d4..db33817e012 100644 --- a/tooling/debugger/src/repl.rs +++ b/tooling/debugger/src/repl.rs @@ -199,6 +199,70 @@ impl<'a, B: BlackBoxFunctionSolver> ReplDebugger<'a, B> { } } + fn add_breakpoint_at_line(&mut self, mut line_number: usize) { + // Make line number 0-indexed. + line_number -= 1; + + let current_file = match self.context.get_current_file() { + Some(file) => file.clone(), + None => { + println!("No current file."); + return; + } + }; + + let mut best_start: usize = 0; + let mut best_location: Option = None; + + // Helper function that first finds the source locations associated with the given opcode. + // Then, it finds the start and end lines of the source location and + // updates the best_start and best_location temporaries, if the given opcode_location + // matches the desired break-line better than the previous best match. + let mut update_best_match = |opcode_location: OpcodeLocation| { + let source_locations = + self.context.get_source_location_for_opcode_location(&opcode_location); + // Last means inner-most in stack trace. + match source_locations.last() { + Some(source_location) => { + let start = self.debug_artifact.location_line_index(*source_location).unwrap(); + let end = + self.debug_artifact.location_end_line_index(*source_location).unwrap(); + if source_location.file == current_file + && start <= line_number + && line_number <= end + && line_number - start < line_number - best_start + { + best_start = start; + best_location = Some(opcode_location); + } + } + None => (), + } + }; + + let opcodes = self.context.get_opcodes(); + for (acir_index, opcode) in opcodes.iter().enumerate() { + match &opcode { + Opcode::BrilligCall { id, .. } => { + let bytecode = &self.unconstrained_functions[*id as usize].bytecode; + for (brillig_index, ..) in bytecode.iter().enumerate() { + let opcode_location = OpcodeLocation::Brillig { acir_index, brillig_index }; + update_best_match(opcode_location); + } + } + _ => { + println!("Not supported: break-line for {:?}", opcode); + return; + } + } + } + + match best_location { + Some(location) => self.add_breakpoint_at(location), + None => println!("No opcode at line {}", line_number), + } + } + fn delete_breakpoint_at(&mut self, location: OpcodeLocation) { if self.context.delete_breakpoint(&location) { println!("Breakpoint at opcode {location} deleted"); @@ -475,6 +539,16 @@ pub fn run>( } }, ) + .add( + "break", + command! { + "add a breakpoint at a line of the current file", + (line_number: usize) => |line_number| { + ref_context.borrow_mut().add_breakpoint_at_line(line_number); + Ok(CommandStatus::Done) + } + }, + ) .add( "break", command! { From c0e93c8b196cf123338a85355a156e60eed64eae Mon Sep 17 00:00:00 2001 From: Ana Perez Ghiglia Date: Tue, 4 Jun 2024 15:24:33 -0600 Subject: [PATCH 2/6] Simplify solition by using context::find_opcode_for_source_location --- tooling/debugger/src/repl.rs | 53 ++---------------------------------- 1 file changed, 3 insertions(+), 50 deletions(-) diff --git a/tooling/debugger/src/repl.rs b/tooling/debugger/src/repl.rs index db33817e012..494bfbcb0eb 100644 --- a/tooling/debugger/src/repl.rs +++ b/tooling/debugger/src/repl.rs @@ -199,10 +199,7 @@ impl<'a, B: BlackBoxFunctionSolver> ReplDebugger<'a, B> { } } - fn add_breakpoint_at_line(&mut self, mut line_number: usize) { - // Make line number 0-indexed. - line_number -= 1; - + fn add_breakpoint_at_line(&mut self, line_number: i64) { let current_file = match self.context.get_current_file() { Some(file) => file.clone(), None => { @@ -211,51 +208,7 @@ impl<'a, B: BlackBoxFunctionSolver> ReplDebugger<'a, B> { } }; - let mut best_start: usize = 0; - let mut best_location: Option = None; - - // Helper function that first finds the source locations associated with the given opcode. - // Then, it finds the start and end lines of the source location and - // updates the best_start and best_location temporaries, if the given opcode_location - // matches the desired break-line better than the previous best match. - let mut update_best_match = |opcode_location: OpcodeLocation| { - let source_locations = - self.context.get_source_location_for_opcode_location(&opcode_location); - // Last means inner-most in stack trace. - match source_locations.last() { - Some(source_location) => { - let start = self.debug_artifact.location_line_index(*source_location).unwrap(); - let end = - self.debug_artifact.location_end_line_index(*source_location).unwrap(); - if source_location.file == current_file - && start <= line_number - && line_number <= end - && line_number - start < line_number - best_start - { - best_start = start; - best_location = Some(opcode_location); - } - } - None => (), - } - }; - - let opcodes = self.context.get_opcodes(); - for (acir_index, opcode) in opcodes.iter().enumerate() { - match &opcode { - Opcode::BrilligCall { id, .. } => { - let bytecode = &self.unconstrained_functions[*id as usize].bytecode; - for (brillig_index, ..) in bytecode.iter().enumerate() { - let opcode_location = OpcodeLocation::Brillig { acir_index, brillig_index }; - update_best_match(opcode_location); - } - } - _ => { - println!("Not supported: break-line for {:?}", opcode); - return; - } - } - } + let best_location = self.context.find_opcode_for_source_location(¤t_file, line_number); match best_location { Some(location) => self.add_breakpoint_at(location), @@ -543,7 +496,7 @@ pub fn run>( "break", command! { "add a breakpoint at a line of the current file", - (line_number: usize) => |line_number| { + (line_number: i64) => |line_number| { ref_context.borrow_mut().add_breakpoint_at_line(line_number); Ok(CommandStatus::Done) } From 765a7a88ed1f030b06f262c635682183c3435c1b Mon Sep 17 00:00:00 2001 From: Ana Perez Ghiglia Date: Tue, 4 Jun 2024 15:43:46 -0600 Subject: [PATCH 3/6] nitpicks * print added breakpoint at line * simplify get_current_file implementation --- tooling/debugger/src/context.rs | 5 +---- tooling/debugger/src/repl.rs | 5 ++++- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/tooling/debugger/src/context.rs b/tooling/debugger/src/context.rs index 243feb48b19..7538cf4a348 100644 --- a/tooling/debugger/src/context.rs +++ b/tooling/debugger/src/context.rs @@ -181,10 +181,7 @@ impl<'a, B: BlackBoxFunctionSolver> DebugContext<'a, B> { /// Returns the `FileId` of the file associated with the innermost function on the call stack. pub(super) fn get_current_file(&mut self) -> Option { match self.get_current_source_location() { - Some(locations) => match locations.last() { - Some(location) => Some(location.file), - None => None, - }, + Some(locations) => locations.last().map(|location| location.file), None => None, } } diff --git a/tooling/debugger/src/repl.rs b/tooling/debugger/src/repl.rs index 494bfbcb0eb..ea9a7aa089e 100644 --- a/tooling/debugger/src/repl.rs +++ b/tooling/debugger/src/repl.rs @@ -211,7 +211,10 @@ impl<'a, B: BlackBoxFunctionSolver> ReplDebugger<'a, B> { let best_location = self.context.find_opcode_for_source_location(¤t_file, line_number); match best_location { - Some(location) => self.add_breakpoint_at(location), + Some(location) => { + println!("Added breakpoint at line {}", line_number); + self.add_breakpoint_at(location) + }, None => println!("No opcode at line {}", line_number), } } From 2289c3d603f4f1d2ffe3c9d5a6c81cc93f2453f1 Mon Sep 17 00:00:00 2001 From: Ana Perez Ghiglia Date: Tue, 4 Jun 2024 16:17:22 -0600 Subject: [PATCH 4/6] fix cargo clippy run --- tooling/debugger/src/repl.rs | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/tooling/debugger/src/repl.rs b/tooling/debugger/src/repl.rs index ea9a7aa089e..ae7e1de9669 100644 --- a/tooling/debugger/src/repl.rs +++ b/tooling/debugger/src/repl.rs @@ -200,21 +200,19 @@ impl<'a, B: BlackBoxFunctionSolver> ReplDebugger<'a, B> { } fn add_breakpoint_at_line(&mut self, line_number: i64) { - let current_file = match self.context.get_current_file() { - Some(file) => file.clone(), - None => { - println!("No current file."); - return; - } + let Some(current_file) = self.context.get_current_file() else { + println!("No current file."); + return; }; - let best_location = self.context.find_opcode_for_source_location(¤t_file, line_number); + let best_location = + self.context.find_opcode_for_source_location(¤t_file, line_number); match best_location { Some(location) => { println!("Added breakpoint at line {}", line_number); self.add_breakpoint_at(location) - }, + } None => println!("No opcode at line {}", line_number), } } From b6ff40cad0e9cc13b611a7d4b9f553ddc726ad13 Mon Sep 17 00:00:00 2001 From: Ana Perez Ghiglia Date: Wed, 5 Jun 2024 10:04:25 -0600 Subject: [PATCH 5/6] Replace Option match with .map().flatten() --- tooling/debugger/src/context.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/tooling/debugger/src/context.rs b/tooling/debugger/src/context.rs index 7538cf4a348..fd4e93adb5a 100644 --- a/tooling/debugger/src/context.rs +++ b/tooling/debugger/src/context.rs @@ -180,10 +180,9 @@ impl<'a, B: BlackBoxFunctionSolver> DebugContext<'a, B> { /// Returns the `FileId` of the file associated with the innermost function on the call stack. pub(super) fn get_current_file(&mut self) -> Option { - match self.get_current_source_location() { - Some(locations) => locations.last().map(|location| location.file), - None => None, - } + self.get_current_source_location() + .map(|locations| locations.last().map(|location| location.file)) + .flatten() } /// Returns the (possible) stack of source locations corresponding to the From 2c1e15c091d0dd434417a21fe3819842200eef8c Mon Sep 17 00:00:00 2001 From: Ana Perez Ghiglia Date: Wed, 5 Jun 2024 10:10:36 -0600 Subject: [PATCH 6/6] oh! Option.flatmap exists, only with another name.. --- tooling/debugger/src/context.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tooling/debugger/src/context.rs b/tooling/debugger/src/context.rs index fd4e93adb5a..4243ade8cb0 100644 --- a/tooling/debugger/src/context.rs +++ b/tooling/debugger/src/context.rs @@ -181,8 +181,7 @@ impl<'a, B: BlackBoxFunctionSolver> DebugContext<'a, B> { /// Returns the `FileId` of the file associated with the innermost function on the call stack. pub(super) fn get_current_file(&mut self) -> Option { self.get_current_source_location() - .map(|locations| locations.last().map(|location| location.file)) - .flatten() + .and_then(|locations| locations.last().map(|location| location.file)) } /// Returns the (possible) stack of source locations corresponding to the