fix: bug in bitset unset function (#16)

* fix: bug in bitset unset function

* fix: clippy warnings

* add: clippy check on CI

Author: qjerome

.github/workflows/rust.yml

@@ -16,6 +16,8 @@ jobs:
 
     steps:
     - uses: actions/checkout@v3
+    - name: Clippy
+      run: cargo clippy -- -D warnings
     - name: Build
       run: cargo build --release --verbose
     - name: Run tests

poppy/src/bitset/array.rs

@@ -1,8 +1,15 @@
+
 /// Array based bitset implementation.
 /// N gives the size in Bytes of the bucket
 #[derive(Debug, Clone)]
 pub struct BitSet<const N: usize>([u8; N]);
 
+impl<const N: usize> Default for BitSet<N> {
+    fn default() -> Self {
+        Self([0u8; N])
+    }
+}
+
 impl<const N: usize> BitSet<N> {
     /// Creates a new bitset
     pub const fn new() -> Self {
@@ -15,7 +22,11 @@ impl<const N: usize> BitSet<N> {
         // equivalent to index % 8
         let mask = (value as u8) << (index & 7);
         let old = self.0[iblock] & mask == mask;
-        self.0[iblock] |= mask;
+        if value {
+            self.0[iblock] |= mask;
+        } else {
+            self.0[iblock] &= mask;
+        }
         old
     }
 
@@ -131,3 +142,131 @@ impl<const N: usize> BitSet<N> {
         N * 8
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn test_new_bitset() {
+        let bitset: BitSet<2> = BitSet::new();
+        assert_eq!(bitset.as_slice(), &[0u8; 2]);
+    }
+
+    #[test]
+    fn test_default_bitset() {
+        let bitset: BitSet<2> = BitSet::default();
+        assert_eq!(bitset.as_slice(), &[0u8; 2]);
+    }
+
+    #[test]
+    fn test_set_nth_bit() {
+        let mut bitset: BitSet<2> = BitSet::new();
+        assert!(!bitset.set_nth_bit(0)); // Bit was 0, now set to 1
+        assert!(bitset.get_nth_bit(0));
+    }
+
+    #[test]
+    fn test_unset_nth_bit() {
+        let mut bitset: BitSet<2> = BitSet::new();
+        bitset.set_nth_bit(0); // Set bit to 1 first
+        assert!(bitset.unset_nth_bit(0)); // Bit was 1, now set to 0
+        assert!(!bitset.get_nth_bit(0));
+    }
+
+    #[test]
+    fn test_get_nth_bit() {
+        let mut bitset: BitSet<2> = BitSet::new();
+        bitset.set_nth_bit(0);
+        assert!(bitset.get_nth_bit(0));
+        assert!(!bitset.get_nth_bit(1));
+    }
+
+    #[test]
+    #[should_panic(expected = "index out of bounds")]
+    fn test_set_nth_bit_out_of_bounds() {
+        let mut bitset: BitSet<2> = BitSet::new();
+        bitset.set_nth_bit(16); // Assuming N=2, so 16 bits is out of bounds
+    }
+
+    #[test]
+    fn test_clear() {
+        let mut bitset: BitSet<2> = BitSet::new();
+        bitset.set_nth_bit(0);
+        bitset.set_nth_bit(1);
+        bitset.clear();
+        assert_eq!(bitset.as_slice(), &[0u8; 2]);
+    }
+
+    #[test]
+    fn test_count_ones() {
+        let mut bitset: BitSet<2> = BitSet::new();
+        assert_eq!(bitset.count_ones(), 0);
+        bitset.set_nth_bit(0);
+        bitset.set_nth_bit(1);
+        assert_eq!(bitset.count_ones(), 2);
+    }
+
+    #[test]
+    fn test_count_zeros() {
+        let mut bitset: BitSet<2> = BitSet::new();
+        assert_eq!(bitset.count_zeros(), 16); // 2 bytes = 16 bits, all zeros initially
+        bitset.set_nth_bit(0);
+        assert_eq!(bitset.count_zeros(), 15);
+    }
+
+    #[test]
+    fn test_union() {
+        let mut bitset1: BitSet<2> = BitSet::new();
+        let mut bitset2: BitSet<2> = BitSet::new();
+        bitset1.set_nth_bit(0);
+        bitset2.set_nth_bit(1);
+        bitset1.union(&bitset2);
+        assert!(bitset1.get_nth_bit(0));
+        assert!(bitset1.get_nth_bit(1));
+    }
+
+    #[test]
+    fn test_intersection() {
+        let mut bitset1: BitSet<2> = BitSet::new();
+        let mut bitset2: BitSet<2> = BitSet::new();
+        bitset1.set_nth_bit(0);
+        bitset1.set_nth_bit(1);
+        bitset2.set_nth_bit(1);
+        bitset1.intersection(&bitset2);
+        assert!(!bitset1.get_nth_bit(0));
+        assert!(bitset1.get_nth_bit(1));
+    }
+
+    #[test]
+    fn test_count_ones_in_common() {
+        let mut bitset1: BitSet<2> = BitSet::new();
+        let mut bitset2: BitSet<2> = BitSet::new();
+        bitset1.set_nth_bit(0);
+        bitset1.set_nth_bit(1);
+        bitset2.set_nth_bit(1);
+        assert_eq!(bitset1.count_ones_in_common(&bitset2), 1);
+    }
+
+    #[test]
+    fn test_bit_len() {
+        let bitset: BitSet<2> = BitSet::new();
+        assert_eq!(bitset.bit_len(), 16); // 2 bytes = 16 bits
+    }
+
+    #[test]
+    fn test_byte_len() {
+        let bitset: BitSet<2> = BitSet::new();
+        assert_eq!(bitset.byte_len(), 2);
+    }
+
+    #[test]
+    fn test_byte_size() {
+        assert_eq!(BitSet::<2>::byte_size(), 2);
+    }
+
+    #[test]
+    fn test_bit_size() {
+        assert_eq!(BitSet::<2>::bit_size(), 16);
+    }
+}

poppy/src/bitset/vec.rs

@@ -55,7 +55,11 @@ impl VecBitSet {
         // equivalent to index % 8
         let mask = (value as u8) << (index & 7);
         let old = self.0[iblock] & mask == mask;
-        self.0[iblock] |= mask;
+        if value {
+            self.0[iblock] |= mask;
+        } else {
+            self.0[iblock] &= mask;
+        }
         old
     }
 
@@ -149,3 +153,108 @@ impl VecBitSet {
         self.0.iter().map(|b| b.count_zeros() as usize).sum()
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn test_with_bit_capacity() {
+        let capacity = 10;
+        let bitset = VecBitSet::with_bit_capacity(capacity);
+        assert_eq!(bitset.bit_len(), 16); // because of byte alignment, with capacity 10 it's 16 bits (2 bytes)
+        assert_eq!(bitset.byte_len(), 2);
+    }
+
+    #[test]
+    fn test_is_empty() {
+        let bitset = VecBitSet::with_bit_capacity(0);
+        assert!(bitset.is_empty());
+    }
+
+    #[test]
+    fn test_set_nth_bit() {
+        let mut bitset = VecBitSet::with_bit_capacity(8);
+        assert!(!bitset.get_nth_bit(0));
+        bitset.set_nth_bit(0);
+        assert!(bitset.get_nth_bit(0));
+    }
+
+    #[test]
+    fn test_unset_nth_bit() {
+        let mut bitset = VecBitSet::with_bit_capacity(8);
+        bitset.set_nth_bit(0);
+        assert!(bitset.get_nth_bit(0));
+        bitset.unset_nth_bit(0);
+        assert!(!bitset.get_nth_bit(0));
+    }
+
+    #[test]
+    fn test_clear() {
+        let mut bitset = VecBitSet::with_bit_capacity(8);
+        bitset.set_nth_bit(0);
+        bitset.set_nth_bit(1);
+        bitset.clear();
+        assert_eq!(bitset.count_ones(), 0);
+    }
+
+    #[test]
+    fn test_union() {
+        let mut bitset1 = VecBitSet::with_bit_capacity(8);
+        let mut bitset2 = VecBitSet::with_bit_capacity(8);
+        bitset1.set_nth_bit(0);
+        bitset2.set_nth_bit(1);
+        bitset1.union(&bitset2);
+        assert!(bitset1.get_nth_bit(0));
+        assert!(bitset1.get_nth_bit(1));
+    }
+
+    #[test]
+    fn test_intersection() {
+        let mut bitset1 = VecBitSet::with_bit_capacity(8);
+        let mut bitset2 = VecBitSet::with_bit_capacity(8);
+        bitset1.set_nth_bit(0);
+        bitset1.set_nth_bit(1);
+        bitset2.set_nth_bit(1);
+        bitset2.set_nth_bit(2);
+        bitset1.intersection(&bitset2);
+        assert!(!bitset1.get_nth_bit(0));
+        assert!(bitset1.get_nth_bit(1));
+        assert!(!bitset1.get_nth_bit(2));
+    }
+
+    #[test]
+    fn test_count_ones_in_common() {
+        let mut bitset1 = VecBitSet::with_bit_capacity(8);
+        let mut bitset2 = VecBitSet::with_bit_capacity(8);
+        bitset1.set_nth_bit(0);
+        bitset1.set_nth_bit(1);
+        bitset2.set_nth_bit(1);
+        bitset2.set_nth_bit(2);
+        assert_eq!(bitset1.count_ones_in_common(&bitset2), 1);
+    }
+
+    #[test]
+    fn test_count_ones() {
+        let mut bitset = VecBitSet::with_bit_capacity(8);
+        bitset.set_nth_bit(0);
+        bitset.set_nth_bit(1);
+        assert_eq!(bitset.count_ones(), 2);
+    }
+
+    #[test]
+    fn test_count_zeros() {
+        let mut bitset = VecBitSet::with_bit_capacity(8);
+        bitset.set_nth_bit(0);
+        bitset.set_nth_bit(1);
+        assert_eq!(bitset.count_zeros(), 6);
+    }
+
+    #[test]
+    #[should_panic]
+    fn test_out_of_bounds_access() {
+        let bitset = VecBitSet::with_bit_capacity(8);
+        // should panic but handled in the test if it does.
+        bitset.get_nth_bit(8);
+    }
+}