Update the error messages

calyxir · Nov 26, 2024 · 29a0b50 · 29a0b50
1 parent eb43d21
commit 29a0b50
Show file tree

Hide file tree

Showing 8 changed files with 61 additions and 28 deletions.
diff --git a/interp/src/errors.rs b/interp/src/errors.rs
@@ -323,8 +323,20 @@ impl RuntimeError {
             RuntimeError::UndefinedReadAddr(c) => CiderError::GenericError(format!("Attempted to read from an undefined memory address from memory named \"{}\"", env.get_full_name(c))),
             RuntimeError::ClockError(clk) => {
                 match clk {
-                    ClockError::ReadWrite(c) => CiderError::GenericError(format!("Concurrent read & write to the same register/memory {}", env.get_full_name(c).underline())),
-                    ClockError::WriteWrite(c) => CiderError::GenericError(format!("Concurrent writes to the same register/memory {}", env.get_full_name(c).underline())),
+                    ClockError::ReadWrite(c, num) => {
+                        if let Some(entry_number) = num {
+                            CiderError::GenericError(format!("Concurrent read & write to the same memory {} in slot {}", env.get_full_name(c).underline(), entry_number))
+                        } else {
+                            CiderError::GenericError(format!("Concurrent read & write to the same register {}", env.get_full_name(c).underline()))
+                        }
+                },
+                    ClockError::WriteWrite(c, num) => {
+                        if let Some(entry_number) = num {
+                            CiderError::GenericError(format!("Concurrent writes to the same memory {} in slot {}", env.get_full_name(c).underline(), entry_number))
+                        } else {
+                            CiderError::GenericError(format!("Concurrent writes to the same register {}", env.get_full_name(c).underline()))
+                        }
+                    },
                     c => CiderError::GenericError(format!("Unexpected clock error: {c:?}")),
                 }
             }

diff --git a/interp/src/flatten/primitives/stateful/memories.rs b/interp/src/flatten/primitives/stateful/memories.rs
@@ -175,7 +175,7 @@ impl RaceDetectionPrimitive for StdReg {
             self.internal_state
                 .clocks
                 .check_write(current_clock_idx, clock_map)
-                .map_err(|e| e.add_cell_info(self.global_idx))?;
+                .map_err(|e| e.add_cell_info(self.global_idx, None))?;
         }
 
         self.exec_cycle(port_map)
@@ -679,7 +679,12 @@ impl RaceDetectionPrimitive for CombMem {
                     if port_map[self.write_en()].as_bool().unwrap_or_default() {
                         val.clocks
                             .check_write(thread_clock, clock_map)
-                            .map_err(|e| e.add_cell_info(self.global_idx))?;
+                            .map_err(|e| {
+                                e.add_cell_info(
+                                    self.global_idx,
+                                    Some(addr.try_into().unwrap()),
+                                )
+                            })?;
                     }
                 } else if addr != 0
                     || port_map[self.write_en()].as_bool().unwrap_or_default()
@@ -994,7 +999,12 @@ impl RaceDetectionPrimitive for SeqMem {
                             ),
                             clock_map,
                         )
-                        .map_err(|e| e.add_cell_info(self.global_idx))?;
+                        .map_err(|e| {
+                            e.add_cell_info(
+                                self.global_idx,
+                                Some(addr.try_into().unwrap()),
+                            )
+                        })?;
                 } else if port_map[self.content_enable()]
                     .as_bool()
                     .unwrap_or_default()
@@ -1009,7 +1019,12 @@ impl RaceDetectionPrimitive for SeqMem {
                             ),
                             clock_map,
                         )
-                        .map_err(|e| e.add_cell_info(self.global_idx))?;
+                        .map_err(|e| {
+                            e.add_cell_info(
+                                self.global_idx,
+                                Some(addr.try_into().unwrap()),
+                            )
+                        })?;
                 }
             }
         }

