refactor: generalize more of breakpoint logic for db/rr; TODO: more

moved setBreakpoints logic to handler; implemented some of the methods
and placeholders for the others

Author: alehander92

src/db-backend/src/dap_server.rs

@@ -1,14 +1,14 @@
 use crate::dap::{self, Capabilities, DapMessage, Event, ProtocolMessage, Response};
-use crate::dap_types::{self, Breakpoint, SetBreakpointsArguments, SetBreakpointsResponseBody, Source};
+use crate::dap_types;
 
 use crate::db::Db;
 use crate::handler::{Handler, TraceKind};
 use crate::paths::CODETRACER_PATHS;
 use crate::rr_dispatcher::CtRRArgs;
 use crate::task::{
-    gen_task_id, Action, CallSearchArg, CalltraceLoadArgs, CollapseCallsArgs, CtLoadFlowArguments,
+    Action, CallSearchArg, CalltraceLoadArgs, CollapseCallsArgs, CtLoadFlowArguments,
     CtLoadLocalsArguments, FunctionLocation, LoadHistoryArg, LocalStepJump, Location, ProgramEvent, RunTracepointsArg,
-    SourceCallJumpTarget, SourceLocation, StepArg, Task, TaskKind, TracepointId, UpdateTableArgs,
+    SourceCallJumpTarget, SourceLocation, StepArg, TracepointId, UpdateTableArgs,
 };
 
 use crate::trace_processor::{load_trace_data, load_trace_metadata, TraceProcessor};
@@ -20,7 +20,7 @@ use crate::transport::{DapResult, WorkerTransport};
 
 use log::{info, warn};
 use serde_json::json;
-use std::collections::{HashMap, HashSet};
+// use std::collections::{HashMap, HashSet};
 use std::error::Error;
 use std::fmt;
 
