From 894525448d0269f044154d298c1216ea41d1e2c3 Mon Sep 17 00:00:00 2001 From: Griffin Berlstein Date: Tue, 6 Aug 2024 15:16:41 -0400 Subject: [PATCH] [Cider2] Watchpoint fixes (#2250) --- .../src/debugger/commands/command_parser.rs | 14 + interp/src/debugger/commands/commands.pest | 9 +- interp/src/debugger/commands/core.rs | 2 + interp/src/debugger/debugger_core.rs | 18 + .../src/debugger/debugging_context/context.rs | 26 +- .../src/flatten/structures/environment/env.rs | 98 ++++- .../structures/environment/traverser.rs | 357 +++++++++++++++--- 7 files changed, 441 insertions(+), 83 deletions(-) diff --git a/interp/src/debugger/commands/command_parser.rs b/interp/src/debugger/commands/command_parser.rs index cea0d0fdcc..d4445342ac 100644 --- a/interp/src/debugger/commands/command_parser.rs +++ b/interp/src/debugger/commands/command_parser.rs @@ -203,6 +203,18 @@ impl CommandParser { )) } + fn enable_watch(input: Node) -> ParseResult { + Ok(match_nodes!(input.into_children(); + [brk_id(br)..] => Command::EnableWatch(br.collect()) + )) + } + + fn disable_watch(input: Node) -> ParseResult { + Ok(match_nodes!(input.into_children(); + [brk_id(br)..] => Command::DisableWatch(br.collect()) + )) + } + fn explain(_input: Node) -> ParseResult { Ok(Command::Explain) } @@ -261,6 +273,8 @@ impl CommandParser { [info_watch(iw), EOI(_)] => iw, [delete(del), EOI(_)] => del, [delete_watch(del), EOI(_)] => del, + [enable_watch(ew), EOI(_)] => ew, + [disable_watch(dw), EOI(_)] => dw, [enable(e), EOI(_)] => e, [disable(dis), EOI(_)] => dis, [exit(exit), EOI(_)] => exit, diff --git a/interp/src/debugger/commands/commands.pest b/interp/src/debugger/commands/commands.pest index 872fd34575..7cc7df7ca2 100644 --- a/interp/src/debugger/commands/commands.pest +++ b/interp/src/debugger/commands/commands.pest @@ -59,8 +59,11 @@ delete = { (^"delete" | ^"del") ~ brk_id+ } delete_watch = { (^"delete-watch" | ^"del-watch") ~ brk_id+ } -enable = { (^"enable") ~ brk_id+ } -disable = { (^"disable") ~ brk_id+ } +enable = { (^"enable" | ^"en") ~ brk_id+ } +disable = { (^"disable" | ^"dis") ~ brk_id+ } + +enable_watch = { (^"enable-watch" | ^"enw") ~ brk_id+ } +disable_watch = { (^"disable-watch" | ^"disw") ~ brk_id+ } exit = { ^"exit" | ^"quit" } @@ -71,6 +74,6 @@ explain = { ^"explain" } restart = { ^"restart" } command = { - SOI ~ (watch | comm_where | print_state | print | print_fail | delete_watch | delete | brk | enable | disable | step_over | step // commands without input + SOI ~ (watch | comm_where | print_state | print | print_fail | delete_watch | delete | brk | enable_watch | disable_watch | enable | disable | step_over | step // commands without input | cont | help | info_break | info_watch | display | exit | explain | restart)? ~ EOI } diff --git a/interp/src/debugger/commands/core.rs b/interp/src/debugger/commands/core.rs index 0d40119743..4cdc9da493 100644 --- a/interp/src/debugger/commands/core.rs +++ b/interp/src/debugger/commands/core.rs @@ -291,6 +291,8 @@ pub enum Command { Disable(Vec), Enable(Vec), Delete(Vec), + EnableWatch(Vec), + DisableWatch(Vec), DeleteWatch(Vec), StepOver(ParsedGroupName), Watch( diff --git a/interp/src/debugger/debugger_core.rs b/interp/src/debugger/debugger_core.rs index ed12bd9f89..08f2f4afa9 100644 --- a/interp/src/debugger/debugger_core.rs +++ b/interp/src/debugger/debugger_core.rs @@ -313,6 +313,24 @@ impl + Clone> Debugger { } } + Command::EnableWatch(targets) => { + for target in targets { + let target = target + .parse_to_watch_ids(self.program_context.as_ref()); + unwrap_error_message!(target); + self.debugging_context.enable_watchpoint(target) + } + } + + Command::DisableWatch(targets) => { + for target in targets { + let target = target + .parse_to_watch_ids(self.program_context.as_ref()); + unwrap_error_message!(target); + self.debugging_context.disable_watchpoint(target) + } + } + Command::Watch( group, watch_pos, diff --git a/interp/src/debugger/debugging_context/context.rs b/interp/src/debugger/debugging_context/context.rs index fad5719e56..a8c30ae183 100644 --- a/interp/src/debugger/debugging_context/context.rs +++ b/interp/src/debugger/debugging_context/context.rs @@ -74,11 +74,11 @@ pub struct WatchPoint { } impl WatchPoint { - pub fn _enable(&mut self) { + pub fn enable(&mut self) { self.state = PointStatus::Enabled; } - pub fn _disable(&mut self) { + pub fn disable(&mut self) { self.state = PointStatus::Disabled; } @@ -254,7 +254,7 @@ impl WatchPointIndices { fn get_before(&self) -> Option<&[WatchpointIdx]> { match self { Self::Before(idx) => Some(idx), - Self::Both { after, .. } => Some(after), + Self::Both { before, .. } => Some(before), Self::After(_) => None, } } @@ -263,7 +263,7 @@ impl WatchPointIndices { match self { Self::Before(_) => None, Self::After(idx) => Some(idx), - Self::Both { before, .. } => Some(before), + Self::Both { after, .. } => Some(after), } } @@ -531,14 +531,22 @@ impl DebuggingContext { self.watchpoints.delete_by_idx(target) } - fn _act_watchpoint(&mut self, target: WatchID, action: PointAction) { + pub fn enable_watchpoint(&mut self, target: WatchID) { + self.act_watchpoint(target, PointAction::Enable) + } + + pub fn disable_watchpoint(&mut self, target: WatchID) { + self.act_watchpoint(target, PointAction::Disable) + } + + fn act_watchpoint(&mut self, target: WatchID, action: PointAction) { fn act(target: &mut WatchPoint, action: PointAction) { match action { PointAction::Enable => { - target._enable(); + target.enable(); } PointAction::Disable => { - target._disable(); + target.disable(); } } } @@ -582,11 +590,11 @@ impl DebuggingContext { } pub fn _enable_watchpoint(&mut self, target: WatchID) { - self._act_watchpoint(target, PointAction::Enable) + self.act_watchpoint(target, PointAction::Enable) } pub fn _disable_watchpoint(&mut self, target: WatchID) { - self._act_watchpoint(target, PointAction::Disable) + self.act_watchpoint(target, PointAction::Disable) } pub fn hit_breakpoints(&self) -> impl Iterator + '_ { diff --git a/interp/src/flatten/structures/environment/env.rs b/interp/src/flatten/structures/environment/env.rs index 2ea02570df..9725cb00ab 100644 --- a/interp/src/flatten/structures/environment/env.rs +++ b/interp/src/flatten/structures/environment/env.rs @@ -10,6 +10,10 @@ use crate::{ errors::{BoxedInterpreterError, InterpreterError, InterpreterResult}, flatten::{ flat_ir::{ + base::{ + LocalCellOffset, LocalPortOffset, LocalRefCellOffset, + LocalRefPortOffset, + }, cell_prototype::{CellPrototype, SingleWidthType}, prelude::{ AssignedValue, AssignmentIdx, BaseIndices, @@ -23,7 +27,7 @@ use crate::{ }, primitives::{self, prim_trait::UpdateStatus, Primitive}, structures::{ - context::LookupName, + context::{LookupName, PortDefinitionInfo}, environment::{ program_counter::ControlPoint, traverser::Traverser, }, @@ -691,8 +695,15 @@ impl + Clone> Environment { &self.cells[point.comp].unwrap_comp().index_bases + l, ), - CellRef::Ref(_) => { - "".to_string() + CellRef::Ref(r) => { + let ref_global_offset = &self.cells[point.comp] + .unwrap_comp() + .index_bases + + r; + let ref_actual = + self.ref_cells[ref_global_offset].unwrap(); + + self.get_full_name(ref_actual) } }; @@ -843,12 +854,12 @@ impl + Clone> Environment { break; } } + } - if let Some(highest_found) = highest_found { - path.push(highest_found); - } else { - return None; - } + if let Some(highest_found) = highest_found { + path.push(highest_found); + } else { + return None; } } } @@ -1124,6 +1135,48 @@ impl + Clone> Environment { let port = self.get_root_input_port(port); self.pinned_ports.remove(port); } + + pub fn get_def_info( + &self, + comp_idx: ComponentIdx, + cell: LocalCellOffset, + ) -> &crate::flatten::flat_ir::base::CellDefinitionInfo + { + let comp = &self.ctx.as_ref().secondary[comp_idx]; + let idx = comp.cell_offset_map[cell]; + &self.ctx.as_ref().secondary[idx] + } + + pub fn get_def_info_ref( + &self, + comp_idx: ComponentIdx, + cell: LocalRefCellOffset, + ) -> &crate::flatten::flat_ir::base::CellDefinitionInfo + { + let comp = &self.ctx.as_ref().secondary[comp_idx]; + let idx = comp.ref_cell_offset_map[cell]; + &self.ctx.as_ref().secondary[idx] + } + + pub fn get_port_def_info( + &self, + comp_idx: ComponentIdx, + port: LocalPortOffset, + ) -> &PortDefinitionInfo { + let comp = &self.ctx.as_ref().secondary[comp_idx]; + let idx = comp.port_offset_map[port]; + &self.ctx.as_ref().secondary[idx] + } + + pub fn get_port_def_info_ref( + &self, + comp_idx: ComponentIdx, + port: LocalRefPortOffset, + ) -> Identifier { + let comp = &self.ctx.as_ref().secondary[comp_idx]; + let idx = comp.ref_port_offset_map[port]; + self.ctx.as_ref().secondary[idx] + } } /// A wrapper struct for the environment that provides the functions used to @@ -2155,9 +2208,9 @@ impl + Clone> Simulator { if state.has_state() { if let Some(name_override) = name { - write!(output, "{name_override}:").unwrap(); + write!(output, "{name_override}: ").unwrap(); } else { - write!(output, "{}:", self.get_full_name(cell_idx)).unwrap(); + write!(output, "{}: ", self.get_full_name(cell_idx)).unwrap(); } writeln!(output, "{state}").unwrap(); @@ -2214,3 +2267,28 @@ impl + Clone> GetFullName for GlobalPortIdx { format!("{path_str}.{name}") } } + +impl + Clone> GetFullName for GlobalRefCellIdx { + fn get_full_name(&self, env: &Environment) -> String { + let parent_path = env.get_parent_path_from_cell(*self).unwrap(); + let path_str = env.format_path(&parent_path); + + let immediate_parent = parent_path.last().unwrap(); + let comp = if env.cells[*immediate_parent].as_comp().is_some() { + *immediate_parent + } else { + // get second-to-last parent + parent_path[parent_path.len() - 2] + }; + + let ledger = env.cells[comp].as_comp().unwrap(); + + let local_offset = *self - &ledger.index_bases; + let comp_def = &env.ctx().secondary[ledger.comp_id]; + let ref_cell_def_idx = &comp_def.ref_cell_offset_map[local_offset]; + let ref_cell_def = &env.ctx().secondary[*ref_cell_def_idx]; + let name = env.ctx().lookup_name(ref_cell_def.name); + + format!("{path_str}.{name}") + } +} diff --git a/interp/src/flatten/structures/environment/traverser.rs b/interp/src/flatten/structures/environment/traverser.rs index 16b1cbb8ef..a2247011ff 100644 --- a/interp/src/flatten/structures/environment/traverser.rs +++ b/interp/src/flatten/structures/environment/traverser.rs @@ -1,6 +1,10 @@ use crate::flatten::{ - flat_ir::prelude::{ - CellRef, GlobalCellIdx, GlobalPortIdx, GlobalRefCellIdx, PortRef, + flat_ir::{ + base::ComponentIdx, + cell_prototype::CellPrototype, + prelude::{ + CellRef, GlobalCellIdx, GlobalPortIdx, GlobalRefCellIdx, PortRef, + }, }, structures::context::Context, }; @@ -123,7 +127,7 @@ impl Traverser { let mut current_comp_idx = *def .prototype .as_component() - .expect("called next_cell on a primitive ref cell"); + .expect("called compute_current_comp_def on a primitive ref cell"); for cell_ref in self.abstract_path.iter() { let def = &env.ctx().secondary[current_comp_idx]; @@ -153,14 +157,136 @@ impl Traverser { target: S, ) -> Result { if let Some(first_ref) = self.first_ref { - let current_comp_idx = if !self.abstract_path.is_empty() { - self.compute_current_comp_def(env) + let (outer_comp, last_cell): (ComponentIdx, CellRef) = if !self + .abstract_path + .is_empty() + { + let current_comp_ledger = env.cells + [*self.concrete_path.last().unwrap()] + .as_comp() + .unwrap(); + + let ref_offset = first_ref - ¤t_comp_ledger.index_bases; + let mut current_comp_id = *env + .get_def_info_ref(current_comp_ledger.comp_id, ref_offset) + .prototype + .as_component() + .unwrap(); + + // iterate over the cells which must be components + for abstract_cell in + self.abstract_path.iter().take(self.abstract_path.len() - 1) + { + match abstract_cell { + CellRef::Local(l) => { + let info = env.get_def_info(current_comp_id, *l); + current_comp_id = + *info.prototype.as_component().unwrap(); + } + CellRef::Ref(r) => { + let info = + env.get_def_info_ref(current_comp_id, *r); + current_comp_id = + *info.prototype.as_component().unwrap(); + } + } + } + + let last_ref = self.abstract_path.last().unwrap(); + (current_comp_id, *last_ref) } else { - let last_comp = *self.concrete_path.last().unwrap(); - let last_comp_ledger = env.cells[last_comp].as_comp().unwrap(); - last_comp_ledger.comp_id + let last_comp_idx = *self.concrete_path.last().unwrap(); + let last_comp_ledger = + env.cells[last_comp_idx].as_comp().unwrap(); + let local_offset = first_ref - &last_comp_ledger.index_bases; + (last_comp_ledger.comp_id, local_offset.into()) }; + let current_comp_idx = match last_cell { + CellRef::Local(l) => { + let info = env.get_def_info(outer_comp, l); + + // first check component ports for the target + for offset in info.ports.iter() { + let def_info = + env.get_port_def_info(outer_comp, offset); + if env.ctx().lookup_name(def_info.name) + == target.as_ref() + { + return Ok(Path::AbstractPort { + cell: LazyCellPath { + concrete_prefix: self.concrete_path, + first_ref, + abstract_suffix: self.abstract_path, + }, + port: offset.into(), + }); + } + } + + // it's not a port + if let Some(cell) = info.prototype.as_component() { + *cell + } else { + return Err(TraversalError::Unknown( + target.as_ref().to_string(), + )); + } + } + CellRef::Ref(r) => { + let info = env.get_def_info_ref(outer_comp, r); + + // first check component ports for the target + for offset in info.ports.iter() { + let name = + env.get_port_def_info_ref(outer_comp, offset); + if env.ctx().lookup_name(name) == target.as_ref() + && info.prototype.is_component() + { + let comp = info.prototype.as_component().unwrap(); + let sig = env.ctx().secondary[*comp].signature(); + for port in sig { + let port_def = + env.get_port_def_info(*comp, port); + if env.ctx().lookup_name(port_def.name) + == target.as_ref() + { + return Ok(Path::AbstractPort { + cell: LazyCellPath { + concrete_prefix: self.concrete_path, + first_ref, + abstract_suffix: self.abstract_path, + }, + port: port.into(), + }); + } + } + unreachable!("port exists on cell but not in signature. Internal structure is in an inconsistent state. This is an error please report it"); + } else if env.ctx().lookup_name(name) == target.as_ref() + { + return Ok(Path::AbstractPort { + cell: LazyCellPath { + concrete_prefix: self.concrete_path, + first_ref, + abstract_suffix: self.abstract_path, + }, + port: offset.into(), + }); + } + } + + // it's not a port + if let Some(cell) = info.prototype.as_component() { + *cell + } else { + return Err(TraversalError::Unknown( + target.as_ref().to_string(), + )); + } + } + }; + + // Check cells let current_comp_def = &env.ctx().secondary[current_comp_idx]; for (offset, def_idx) in current_comp_def.cell_offset_map.iter() { if env.ctx().lookup_name(env.ctx().secondary[*def_idx].name) @@ -173,23 +299,9 @@ impl Traverser { abstract_suffix: self.abstract_path, })); } - - for port in env.ctx().secondary[*def_idx].ports.iter() { - let port_def_idx = current_comp_def.port_offset_map[port]; - let port_name = env.ctx().secondary[port_def_idx].name; - if env.ctx().lookup_name(port_name) == target.as_ref() { - return Ok(Path::AbstractPort { - cell: LazyCellPath { - concrete_prefix: self.concrete_path, - first_ref, - abstract_suffix: self.abstract_path, - }, - port: port.into(), - }); - } - } } + // check ref_cells for (offset, def_idx) in current_comp_def.ref_cell_offset_map.iter() { if env.ctx().lookup_name(env.ctx().secondary[*def_idx].name) @@ -202,22 +314,6 @@ impl Traverser { abstract_suffix: self.abstract_path, })); } - - for port in env.ctx().secondary[*def_idx].ports.iter() { - let port_def_idx = - current_comp_def.ref_port_offset_map[port]; - let port_name = env.ctx().secondary[port_def_idx]; - if env.ctx().lookup_name(port_name) == target.as_ref() { - return Ok(Path::AbstractPort { - cell: LazyCellPath { - concrete_prefix: self.concrete_path, - first_ref, - abstract_suffix: self.abstract_path, - }, - port: port.into(), - }); - } - } } } else { let current_comp = *self.concrete_path.last().unwrap(); @@ -238,17 +334,6 @@ impl Traverser { ¤t_comp_ledger.index_bases + offset, )); } - - // check ports - for port in def.ports.iter() { - let name = env.ctx().secondary - [comp_def.port_offset_map[port]] - .name; - if env.ctx().lookup_name(name) == target.as_ref() { - let port_idx = ¤t_comp_ledger.index_bases + port; - return Ok(Path::Port(port_idx)); - } - } } // check ref cells @@ -268,6 +353,41 @@ impl Traverser { // we don't check ports here since there can't be ref ports // without first having visited a ref cell } + + // check ports + if current_comp == *self.concrete_path.last().unwrap() { + for port in + env.ctx().secondary[current_comp_ledger.comp_id].signature() + { + let def_idx = env.ctx().secondary + [current_comp_ledger.comp_id] + .port_offset_map[port]; + let port_name = env.ctx().secondary[def_idx].name; + if env.ctx().lookup_name(port_name) == target.as_ref() { + let port = ¤t_comp_ledger.index_bases + port; + return Ok(Path::Port(port)); + } + } + } else { + let local_offset = *self.concrete_path.last().unwrap() + - ¤t_comp_ledger.index_bases; + let cell_def_idx = &env.ctx().secondary + [current_comp_ledger.comp_id] + .cell_offset_map[local_offset]; + let cell_def = &env.ctx().secondary[*cell_def_idx]; + for port in cell_def.ports.iter() { + let port_def_idx = env.ctx().secondary + [current_comp_ledger.comp_id] + .port_offset_map[port]; + let port_name = env.ctx().secondary[port_def_idx].name; + + if env.ctx().lookup_name(port_name) == target.as_ref() { + return Ok(Path::Port( + ¤t_comp_ledger.index_bases + port, + )); + } + } + } } Err(TraversalError::Unknown(target.as_ref().to_string())) @@ -281,6 +401,104 @@ pub struct LazyCellPath { pub abstract_suffix: SmallVec<[CellRef; 2]>, } +impl LazyCellPath { + pub fn terminal_comp + Clone>( + &self, + env: &Environment, + ) -> ComponentIdx { + let last = self.concrete_prefix.last().unwrap(); + let ledger = env.cells[*last].as_comp().unwrap(); + let offset = self.first_ref - &ledger.index_bases; + let cell_idx = + env.ctx().secondary[ledger.comp_id].ref_cell_offset_map[offset]; + let mut current_comp = *env.ctx().secondary[cell_idx] + .prototype + .as_component() + .unwrap_or(&ledger.comp_id); + + for cell_ref in self.abstract_suffix.iter() { + match cell_ref { + CellRef::Local(l) => { + let local_def = + env.ctx().secondary[current_comp].cell_offset_map[*l]; + + if let CellPrototype::Component(c) = + env.ctx().secondary[local_def].prototype + { + current_comp = c; + } + } + CellRef::Ref(r) => { + let ref_def = env.ctx().secondary[current_comp] + .ref_cell_offset_map[*r]; + if let CellPrototype::Component(c) = + env.ctx().secondary[ref_def].prototype + { + current_comp = c; + } + } + } + } + current_comp + } + + pub fn as_string + Clone>( + &self, + env: &Environment, + ) -> String { + let last = self.concrete_prefix.last().unwrap(); + let mut path = env.get_full_name(last); + let ledger = env.cells[*last].as_comp().unwrap(); + let offset = self.first_ref - &ledger.index_bases; + let cell_idx = + env.ctx().secondary[ledger.comp_id].ref_cell_offset_map[offset]; + path.push('.'); + path.push_str( + env.ctx().lookup_name(env.ctx().secondary[cell_idx].name), + ); + + let mut current_comp = *env.ctx().secondary[cell_idx] + .prototype + .as_component() + .unwrap_or(&ledger.comp_id); + + for cell_ref in self.abstract_suffix.iter() { + match cell_ref { + CellRef::Local(l) => { + let local_def = + env.ctx().secondary[current_comp].cell_offset_map[*l]; + path.push('.'); + path.push_str( + env.ctx() + .lookup_name(env.ctx().secondary[local_def].name), + ); + if let CellPrototype::Component(c) = + env.ctx().secondary[local_def].prototype + { + current_comp = c; + } + } + CellRef::Ref(r) => { + let ref_def = env.ctx().secondary[current_comp] + .ref_cell_offset_map[*r]; + path.push('.'); + path.push_str( + env.ctx() + .lookup_name(env.ctx().secondary[ref_def].name), + ); + if let CellPrototype::Component(c) = + env.ctx().secondary[ref_def].prototype + { + current_comp = c; + } + } + } + } + + path + } +} + #[derive(Debug, Clone)] pub enum Path { Cell(GlobalCellIdx), @@ -312,10 +530,9 @@ impl Path { current = ref_actual; } else { // todo griffin: improve error message - return Err(ResolvePathError(format!( - "", - ref_global_offset - ))); + return Err(ResolvePathError( + env.get_full_name(ref_global_offset), + )); } } } @@ -323,10 +540,7 @@ impl Path { Ok(current) } else { // todo griffin: improve error message - Err(ResolvePathError(format!( - "", - path.first_ref - ))) + Err(ResolvePathError(env.get_full_name(path.first_ref))) } } @@ -379,9 +593,30 @@ impl Path { match self { Path::Cell(c) => env.get_full_name(c), Path::Port(p) => env.get_full_name(p), - Path::AbstractCell(_) => todo!("ref cells not supported yet"), - Path::AbstractPort { .. } => { - todo!("ref ports not supported yet") + Path::AbstractCell(path) => path.as_string(env), + Path::AbstractPort { cell, port } => { + let mut path = cell.as_string(env); + let comp = cell.terminal_comp(env); + match port { + PortRef::Local(l) => { + let idx = env.ctx().secondary[comp].port_offset_map[*l]; + path.push('.'); + path.push_str( + env.ctx() + .lookup_name(env.ctx().secondary[idx].name), + ); + } + PortRef::Ref(r) => { + let idx = + env.ctx().secondary[comp].ref_port_offset_map[*r]; + path.push('.'); + path.push_str( + env.ctx().lookup_name(env.ctx().secondary[idx]), + ); + } + } + + path } } }