allow for larger var index

Cargo.toml

@@ -8,7 +8,7 @@ members = ["wellen"]
 default-members = ["wellen"]
 
 [workspace.package]
-version = "0.9.18"
+version = "0.9.19"
 edition = "2021"
 # we require the `div_ceil` method on integers
 rust-version = "1.73.0"

wellen/src/ghw/hierarchy.rs

@@ -1341,9 +1341,7 @@ impl IntRange {
     }
 
     fn as_var_index(&self) -> VarIndex {
-        let msb = self.1 as i32;
-        let lsb = self.2 as i32;
-        VarIndex::new(msb, lsb)
+        VarIndex::new(self.1, self.2)
     }
 
     fn from_i32_option(opt: Option<Self>) -> Self {

wellen/src/hierarchy.rs

@@ -6,7 +6,7 @@
 
 use crate::FileFormat;
 use std::collections::HashMap;
-use std::num::{NonZeroU16, NonZeroU32, NonZeroU64};
+use std::num::{NonZeroI32, NonZeroU16, NonZeroU32};
 
 #[derive(Debug, Clone, Copy, PartialEq)]
 #[cfg_attr(feature = "serde1", derive(serde::Serialize, serde::Deserialize))]
@@ -208,23 +208,33 @@ impl VarDirection {
 }
 
 #[derive(Debug, Clone, Copy, Eq, PartialEq)]
+#[repr(packed(4))]
 #[cfg_attr(feature = "serde1", derive(serde::Serialize, serde::Deserialize))]
-pub struct VarIndex(NonZeroU64);
+pub struct VarIndex {
+    lsb: i64,
+    width: NonZeroI32,
+}
+
+const DEFAULT_ZERO_REPLACEMENT: NonZeroI32 = unsafe { NonZeroI32::new_unchecked(i32::MIN) };
 
 impl VarIndex {
-    pub fn new(msb: i32, lsb: i32) -> Self {
-        let value = ((msb as u64) << 32) | ((lsb as u64) & (u32::MAX as u64));
-        Self(NonZeroU64::new(value + 1).unwrap())
+    pub fn new(msb: i64, lsb: i64) -> Self {
+        let width = NonZeroI32::new((msb - lsb) as i32).unwrap_or(DEFAULT_ZERO_REPLACEMENT);
+        Self { lsb, width }
     }
 
-    pub fn msb(&self) -> i32 {
-        let value = self.0.get() - 1;
-        (value >> 32) as i32
+    #[inline]
+    pub fn msb(&self) -> i64 {
+        if self.width == DEFAULT_ZERO_REPLACEMENT {
+            self.lsb()
+        } else {
+            self.width.get() as i64 + self.lsb()
+        }
     }
 
-    pub fn lsb(&self) -> i32 {
-        let value = self.0.get() - 1;
-        (value & (u32::MAX as u64)) as i32
+    #[inline]
+    pub fn lsb(&self) -> i64 {
+        self.lsb
     }
 }
 
@@ -1189,19 +1199,28 @@ mod tests {
         // 4 byte length + tag + padding
         assert_eq!(std::mem::size_of::<SignalEncoding>(), 8);
 
+        // var index is packed in order to take up 12 bytes and contains a NonZero field to allow
+        // for zero cost optioning
+        assert_eq!(
+            std::mem::size_of::<VarIndex>(),
+            std::mem::size_of::<Option<VarIndex>>()
+        );
+        assert_eq!(std::mem::size_of::<VarIndex>(), 12);
+
         // Var
         assert_eq!(
             std::mem::size_of::<Var>(),
-            std::mem::size_of::<HierarchyStringId>() // name
-                + 1 // tpe
-                + 1 // direction
-                + 16 // signal tpe
+            std::mem::size_of::<HierarchyStringId>()        // name
+                + std::mem::size_of::<VarType>()            // var_tpe
+                + std::mem::size_of::<VarDirection>()       // direction
+                + std::mem::size_of::<SignalEncoding>()     // signal_encoding
+                + std::mem::size_of::<Option<VarIndex>>()   // index
+                + std::mem::size_of::<SignalRef>()          // signal_idx
                 + std::mem::size_of::<Option<EnumTypeId>>() // enum type
-                + std::mem::size_of::<HierarchyStringId>() // VHDL type name
-                + std::mem::size_of::<SignalRef>() // handle
-                + std::mem::size_of::<ScopeRef>() // parent
-                + std::mem::size_of::<HierarchyItemId>() // next
-                + 6 // padding
+                + std::mem::size_of::<HierarchyStringId>()  // VHDL type name
+                + std::mem::size_of::<Option<ScopeRef>>()   // parent
+                + std::mem::size_of::<HierarchyItemId>()    // next
+                + 2 // padding
         );
         // currently this all comes out to 48 bytes (~= 6x 64-bit pointers)
         assert_eq!(std::mem::size_of::<Var>(), 48);

wellen/src/vcd.rs

@@ -401,19 +401,20 @@ fn find_last(haystack: &[u8], needle: u8) -> Option<usize> {
 #[inline]
 fn parse_inner_index(index: &[u8]) -> Option<VarIndex> {
     let sep = index.iter().position(|b| *b == b':');
+    println!("{}", std::str::from_utf8(index).unwrap());
     match sep {
         None => {
             let inner_str = std::str::from_utf8(index).unwrap();
-            let bit = inner_str.parse::<i32>().unwrap();
+            let bit = inner_str.parse::<i64>().unwrap();
             Some(VarIndex::new(bit, bit))
         }
         Some(pos) => {
             let msb_bytes = &index[0..pos];
             let msb_str = std::str::from_utf8(msb_bytes).unwrap();
-            let msb = msb_str.parse::<i32>().unwrap();
+            let msb = msb_str.parse::<i64>().unwrap();
             let lsb_bytes = &index[(pos + 1)..index.len()];
             let lsb_str = std::str::from_utf8(lsb_bytes).unwrap();
-            let lsb = lsb_str.parse::<i32>().unwrap();
+            let lsb = lsb_str.parse::<i64>().unwrap();
             Some(VarIndex::new(msb, lsb))
         }
     }
@@ -1305,7 +1306,7 @@ x%i"
         assert_eq!(find_last(b"12341", b'1'), Some(4));
     }
 
-    fn do_test_parse_name(full_name: &str, name: &str, index: Option<(i32, i32)>, scopes: &[&str]) {
+    fn do_test_parse_name(full_name: &str, name: &str, index: Option<(i64, i64)>, scopes: &[&str]) {
         let (a_name, a_index, a_scopes) = parse_name(full_name.as_bytes()).unwrap();
         assert_eq!(a_name, name);
         match index {

wellen/tests/diff_tests.rs

@@ -212,12 +212,12 @@ fn diff_hierarchy_item(
             match ref_var.index {
                 None => assert!(our_var.index().is_none() || ref_name_contains_index),
                 Some(::vcd::ReferenceIndex::BitSelect(bit)) => {
-                    assert_eq!(our_var.index().unwrap().msb(), bit);
-                    assert_eq!(our_var.index().unwrap().lsb(), bit);
+                    assert_eq!(our_var.index().unwrap().msb(), bit as i64);
+                    assert_eq!(our_var.index().unwrap().lsb(), bit as i64);
                 }
                 Some(::vcd::ReferenceIndex::Range(msb, lsb)) => {
-                    assert_eq!(our_var.index().unwrap().msb(), msb);
-                    assert_eq!(our_var.index().unwrap().lsb(), lsb);
+                    assert_eq!(our_var.index().unwrap().msb(), msb as i64);
+                    assert_eq!(our_var.index().unwrap().lsb(), lsb as i64);
                 }
             }
         }

wellen/tests/fst.rs

@@ -222,3 +222,11 @@ fn test_nvc_vhdl_test_bool_issue_16() {
         ]
     );
 }
+
+/// This file was provided by Tiago Gomes in the following issue:
+/// https://github.com/ekiwi/wellen/issues/21
+#[test]
+fn test_nvc_overlay_tb_issue_21() {
+    // This used to crash because one of the variables used an index that does not fit into 32 bits.
+    let _ = read("inputs/nvc/overlay_tb_issue_21.fst").unwrap();
+}