@@ -200,6 +200,7 @@ fn handle_request<T: DapTransport>(
         "stackTrace" => handler.stack_trace(req.clone(), req.load_args::<dap_types::StackTraceArguments>()?)?,
         "variables" => handler.variables(req.clone(), req.load_args::<dap_types::VariablesArguments>()?)?,
         "restart" => handler.run_to_entry(req.clone())?,
+        "setBreakpoints" => handler.set_breakpoints(req.clone(), req.load_args::<dap_types::SetBreakpointsArguments>()?)?,
         "ct/load-locals" => handler.load_locals(req.clone(), req.load_args::<CtLoadLocalsArguments>()?)?,
         "ct/update-table" => handler.update_table(req.clone(), req.load_args::<UpdateTableArgs>()?)?,
         "ct/event-load" => handler.event_load(req.clone())?,
@@ -250,7 +251,6 @@ fn handle_request<T: DapTransport>(
 
 pub struct Ctx {
     pub seq: i64,
-    pub breakpoints: HashMap<String, HashSet<i64>>,
     pub handler: Option<Handler>,
     pub received_launch: bool,
     pub launch_trace_folder: PathBuf,
@@ -264,7 +264,6 @@ impl Default for Ctx {
     fn default() -> Self {
         Self {
             seq: 1i64,
-            breakpoints: HashMap::new(),
             handler: None,
             received_launch: false,
             launch_trace_folder: PathBuf::from(""),
@@ -322,92 +321,6 @@ pub fn handle_message<T: DapTransport>(
             ctx.seq += 1;
             transport.send(&event)?;
         }
-        DapMessage::Request(req) if req.command == "setBreakpoints" => {
-            let mut results = Vec::new();
-            let args = req.load_args::<SetBreakpointsArguments>()?;
-            if let Some(path) = args.source.path.clone() {
-                let lines: Vec<i64> = if let Some(bps) = args.breakpoints {
-                    bps.into_iter().map(|b| b.line).collect()
-                } else {
-                    args.lines.unwrap_or_default()
-                };
-                let entry = ctx.breakpoints.entry(path.clone()).or_default();
-                if let Some(h) = ctx.handler.as_mut() {
-                    h.clear_breakpoints();
-                    for line in lines {
-                        entry.insert(line);
-                        let _ = h.add_breakpoint(
-                            SourceLocation {
-                                path: path.clone(),
-                                line: line as usize,
-                            },
-                            Task {
-                                kind: TaskKind::AddBreak,
-                                id: gen_task_id(TaskKind::AddBreak),
-                            },
-                        );
-                        results.push(Breakpoint {
-                            id: None,
-                            verified: true,
-                            message: None,
-                            source: Some(Source {
-                                name: args.source.name.clone(),
-                                path: Some(path.clone()),
-                                source_reference: args.source.source_reference,
-                                presentation_hint: None,
-                                origin: None,
-                                sources: None,
-                                adapter_data: None,
-                                checksums: None,
-                            }),
-                            line: Some(line),
-                            column: None,
-                            end_line: None,
-                            end_column: None,
-                            instruction_reference: None,
-                            offset: None,
-                            reason: None,
-                        });
-                    }
-                }
-            } else {
-                let lines = args
-                    .breakpoints
-                    .unwrap_or_default()
-                    .into_iter()
-                    .map(|b| b.line)
-                    .collect::<Vec<_>>();
-                for line in lines {
-                    results.push(Breakpoint {
-                        id: None,
-                        verified: false,
-                        message: Some("missing source path".to_string()),
-                        source: None,
-                        line: Some(line),
-                        column: None,
-                        end_line: None,
-                        end_column: None,
-                        instruction_reference: None,
-                        offset: None,
-                        reason: None,
-                    });
-                }
-            }
-            let body = SetBreakpointsResponseBody { breakpoints: results };
-            let resp = DapMessage::Response(Response {
-                base: ProtocolMessage {
-                    seq: ctx.seq,
-                    type_: "response".to_string(),
-                },
-                request_seq: req.base.seq,
-                success: true,
-                command: "setBreakpoints".to_string(),
-                message: None,
-                body: serde_json::to_value(body)?,
-            });
-            ctx.seq += 1;
-            transport.send(&resp)?;
-        }
         DapMessage::Request(req) if req.command == "launch" => {
             ctx.received_launch = true;
             let args = req.load_args::<dap::LaunchRequestArguments>()?;

src/db-backend/src/db.rs

@@ -785,13 +785,14 @@ pub struct DbReplay {
     pub step_id: StepId,
     pub call_key: CallKey,
     pub breakpoint_list: Vec<HashMap<usize, Breakpoint>>,
+    breakpoint_next_id: usize,
 }
 
 impl DbReplay {
     pub fn new(db: Box<Db>) -> DbReplay {
         let mut breakpoint_list: Vec<HashMap<usize, Breakpoint>> = Default::default();
         breakpoint_list.resize_with(db.paths.len(), HashMap::new);
-        DbReplay { db, step_id: StepId(0), call_key: CallKey(0), breakpoint_list }
+        DbReplay { db, step_id: StepId(0), call_key: CallKey(0), breakpoint_list, breakpoint_next_id: 0 }
     }
 
     pub fn step_id_jump(&mut self, step_id: StepId) {
@@ -883,11 +884,11 @@ impl DbReplay {
     fn step_continue(&mut self, forward: bool) -> Result<bool, Box<dyn Error>> {
         for step in self.db.step_from(self.step_id, forward) {
             if !self.breakpoint_list.is_empty() {
-                if let Some(is_active) = self.breakpoint_list[step.path_id.0]
+                if let Some(enabled) = self.breakpoint_list[step.path_id.0]
                     .get(&step.line.into())
-                    .map(|bp| bp.is_active)
+                    .map(|bp| bp.enabled)
                 {
-                    if is_active {
+                    if enabled {
                         self.step_id_jump(step.step_id);
                         // true: has hit a breakpoint
                         return Ok(true);
@@ -1032,14 +1033,43 @@ impl Replay for DbReplay {
 
     fn add_breakpoint(&mut self, path: &str, line: i64) -> Result<Breakpoint, Box<dyn Error>> {
         let path_id_res: Result<PathId, Box<dyn Error>> = self
-            .load_path_id(&loc.path)
-            .ok_or(format!("can't add a breakpoint: can't find path `{}`` in trace", loc.path).into());
+            .load_path_id(path)
+            .ok_or(format!("can't add a breakpoint: can't find path `{}`` in trace", path).into());
         let path_id = path_id_res?;
         let inner_map = &mut self.breakpoint_list[path_id.0];
-        let breakpoint = Breakpoint { enabled: true, id: self.breakpoint_next_id };
-        self.breakpoint_next_i
-        inner_map.insert(loc.line, Breakpoint { is_active: true });
+        let breakpoint = Breakpoint { enabled: true, id: self.breakpoint_next_id as i64 };
+        self.breakpoint_next_id += 1;
+        inner_map.insert(line as usize, breakpoint.clone());
+        Ok(breakpoint)
     }
+
+    fn delete_breakpoint(&mut self, _breakpoint: &Breakpoint) -> Result<bool, Box<dyn Error>> {
+        // let path_id_res: Result<PathId, Box<dyn Error>> = self
+        //     .load_path_id(&loc.path)
+        //     .ok_or(format!("can't add a breakpoint: can't find path `{}`` in trace", loc.path).into());
+        // let path_id = path_id_res?;
+        // let inner_map = &mut self.breakpoint_list[path_id.0];
+        // inner_map.remove(&loc.line);
+        todo!()        
+    }
+
+    fn delete_breakpoints(&mut self) -> Result<bool, Box<dyn Error>> {
+        self.breakpoint_list.clear();
+        self.breakpoint_list.resize_with(self.db.paths.len(), HashMap::new);
+        Ok(true)
+    } 
+
+    fn toggle_breakpoint(&mut self, breakpoint: &Breakpoint) -> Result<Breakpoint, Box<dyn Error>> {
+        // let path_id_res: Result<PathId, Box<dyn Error>> = self
+        //     .load_path_id(&loc.path)
+        //     .ok_or(format!("can't add a breakpoint: can't find path `{}`` in trace", loc.path).into());
+        // let path_id = path_id_res?;
+        // if let Some(breakpoint) = self.breakpoint_list[path_id.0].get_mut(&loc.line) {
+        let mut toggled_breakpoint = breakpoint.clone();
+        toggled_breakpoint.enabled = !toggled_breakpoint.enabled;
+        Ok(toggled_breakpoint)
+    }
+
     fn current_step_id(&mut self) -> StepId {
         self.step_id
     }

src/db-backend/src/handler.rs

@@ -1,5 +1,5 @@
 use indexmap::IndexMap;
-use std::collections::HashMap;
+use std::collections::{HashMap, HashSet};
 use std::error::Error;
 
 use log::{error, info, warn};
@@ -52,6 +52,7 @@ pub struct Handler {
     pub resulting_dap_messages: Vec<DapMessage>,
     pub raw_diff_index: Option<String>,
     pub previous_step_id: StepId,
+    pub breakpoints: HashMap<String, HashSet<i64>>,
 
     pub trace_kind: TraceKind,
     pub replay: Box<dyn Replay>,
@@ -114,6 +115,7 @@ impl Handler {
             step_lines_loader,
             dap_client: DapClient::default(),
             previous_step_id: StepId(0),
+            breakpoints: HashMap::new(),
             replay,
             ct_rr_args,
             load_flow_index: 0,
@@ -893,34 +895,95 @@ impl Handler {
         }
     }
 
-    pub fn add_breakpoint(&mut self, loc: SourceLocation, _task: Task) -> Result<(), Box<dyn Error>> {
-       self.replay.add_breakpoint(&loc.path, loc.line as i64)?;
+    pub fn set_breakpoints(&mut self, request: dap::Request, args: dap_types::SetBreakpointsArguments) -> Result<(), Box<dyn Error>> {
+        let mut results = Vec::new();
+        self.clear_breakpoints()?;
+        if let Some(path) = args.source.path.clone() {
+            let lines: Vec<i64> = if let Some(bps) = args.breakpoints {
+                bps.into_iter().map(|b| b.line).collect()
+            } else {
+                args.lines.unwrap_or_default()
+            };
+        
+            for line in lines {
+                { 
+                    let entry = self.breakpoints.entry(path.clone()).or_default();
+                    entry.insert(line);
+                }
+                let _ = self.add_breakpoint(
+                    SourceLocation {
+                        path: path.clone(),
+                        line: line as usize,
+                    },
+                );
+                results.push(dap_types::Breakpoint {
+                    id: None,
+                    verified: true,
+                    message: None,
+                    source: Some(dap_types::Source {
+                        name: args.source.name.clone(),
+                        path: Some(path.clone()),
+                        source_reference: args.source.source_reference,
+                        presentation_hint: None,
+                        origin: None,
+                        sources: None,
+                        adapter_data: None,
+                        checksums: None,
+                    }),
+                    line: Some(line),
+                    column: None,
+                    end_line: None,
+                    end_column: None,
+                    instruction_reference: None,
+                    offset: None,
+                    reason: None,
+                });
+            }
+        } else {
+            let lines = args
+                .breakpoints
+                .unwrap_or_default()
+                .into_iter()
+                .map(|b| b.line)
+                .collect::<Vec<_>>();
+            for line in lines {
+                results.push(dap_types::Breakpoint {
+                    id: None,
+                    verified: false,
+                    message: Some("missing source path".to_string()),
+                    source: None,
+                    line: Some(line),
+                    column: None,
+                    end_line: None,
+                    end_column: None,
+                    instruction_reference: None,
+                    offset: None,
+                    reason: None,
+                });
+            }
+        }
+        self.respond_dap(request, dap_types::SetBreakpointsResponseBody { breakpoints: results })?;
+        Ok(())
+    }
+
+    pub fn add_breakpoint(&mut self, loc: SourceLocation) -> Result<(), Box<dyn Error>> {
+       let _breakpoint = self.replay.add_breakpoint(&loc.path, loc.line as i64)?;
+       // TODO: map them to id-s in handler so we can do the reverse on delete and let replay work with breakpoints?
        Ok(())
     }
 
-    pub fn delete_breakpoint(&mut self, loc: SourceLocation, _task: Task) -> Result<(), Box<dyn Error>> {
-        let path_id_res: Result<PathId, Box<dyn Error>> = self
-            .load_path_id(&loc.path)
-            .ok_or(format!("can't add a breakpoint: can't find path `{}`` in trace", loc.path).into());
-        let path_id = path_id_res?;
-        let inner_map = &mut self.breakpoint_list[path_id.0];
-        inner_map.remove(&loc.line);
+    pub fn delete_breakpoint(&mut self, _loc: SourceLocation, _task: Task) -> Result<(), Box<dyn Error>> {
+        // TODO: load themfrom handler id map: self.replay.delete_breakpoint(loc)
         Ok(())
     }
 
-    pub fn clear_breakpoints(&mut self) {
-        self.breakpoint_list.clear();
-        self.breakpoint_list.resize_with(self.db.paths.len(), HashMap::new);
+    pub fn clear_breakpoints(&mut self) -> Result<(), Box<dyn Error>> {
+        let _ = self.replay.delete_breakpoints()?;
+        Ok(())
     }
 
-    pub fn toggle_breakpoint(&mut self, loc: SourceLocation, _task: Task) -> Result<(), Box<dyn Error>> {
-        let path_id_res: Result<PathId, Box<dyn Error>> = self
-            .load_path_id(&loc.path)
-            .ok_or(format!("can't add a breakpoint: can't find path `{}`` in trace", loc.path).into());
-        let path_id = path_id_res?;
-        if let Some(breakpoint) = self.breakpoint_list[path_id.0].get_mut(&loc.line) {
-            breakpoint.is_active = !breakpoint.is_active;
-        }
+    pub fn toggle_breakpoint(&mut self, _loc: SourceLocation, _task: Task) -> Result<(), Box<dyn Error>> {
+        // TODO: use path,line to id map: self.replay.toggle_breakpoint()?;
         Ok(())
     }
 

src/db-backend/src/query.rs

@@ -1,6 +1,6 @@
 use serde::{Deserialize, Serialize};
 
-use crate::task::{Action, CtLoadLocalsArguments};
+use crate::task::{Action, Breakpoint, CtLoadLocalsArguments};
 
 #[derive(Debug, Clone, Serialize, Deserialize)]
 #[serde(tag = "kind")]
@@ -11,4 +11,8 @@ pub enum CtRRQuery {
     LoadLocals { arg: CtLoadLocalsArguments },
     LoadReturnValue,
     LoadValue { expression: String },
+    AddBreakpoint { path: String, line: i64, },
+    DeleteBreakpoint { breakpoint: Breakpoint, },
+    DeleteBreakpoints,
+    ToggleBreakpoint { breakpoint: Breakpoint, },
 }

src/db-backend/src/replay.rs

@@ -25,5 +25,7 @@ pub trait Replay: std::fmt::Debug {
     fn jump_to(&mut self, step_id: StepId) -> Result<bool, Box<dyn Error>>;
     fn add_breakpoint(&mut self, path: &str, line: i64) -> Result<Breakpoint, Box<dyn Error>>;
     fn delete_breakpoint(&mut self, breakpoint: &Breakpoint) -> Result<bool, Box<dyn Error>>;
+    fn delete_breakpoints(&mut self) -> Result<bool, Box<dyn Error>>;
+    fn toggle_breakpoint(&mut self, breakpoint: &Breakpoint) -> Result<Breakpoint, Box<dyn Error>>;
     fn current_step_id(&mut self) -> StepId;
 }