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 <[email protected]>

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<StackFrame>,   // This field is a placeholder
-    threads: Vec<Thread>,            // This field is a placeholder
+    breakpoints: HashSet<i64>,
+    stack_frames: Vec<StackFrame>, // This field is a placeholder
+    threads: Vec<Thread>,          // This field is a placeholder
     object_references: HashMap<i64, Vec<String>>,
     source: String,
     ids: NewSourceMap,
@@ -30,39 +31,88 @@ 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(),
             source: path.to_string(),
             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<SourceBreakpoint>,
+        points: &Vec<SourceBreakpoint>,
     ) -> Vec<Breakpoint> {
-        //Keep all the new breakpoints made
-        let mut out_vec: Vec<Breakpoint> = 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<i64>,
+            old_set: &HashSet<i64>,
+        ) -> (HashSet<i64>, HashSet<i64>) {
+            let to_set: HashSet<i64> =
+                new_set.difference(old_set).copied().collect();
+            let to_delete: HashSet<i64> =
+                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<ParsedGroupName> = vec![];
+        let mut to_client: Vec<Breakpoint> = 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<i64>) {
+        let mut to_debugger: Vec<ParsedGroupName> = 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<i64>,
     verified: bool,
     source: Option<Source>,
     line: Option<i64>,
 ) -> 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,
+        }
     }
 }

cider-dap/src/main.rs

@@ -200,9 +200,9 @@ fn run_server<R: Read, W: Write>(
 
             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(

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<String>,
+    status: HashSet<(String, String)>,
     /// states whether the program has finished
     done: bool,
 }
 
 impl ProgramStatus {
     /// get status
-    pub fn get_status(&self) -> &HashSet<String> {
+    pub fn get_status(&self) -> &HashSet<(String, String)> {
         &self.status
     }
 
@@ -113,7 +114,20 @@ impl<C: AsRef<Context> + Clone> Debugger<C> {
             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<C: AsRef<Context> + Clone> Debugger<C> {
         Ok(self.status())
     }
 
+    pub fn set_breakpoints(&mut self, breakpoints: Vec<ParsedGroupName>) {
+        self.create_breakpoints(breakpoints)
+    }
+
+    pub fn delete_breakpoints(&mut self, breakpoints: Vec<ParsedGroupName>) {
+        let parsed_bp_ids: Vec<ParsedBreakPointID> = 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 {

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

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<NewSourceMap> {
         Ok(match_nodes!(input.into_children();
             [entry(e).., EOI(_)] => {
-                let map: HashMap<String, GroupContents> = e.collect();
+                let map: HashMap<(String, String), GroupContents> = e.collect();
                 map.into()
             }
         ))
@@ -63,9 +63,9 @@ pub fn parse_metadata(input_str: &str) -> InterpreterResult<NewSourceMap> {
 #[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 {

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<String, GroupContents>);
+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<HashMap<String, GroupContents>> for NewSourceMap {
-    fn from(i: HashMap<String, GroupContents>) -> Self {
+impl From<HashMap<(String, String), GroupContents>> for NewSourceMap {
+    fn from(i: HashMap<(String, String), GroupContents>) -> Self {
         Self(i)
     }
 }