Skip to content

BitwiseForSlice::is_unit only checks a single block, misses bits in the rest of the slice #53

@vadym-kl

Description

@vadym-kl

Summary

BitwiseForSlice::is_unit(index) (in src/bit/bitwise_for_slice.rs) returns true for slices that have set bits outside the single BitBlock
containing index, violating the contract documented on Bitwise::is_unit ("Returns true if exactly one bit is set and it's at the given
index").

The bug propagates through every type whose Bitwise impl ultimately delegates to a slice-of-BitBlock representation: AlignedBitVec,
AlignedBitView, BitVec, BitView, and BitMatrix::row(r) (since Row<'_> = BitView<'_>).

This is a silent correctness bug — wrong result, no panic.

Version

binar 0.1.1

Affected code

src/bit/bitwise_for_slice.rs, around line 38:

fn is_unit(&self, index: usize) -> bool {
    let (block_index, bit_index) = block_and_bit_index::<BitBlock>(index);
    self.borrow()[block_index].is_unit(bit_index)
}

Only self.borrow()[block_index] is examined. Other entries of self.borrow() are never inspected.

The sibling BitwiseForSlice::is_zero and BitwiseForSlice::weight (same file) both correctly iterate across all blocks.

Reproduction

  use binar::{BitMatrix, Bitwise, BitwiseMut};

  fn main() {
      // 128 columns — wide enough that cols 0 and 100 land in different 64-bit blocks.
      let mut m = BitMatrix::zeros(128, 128);
      m.set((0, 0), true);
      m.set((0, 100), true);

      let row = m.row(0);
      assert_eq!(row.weight(), 2);

      // Expected: both false (weight != 1).
      // Actual under 0.1.1: both return true.
      assert!(!row.is_unit(0), "row.is_unit(0) should be false");
      assert!(!row.is_unit(100), "row.is_unit(100) should be false");
  }

Expected behavior

Per the doc comment on Bitwise::is_unit:

Returns true if exactly one bit is set and it's at the given index.

For the example above, both calls should return false because the row has weight 2.

Actual behavior

Both row.is_unit(0) and row.is_unit(100) return true. Any code that uses is_unit to test "is this row/vector exactly a unit basis vector" silently
misbehaves for slices wider than one BitBlock (typically 64 bits) whenever set bits land in distinct blocks.

Suggested fix

The default Bitwise::is_unit implementation in src/bit/bitwise.rs is correct and slice-aware:

  fn is_unit(&self, index: usize) -> bool {
      self.weight() == 1 && self.index(index)
  }

The BitwiseForSlice override should match it (or delegate to it), e.g.:

  #[inline]
  fn is_unit(&self, index: usize) -> bool {
      self.weight() == 1 && BitwiseForSlice::index(self, index)
  }

That mirrors BitwiseForSlice::is_zero (line 34), which correctly walks every block.

Suggested regression test

  #[test]
  fn is_unit_rejects_extra_bits_in_other_blocks() {
      let mut v = BitVec::zeros(128); // spans two 64-bit blocks
      v.assign_index(0, true);
      v.assign_index(100, true);
      assert_eq!(v.weight(), 2);
      assert!(!v.is_unit(0));
      assert!(!v.is_unit(100));
  }

Fails on 0.1.1; passes after the fix above.

Workaround for downstream users

Replace slice.is_unit(i) with slice.weight() == 1 && slice.index(i). Same semantics; the slice weight() impl is already correct.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions