Use compiler source map directly in forc-debug (#6872)

JoshuaBatty · web-flow · commit f0ed57af002d · 2025-01-30T21:44:51.000Z
## Description Replace the custom HashMap-based source map implementation in `forc-debug` with the compiler's source map format directly. This simplifies our source mapping logic and maintains better consistency with the compiler's source location tracking, while preserving all existing functionality. Key changes: - Remove custom source map types and use compiler's SourceMap - Simplify breakpoint verification using compiler spans - Add test to verify source map instruction-to-line mappings closes #6733 ## Checklist - [x] I have linked to any relevant issues. - [x] I have commented my code, particularly in hard-to-understand areas. - [x] I have updated the documentation where relevant (API docs, the reference, and the Sway book). - [x] If my change requires substantial documentation changes, I have [requested support from the DevRel team](https://github.com/FuelLabs/devrel-requests/issues/new/choose) - [x] I have added tests that prove my fix is effective or that my feature works. - [x] I have added (or requested a maintainer to add) the necessary `Breaking*` or `New Feature` labels where relevant. - [x] I have done my best to ensure that my PR adheres to [the Fuel Labs Code Review Standards](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md). - [x] I have requested a review from the relevant team or maintainers.
diff --git a/forc-plugins/forc-debug/src/server/handlers/handle_set_breakpoints.rs b/forc-plugins/forc-debug/src/server/handlers/handle_set_breakpoints.rs
@@ -45,20 +45,20 @@ impl DapServer {
             .cloned()
             .unwrap_or_default();
 
-        let source_map = self
-            .state
-            .source_map
-            .get(&source_path_buf)
-            .cloned()
-            .unwrap_or_default();
-
         let breakpoints = args
             .breakpoints
             .clone()
             .unwrap_or_default()
             .iter()
             .map(|source_bp| {
-                let verified = source_map.contains_key(&source_bp.line);
+                // Check if there are any instructions mapped to this line in the source
+                let verified = self.state.source_map.map.iter().any(|(pc, _)| {
+                    if let Some((path, range)) = self.state.source_map.addr_to_span(*pc) {
+                        path == source_path_buf && range.start.line as i64 == source_bp.line
+                    } else {
+                        false
+                    }
+                });
                 if let Some(existing_bp) = existing_breakpoints
                     .iter()
                     .find(|bp| bp.line.map_or(false, |line| line == source_bp.line))
@@ -91,9 +91,9 @@ impl DapServer {
 
 #[cfg(test)]
 mod tests {
-    use std::{collections::HashMap, iter};
-
     use super::*;
+    use sway_core::source_map::{LocationRange, PathIndex, SourceMap, SourceMapSpan};
+    use sway_types::LineCol;
 
     const MOCK_SOURCE_PATH: &str = "some/path";
     const MOCK_BP_ID: i64 = 1;
@@ -103,10 +103,26 @@ mod tests {
     fn get_test_server(source_map: bool, existing_bp: bool) -> DapServer {
         let mut server = DapServer::default();
         if source_map {
-            server.state.source_map.insert(
-                PathBuf::from(MOCK_SOURCE_PATH),
-                HashMap::from_iter(iter::once((MOCK_LINE, vec![MOCK_INSTRUCTION]))),
+            // Create a source map with our test line
+            let mut map = SourceMap::new();
+            map.paths.push(PathBuf::from(MOCK_SOURCE_PATH));
+            map.map.insert(
+                MOCK_INSTRUCTION as usize,
+                SourceMapSpan {
+                    path: PathIndex(0),
+                    range: LocationRange {
+                        start: LineCol {
+                            line: MOCK_LINE as usize,
+                            col: 0,
+                        },
+                        end: LineCol {
+                            line: MOCK_LINE as usize,
+                            col: 10,
+                        },
+                    },
+                },
             );
+            server.state.source_map = map;
         }
         if existing_bp {
             server.state.breakpoints.insert(
diff --git a/forc-plugins/forc-debug/src/server/handlers/handle_stack_trace.rs b/forc-plugins/forc-debug/src/server/handlers/handle_stack_trace.rs
@@ -46,7 +46,7 @@ impl DapServer {
                     executor.interpreter.registers(),
                 ))
                 .ok()
-                .map(|(source_path, line)| (Some(util::path_into_source(source_path)), line)),
+                .map(|(source_path, line)| (Some(util::path_into_source(&source_path)), line)),
         };
 
         // For now, we only return 1 stack frame.
diff --git a/forc-plugins/forc-debug/src/server/mod.rs b/forc-plugins/forc-debug/src/server/mod.rs
@@ -24,13 +24,11 @@ use forc_test::{
 };
 use serde::{Deserialize, Serialize};
 use std::{
-    collections::HashMap,
     io::{BufReader, BufWriter, Read, Write},
     process,
     sync::Arc,
 };
 use sway_core::BuildTarget;
-use sway_types::LineCol;
 
 pub const THREAD_ID: i64 = 0;
 pub const REGISTERS_VARIABLE_REF: i64 = 1;
@@ -53,7 +51,7 @@ pub struct DapServer {
     /// Used to generate unique breakpoint IDs.
     breakpoint_id_gen: IdGenerator,
     /// The server state.
-    state: ServerState,
+    pub state: ServerState,
 }
 
 impl Default for DapServer {
@@ -261,7 +259,7 @@ impl DapServer {
     }
 
     /// Builds the tests at the given [PathBuf] and stores the source maps.
-    pub(crate) fn build_tests(&mut self) -> Result<(BuiltPackage, TestSetup), AdapterError> {
+    pub fn build_tests(&mut self) -> Result<(BuiltPackage, TestSetup), AdapterError> {
         if let Some(pkg) = &self.state.built_package {
             if let Some(setup) = &self.state.test_setup {
                 return Ok((pkg.clone(), setup.clone()));
@@ -327,51 +325,19 @@ impl DapServer {
             reason: format!("build packages: {err:?}"),
         })?;
 
-        // 2. Store the source maps
-        let mut pkg_to_debug: Option<&BuiltPackage> = None;
-        for (_, built_pkg) in &built_packages {
-            if built_pkg.descriptor.manifest_file == pkg_manifest {
-                pkg_to_debug = Some(built_pkg);
-            }
-            let source_map = &built_pkg.source_map;
-
-            let paths = &source_map.paths;
-            source_map.map.iter().for_each(|(instruction, sm_span)| {
-                if let Some(path_buf) = paths.get(sm_span.path.0) {
-                    let LineCol { line, .. } = sm_span.range.start;
-                    let (line, instruction) = (line as i64, *instruction as Instruction);
-
-                    self.state
-                        .source_map
-                        .entry(path_buf.clone())
-                        .and_modify(|new_map| {
-                            new_map
-                                .entry(line)
-                                .and_modify(|val| {
-                                    // Store the instructions in ascending order
-                                    match val.binary_search(&instruction) {
-                                        Ok(_) => {} // Ignore duplicates
-                                        Err(pos) => val.insert(pos, instruction),
-                                    }
-                                })
-                                .or_insert(vec![instruction]);
-                        })
-                        .or_insert(HashMap::from([(line, vec![instruction])]));
-                } else {
-                    self.error(format!(
-                        "Path missing from source map: {:?}",
-                        sm_span.path.0
-                    ));
-                }
-            });
-        }
+        // 2. Store the source maps and find debug package
+        let pkg_to_debug = built_packages
+            .iter()
+            .find(|(_, pkg)| pkg.descriptor.manifest_file == pkg_manifest)
+            .map(|(_, pkg)| pkg)
+            .ok_or(AdapterError::BuildFailed {
+                reason: format!("find package: {project_name}"),
+            })?;
 
-        // 3. Build the tests
-        let built_package = pkg_to_debug.ok_or(AdapterError::BuildFailed {
-            reason: format!("find package: {project_name}"),
-        })?;
+        self.state.source_map = pkg_to_debug.source_map.clone();
 
-        let built = Built::Package(Arc::from(built_package.clone()));
+        // 3. Build the tests
+        let built = Built::Package(Arc::from(pkg_to_debug.clone()));
 
         let built_tests = BuiltTests::from_built(built, &build_plan).map_err(|err| {
             AdapterError::BuildFailed {
@@ -390,9 +356,9 @@ impl DapServer {
         let test_setup = pkg_tests.setup().map_err(|err| AdapterError::BuildFailed {
             reason: format!("test setup: {err:?}"),
         })?;
-        self.state.built_package = Some(built_package.clone());
+        self.state.built_package = Some(pkg_to_debug.clone());
         self.state.test_setup = Some(test_setup.clone());
-        Ok((built_package.clone(), test_setup))
+        Ok((pkg_to_debug.clone(), test_setup))
     }
 
     /// Sends the 'exited' event to the client and kills the server process.
diff --git a/forc-plugins/forc-debug/src/server/state.rs b/forc-plugins/forc-debug/src/server/state.rs
@@ -1,11 +1,12 @@
 use crate::{
     error::AdapterError,
-    types::{Breakpoints, Instruction, SourceMap},
+    types::{Breakpoints, Instruction},
 };
 use dap::types::StartDebuggingRequestKind;
 use forc_pkg::BuiltPackage;
 use forc_test::{execute::TestExecutor, setup::TestSetup, TestResult};
 use std::path::PathBuf;
+use sway_core::source_map::SourceMap;
 
 #[derive(Default, Debug, Clone)]
 /// The state of the DAP server.
@@ -58,24 +59,55 @@ impl ServerState {
     pub fn vm_pc_to_source_location(
         &self,
         pc: Instruction,
-    ) -> Result<(&PathBuf, i64), AdapterError> {
-        // Try to find the source location by looking for the program counter in the source map.
-        self.source_map
+    ) -> Result<(PathBuf, i64), AdapterError> {
+        // Convert PC to instruction index (divide by 4 for byte offset)
+        let instruction_idx = (pc / 4) as usize;
+        if let Some((path, range)) = self.source_map.addr_to_span(instruction_idx) {
+            Ok((path, range.start.line as i64))
+        } else {
+            Err(AdapterError::MissingSourceMap { pc })
+        }
+    }
+
+    /// Updates the breakpoints in the VM for all remaining [TestExecutor]s.
+    pub(crate) fn update_vm_breakpoints(&mut self) {
+        if !self.breakpoints_need_update {
+            return;
+        }
+
+        // Convert breakpoints to instruction offsets using the source map
+        let opcode_indexes = self
+            .breakpoints
             .iter()
-            .find_map(|(source_path, source_map)| {
-                for (&line, instructions) in source_map {
-                    // Divide by 4 to get the opcode offset rather than the program counter offset.
-                    let instruction_offset = pc / 4;
-                    if instructions
-                        .iter()
-                        .any(|instruction| instruction_offset == *instruction)
-                    {
-                        return Some((source_path, line));
-                    }
-                }
-                None
-            })
-            .ok_or(AdapterError::MissingSourceMap { pc })
+            .flat_map(|(source_path, breakpoints)| {
+                breakpoints
+                    .iter()
+                    .filter_map(|bp| {
+                        bp.line.and_then(|line| {
+                            // Find any instruction that maps to this line in the source map
+                            self.source_map.map.iter().find_map(|(pc, _)| {
+                                self.source_map
+                                    .addr_to_span(*pc)
+                                    .filter(|(path, range)| {
+                                        path == source_path && range.start.line as i64 == line
+                                    })
+                                    .map(|_| pc)
+                            })
+                        })
+                    })
+                    .collect::<Vec<_>>()
+            });
+
+        // Set breakpoints in the VM
+        self.executors.iter_mut().for_each(|executor| {
+            let bps: Vec<_> = opcode_indexes
+                .clone()
+                .map(|opcode_index| fuel_vm::state::Breakpoint::script(*opcode_index as u64))
+                .collect();
+            executor.interpreter.overwrite_breakpoints(&bps);
+        });
+
+        self.breakpoints_need_update = false;
     }
 
     /// Finds the breakpoint matching a VM program counter.
@@ -85,7 +117,7 @@ impl ServerState {
         // Find the breakpoint ID matching the source location.
         let source_bps = self
             .breakpoints
-            .get(source_path)
+            .get(&source_path)
             .ok_or(AdapterError::UnknownBreakpoint { pc })?;
         let breakpoint_id = source_bps
             .iter()
@@ -101,41 +133,6 @@ impl ServerState {
         Ok(breakpoint_id)
     }
 
-    /// Updates the breakpoints in the VM for all remaining [TestExecutor]s.
-    pub(crate) fn update_vm_breakpoints(&mut self) {
-        if !self.breakpoints_need_update {
-            return;
-        }
-        let opcode_indexes = self
-            .breakpoints
-            .iter()
-            .flat_map(|(source_path, breakpoints)| {
-                if let Some(source_map) = self.source_map.get(&PathBuf::from(source_path)) {
-                    breakpoints
-                        .iter()
-                        .filter_map(|bp| {
-                            bp.line.and_then(|line| {
-                                source_map
-                                    .get(&line)
-                                    .and_then(|instructions| instructions.first())
-                            })
-                        })
-                        .collect::<Vec<_>>()
-                } else {
-                    vec![]
-                }
-            });
-
-        self.executors.iter_mut().for_each(|executor| {
-            // TODO: use `overwrite_breakpoints` when released
-            opcode_indexes.clone().for_each(|opcode_index| {
-                let bp: fuel_vm::prelude::Breakpoint =
-                    fuel_vm::state::Breakpoint::script(*opcode_index);
-                executor.interpreter.set_breakpoint(bp);
-            });
-        });
-    }
-
     pub(crate) fn test_complete(&mut self, result: TestResult) {
         self.test_results.push(result);
         self.executors.remove(0);
diff --git a/forc-plugins/forc-debug/src/types.rs b/forc-plugins/forc-debug/src/types.rs
@@ -1,9 +1,6 @@
 use dap::types::Breakpoint;
 use std::{collections::HashMap, path::PathBuf};
 
-pub type Line = i64;
 pub type ExitCode = i64;
 pub type Instruction = u64;
-pub type FileSourceMap = HashMap<Line, Vec<Instruction>>;
-pub type SourceMap = HashMap<PathBuf, FileSourceMap>;
 pub type Breakpoints = HashMap<PathBuf, Vec<Breakpoint>>;
diff --git a/forc-plugins/forc-debug/tests/server_integration.rs b/forc-plugins/forc-debug/tests/server_integration.rs