diff --git a/interp/src/flatten/structures/environment/clock.rs b/interp/src/flatten/structures/environment/clock.rs
@@ -427,9 +427,10 @@ impl ClockPair {
         (thread, reading_clock): ThreadClockPair,
         clock_map: &mut ClockMap,
         cell: GlobalCellIdx,
+        entry_number: Option<u32>,
     ) -> Result<(), ClockError> {
         self.check_read((thread, reading_clock), clock_map)
-            .map_err(|e| e.add_cell_info(cell))
+            .map_err(|e| e.add_cell_info(cell, entry_number))
     }
 
     pub fn check_write(
@@ -471,16 +472,24 @@ pub enum ClockError {
     #[error("Concurrent writes to the same register/memory")]
     WriteWriteUnhelpful,
     #[error("Concurrent read & write to the same register/memory {0:?}")]
-    ReadWrite(GlobalCellIdx),
+    ReadWrite(GlobalCellIdx, Option<u32>),
     #[error("Concurrent writes to the same register/memory {0:?}")]
-    WriteWrite(GlobalCellIdx),
+    WriteWrite(GlobalCellIdx, Option<u32>),
 }
 
 impl ClockError {
-    pub fn add_cell_info(self, cell: GlobalCellIdx) -> Self {
+    pub fn add_cell_info(
+        self,
+        cell: GlobalCellIdx,
+        entry_number: Option<u32>,
+    ) -> Self {
         match self {
-            ClockError::ReadWriteUnhelpful => ClockError::ReadWrite(cell),
-            ClockError::WriteWriteUnhelpful => ClockError::WriteWrite(cell),
+            ClockError::ReadWriteUnhelpful => {
+                ClockError::ReadWrite(cell, entry_number)
+            }
+            ClockError::WriteWriteUnhelpful => {
+                ClockError::WriteWrite(cell, entry_number)
+            }
             _ => self,
         }
     }

diff --git a/interp/src/flatten/structures/environment/env.rs b/interp/src/flatten/structures/environment/env.rs
@@ -3,7 +3,7 @@ use super::{
         context::Context, index_trait::IndexRange, indexed_map::IndexedMap,
     },
     assignments::{GroupInterfacePorts, ScheduledAssignments},
-    clock::{ClockMap, VectorClock},
+    clock::ClockMap,
     program_counter::{
         ControlTuple, ParEntry, PcMaps, ProgramCounter, WithEntry,
     },
@@ -874,7 +874,7 @@ impl<C: AsRef<Context> + Clone> Environment<C> {
 
     /// Attempt to find the parent cell for a port. If no such cell exists (i.e.
     /// it is a hole port, then it returns None)
-    fn get_parent_cell_from_port(
+    fn _get_parent_cell_from_port(
         &self,
         port: PortRef,
         comp: GlobalCellIdx,
@@ -2853,25 +2853,22 @@ impl<C: AsRef<Context> + Clone> Simulator<C> {
             // TODO griffin: Sort this out
             panic!("Value has both direct clock and transitive clock. This shouldn't happen?")
         } else if let Some(clocks) = val.clocks() {
-            let port_cell = self
-                .env
-                .get_parent_cell_from_port(port, active_cell)
-                .unwrap();
+            let info = clock_map.lookup_cell(clocks).expect("Clock pair without cell. This should never happen, please report this bug");
             clocks.check_read_w_cell(
                 (thread, thread_clock),
                 clock_map,
-                port_cell,
+                info.attached_cell,
+                info.entry_number,
             )?
         } else if let Some(transitive_clocks) = val.transitive_clocks() {
-            let port_cell = self
-                .env
-                .get_parent_cell_from_port(port, active_cell)
-                .unwrap();
             for clock_pair in transitive_clocks {
+                let info = clock_map.lookup_cell(*clock_pair).expect("Clock pair without cell. This should never happen, please report this bug");
+
                 clock_pair.check_read_w_cell(
                     (thread, thread_clock),
                     clock_map,
-                    port_cell,
+                    info.attached_cell,
+                    info.entry_number,
                 )?
             }
         }

diff --git a/interp/tests/data-race/continuous-race.expect b/interp/tests/data-race/continuous-race.expect
@@ -1,4 +1,4 @@
 ---CODE---
 1
 ---STDERR---
-Error: Concurrent read & write to the same register/memory [4mmain.val[0m
+Error: Concurrent read & write to the same register [4mmain.val[0m
diff --git a/interp/tests/data-race/guard-conflict.expect b/interp/tests/data-race/guard-conflict.expect
@@ -1,4 +1,4 @@
 ---CODE---
 1
 ---STDERR---
-Error: Concurrent read & write to the same register/memory [4mmain.cond_reg[0m
+Error: Concurrent read & write to the same register [4mmain.cond_reg[0m
diff --git a/interp/tests/data-race/par-conflict-cmem.expect b/interp/tests/data-race/par-conflict-cmem.expect
@@ -1,4 +1,4 @@
 ---CODE---
 1
 ---STDERR---
-Error: Concurrent read & write to the same register/memory [4mmain.cond_mem[0m
+Error: Concurrent read & write to the same memory [4mmain.cond_mem[0m in slot 0
diff --git a/interp/tests/data-race/par-conflict.expect b/interp/tests/data-race/par-conflict.expect
@@ -1,4 +1,4 @@
 ---CODE---
 1
 ---STDERR---
-Error: Concurrent read & write to the same register/memory [4mmain.cond_reg[0m
+Error: Concurrent read & write to the same register [4mmain.cond_reg[0m