From aa743817295474ffe64803f0019748143dca8490 Mon Sep 17 00:00:00 2001 From: Serena Duncan <57880238+smd21@users.noreply.github.com> Date: Tue, 6 Aug 2024 13:17:59 -0400 Subject: [PATCH] Cider DAP Breakpoints (#2245) This PR implements breakpoints for the cider debug adapter. It ensures only valid breakpoints are sent to the debugger and flags invalid breakpoint placements. It also ensures that at any given time the debugger has an updated list of valid breakpoints that have been added to the vscode interface. --------- Co-authored-by: Serena --- cider-dap/src/adapter.rs | 129 ++++++++++++++----- cider-dap/src/main.rs | 4 +- interp/src/debugger/debugger_core.rs | 33 ++++- interp/src/debugger/source/new_metadata.pest | 4 +- interp/src/debugger/source/new_parser.rs | 18 +-- interp/src/debugger/source/structures.rs | 17 ++- 6 files changed, 150 insertions(+), 55 deletions(-) diff --git a/cider-dap/src/adapter.rs b/cider-dap/src/adapter.rs index 853a31be37..469e8449fb 100644 --- a/cider-dap/src/adapter.rs +++ b/cider-dap/src/adapter.rs @@ -2,9 +2,10 @@ use crate::error::AdapterResult; use dap::types::{ Breakpoint, Scope, Source, SourceBreakpoint, StackFrame, Thread, Variable, }; +use interp::debugger::commands::ParsedGroupName; use interp::debugger::source::structures::NewSourceMap; use interp::debugger::OwnedDebugger; -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use std::path::PathBuf; pub struct MyAdapter { @@ -13,9 +14,9 @@ pub struct MyAdapter { break_count: Counter, thread_count: Counter, stack_count: Counter, - breakpoints: Vec<(Source, i64)>, // This field is a placeholder - stack_frames: Vec, // This field is a placeholder - threads: Vec, // This field is a placeholder + breakpoints: HashSet, + stack_frames: Vec, // This field is a placeholder + threads: Vec, // This field is a placeholder object_references: HashMap>, source: String, ids: NewSourceMap, @@ -30,7 +31,7 @@ impl MyAdapter { break_count: Counter::new(), thread_count: Counter::new(), stack_count: Counter::new(), - breakpoints: Vec::new(), + breakpoints: HashSet::new(), stack_frames: Vec::new(), threads: Vec::new(), object_references: HashMap::new(), @@ -38,31 +39,80 @@ impl MyAdapter { ids: metadata, }) } - - ///Set breakpoints for adapter - pub fn set_breakpoint( + /// function to deal with setting breakpoints and updating debugger accordingly + pub fn handle_breakpoint( &mut self, path: Source, - source: &Vec, + points: &Vec, ) -> Vec { - //Keep all the new breakpoints made - let mut out_vec: Vec = vec![]; + // helper method to get diffs since it yelled at me about borrows when i had it in main method + fn calc_diffs( + new_set: &HashSet, + old_set: &HashSet, + ) -> (HashSet, HashSet) { + let to_set: HashSet = + new_set.difference(old_set).copied().collect(); + let to_delete: HashSet = + old_set.difference(new_set).copied().collect(); + (to_set, to_delete) + } + + //check diffs + let mut new_point_set = HashSet::new(); + for p in points { + new_point_set.insert(p.line); + } + let (to_set, to_delete) = calc_diffs(&new_point_set, &self.breakpoints); + + //update adapter + self.breakpoints.clear(); + + let mut to_debugger_set: Vec = vec![]; + let mut to_client: Vec = vec![]; + + // iterate over points received in request + for source_point in points { + self.breakpoints.insert(source_point.line); + let name = self.ids.lookup_line(source_point.line as u64); - //Loop over all breakpoints - why do we need to loop over all of them? is it bc input vec isnt mutable? - for source_point in source { - self.breakpoints.push((path.clone(), source_point.line)); - //Create new Breakpoint instance let breakpoint = make_breakpoint( self.break_count.increment().into(), - true, + name.is_some(), Some(path.clone()), Some(source_point.line), ); + to_client.push(breakpoint); - out_vec.push(breakpoint); + if let Some((component, group)) = name { + if to_set.contains(&source_point.line) { + to_debugger_set.push(ParsedGroupName::from_comp_and_group( + component.clone(), + group.clone(), + )) + } + } } - //push breakpoints to cider debugger once have go ahead - out_vec + //send ones to set to debugger + self.debugger.set_breakpoints(to_debugger_set); + //delete from debugger + self.delete_breakpoints(to_delete); + + //return list of created points to client + to_client + } + /// handles deleting breakpoints in the debugger + fn delete_breakpoints(&mut self, to_delete: HashSet) { + let mut to_debugger: Vec = vec![]; + for point in to_delete { + let name = self.ids.lookup_line(point as u64); + if let Some((component, group)) = name { + to_debugger.push(ParsedGroupName::from_comp_and_group( + component.clone(), + group.clone(), + )) + } + } + self.debugger.delete_breakpoints(to_debugger); } ///Creates a thread using the parameter name. @@ -132,7 +182,7 @@ impl MyAdapter { // the code for now goes to the line of the last group running in the map, should deal // with this in the future for when groups run in parallel. for id in map { - let value = self.ids.lookup(id.to_string()).unwrap().start_line; + let value = self.ids.lookup(id).unwrap().start_line; line_number = value; } // Set line of the stack frame and tell debugger we're not finished. @@ -221,23 +271,36 @@ impl Counter { /// This function takes in relevant fields in Breakpoint that are used /// by the adapter. This is subject to change. pub fn make_breakpoint( - //probably add debugger call here? id: Option, verified: bool, source: Option, line: Option, ) -> Breakpoint { - println!("bkpt"); - Breakpoint { - id, - verified, - message: None, - source, - line, - column: None, - end_line: None, - end_column: None, - instruction_reference: None, - offset: None, + if verified { + Breakpoint { + id, + verified, + message: None, + source, + line, + column: None, + end_line: None, + end_column: None, + instruction_reference: None, + offset: None, + } + } else { + Breakpoint { + id, + verified, + message: Some(String::from("Invalid placement for breakpoint")), + source, + line, + column: None, + end_line: None, + end_column: None, + instruction_reference: None, + offset: None, + } } } diff --git a/cider-dap/src/main.rs b/cider-dap/src/main.rs index fae32fbebd..1f8131a987 100644 --- a/cider-dap/src/main.rs +++ b/cider-dap/src/main.rs @@ -200,9 +200,9 @@ fn run_server( Command::SetBreakpoints(args) => { // Add breakpoints - if let Some(breakpoint) = &args.breakpoints { + if let Some(bkpts) = &args.breakpoints { let out = - adapter.set_breakpoint(args.source.clone(), breakpoint); + adapter.handle_breakpoint(args.source.clone(), bkpts); // Success let rsp = req.success(ResponseBody::SetBreakpoints( diff --git a/interp/src/debugger/debugger_core.rs b/interp/src/debugger/debugger_core.rs index c1f89de7a7..ed12bd9f89 100644 --- a/interp/src/debugger/debugger_core.rs +++ b/interp/src/debugger/debugger_core.rs @@ -1,5 +1,5 @@ use super::{ - commands::{Command, PrintMode}, + commands::{Command, ParsedBreakPointID, ParsedGroupName, PrintMode}, debugging_context::context::DebuggingContext, io_utils::Input, source::structures::NewSourceMap, @@ -20,6 +20,7 @@ use crate::{ use std::{collections::HashSet, path::PathBuf, rc::Rc}; +use itertools::Itertools; use owo_colors::OwoColorize; use std::path::Path as FilePath; @@ -31,14 +32,14 @@ pub(super) const SPACING: &str = " "; /// is finished or not. If program is done then the debugger is exited pub struct ProgramStatus { /// all groups currently running - status: HashSet, + status: HashSet<(String, String)>, /// states whether the program has finished done: bool, } impl ProgramStatus { /// get status - pub fn get_status(&self) -> &HashSet { + pub fn get_status(&self) -> &HashSet<(String, String)> { &self.status } @@ -113,7 +114,20 @@ impl + Clone> Debugger { status: self .interpreter .get_currently_running_groups() - .map(|x| self.program_context.as_ref().lookup_name(x).clone()) + .map(|x| { + let group_name = + self.program_context.as_ref().lookup_name(x).clone(); + let parent_comp = self + .program_context + .as_ref() + .get_component_from_group(x); + let parent_name = self + .program_context + .as_ref() + .lookup_name(parent_comp) + .clone(); + (parent_name, group_name) + }) .collect(), done: self.interpreter.is_done(), } @@ -132,6 +146,17 @@ impl + Clone> Debugger { Ok(self.status()) } + pub fn set_breakpoints(&mut self, breakpoints: Vec) { + self.create_breakpoints(breakpoints) + } + + pub fn delete_breakpoints(&mut self, breakpoints: Vec) { + let parsed_bp_ids: Vec = breakpoints + .into_iter() + .map(ParsedBreakPointID::from) + .collect_vec(); + self.manipulate_breakpoint(Command::Delete(parsed_bp_ids)); + } #[inline] fn do_step(&mut self, n: u32) -> InterpreterResult<()> { for _ in 0..n { diff --git a/interp/src/debugger/source/new_metadata.pest b/interp/src/debugger/source/new_metadata.pest index a873c460c5..9acd1ff103 100644 --- a/interp/src/debugger/source/new_metadata.pest +++ b/interp/src/debugger/source/new_metadata.pest @@ -3,10 +3,10 @@ dot = _{ "." } colon = _{ ":" } ident_syms = _{ "_" | "-" | "'" } num = @{ ASCII_DIGIT+ } -group_name = @{ (ASCII_ALPHA | ASCII_DIGIT | ident_syms | dot)+ } +group_name = @{ (ASCII_ALPHA | ASCII_DIGIT | ident_syms)+ } path = @{ (ASCII_ALPHA | ASCII_DIGIT | ident_syms | dot | "/")+ } -entry = { WHITE_SPACE* ~ group_name ~ colon ~ WHITE_SPACE ~ path ~ WHITE_SPACE ~ num ~ ("-") ~ num ~ (",")* ~ WHITE_SPACE* } +entry = { WHITE_SPACE* ~ group_name ~ dot ~ group_name ~ colon ~ WHITE_SPACE ~ path ~ WHITE_SPACE ~ num ~ ("-") ~ num ~ (",")* ~ WHITE_SPACE* } metadata = { SOI ~ entry* ~ EOI diff --git a/interp/src/debugger/source/new_parser.rs b/interp/src/debugger/source/new_parser.rs index 7a8c79fa5a..170b13fee5 100644 --- a/interp/src/debugger/source/new_parser.rs +++ b/interp/src/debugger/source/new_parser.rs @@ -31,16 +31,16 @@ impl MetadataParser { Ok(input.as_str().to_string()) } - fn entry(input: Node) -> ParseResult<(String, GroupContents)> { + fn entry(input: Node) -> ParseResult<((String, String), GroupContents)> { Ok(match_nodes!(input.into_children(); - [group_name(g), path(p),num(st), num(end)] => (g,GroupContents {path:p, start_line: st, end_line:end}) + [group_name(comp), group_name(grp), path(p),num(st), num(end)] => ((comp, grp),GroupContents {path:p, start_line: st, end_line:end}) )) } fn metadata(input: Node) -> ParseResult { Ok(match_nodes!(input.into_children(); [entry(e).., EOI(_)] => { - let map: HashMap = e.collect(); + let map: HashMap<(String, String), GroupContents> = e.collect(); map.into() } )) @@ -63,9 +63,9 @@ pub fn parse_metadata(input_str: &str) -> InterpreterResult { #[cfg(test)] #[test] fn one_entry() { - let entry = parse_metadata("hello: your/mom 6-9").unwrap(); + let entry = parse_metadata("hello.from: your/mom 6-9").unwrap(); dbg!(&entry); - let tup = entry.lookup(String::from("hello")); + let tup = entry.lookup(&(String::from("hello"), String::from("from"))); assert_eq!( tup.unwrap().clone(), GroupContents { @@ -79,12 +79,12 @@ fn one_entry() { #[test] fn mult_entries() { let entry = parse_metadata( - "wr_reg0: calyx/interp/test/control/reg_seq.futil 10-13, - wr_reg1: calyx/interp/test/control/reg_seq.futil 15-19", + "mom.wr_reg0: calyx/interp/test/control/reg_seq.futil 10-13, + dad.wr_reg1: calyx/interp/test/control/reg_seq.futil 15-19", ) .unwrap(); - let tup = entry.lookup(String::from("wr_reg0")); - let tup2 = entry.lookup(String::from("wr_reg1")); + let tup = entry.lookup(&(String::from("mom"), String::from("wr_reg0"))); + let tup2 = entry.lookup(&(String::from("dad"), String::from("wr_reg1"))); assert_eq!( tup.unwrap().clone(), GroupContents { diff --git a/interp/src/debugger/source/structures.rs b/interp/src/debugger/source/structures.rs index e93205799a..f85c4dd703 100644 --- a/interp/src/debugger/source/structures.rs +++ b/interp/src/debugger/source/structures.rs @@ -31,17 +31,24 @@ pub struct GroupContents { #[derive(Debug, Clone)] /// NewSourceMap contains the group name as the key and the line it lies on with /// as respect to its corresponding .futil file -pub struct NewSourceMap(HashMap); +pub struct NewSourceMap(HashMap<(String, String), GroupContents>); impl NewSourceMap { /// look up group name, if not present, return None - pub fn lookup(&self, key: String) -> Option<&GroupContents> { - self.0.get(&key) + pub fn lookup(&self, key: &(String, String)) -> Option<&GroupContents> { + self.0.get(key) + } + + pub fn lookup_line(&self, line_num: u64) -> Option<(&String, &String)> { + self.0 + .iter() + .find(|(_, v)| v.start_line == line_num) + .map(|(k, _)| (&k.0, &k.1)) } } -impl From> for NewSourceMap { - fn from(i: HashMap) -> Self { +impl From> for NewSourceMap { + fn from(i: HashMap<(String, String), GroupContents>) -> Self { Self(i) } }