Skip to content

Commit

Permalink
Corrected bug in clearBit
Browse files Browse the repository at this point in the history
The default implementation of clearBit also clears
the bits to the left of the one passed as
parameter. The solution is to give an explicit
implementation.

Also, a quickCheck for clearBit has been added.

Review by Iago Abal <[email protected]>:
---------------------------------------
I personally think that the problem here is that certain operations like
.&. should sign-extend their operands rather than zero-extend. However,
that would be backwards incompatible, and I think this patch is a good
temporary fix. Long term, we should fix the real problem even if that
means changing the philosophy of the library a little bit.
---------------------------------------
  • Loading branch information
jvilar authored and IagoAbal committed Dec 19, 2019
1 parent 28171a6 commit 0aff564
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 0 deletions.
3 changes: 3 additions & 0 deletions src/Data/BitVector.hs
Original file line number Diff line number Diff line change
Expand Up @@ -747,6 +747,9 @@ instance Bits BV where
#endif
bit i = BV (i+1) (2^i)
{-# INLINE bit #-}
clearBit u@(BV n _) i = u .&. v
where v = bitVec n $ 2^n - 2^i - (1 :: Integer)
{-# INLINE clearBit #-}
testBit (BV n a) i | i < n = testBit a i
| otherwise = False
{-# INLINE testBit #-}
Expand Down
10 changes: 10 additions & 0 deletions test/Properties.hs
Original file line number Diff line number Diff line change
Expand Up @@ -271,3 +271,13 @@ prop_group_join_id a = forallDivisorOf (size a) $ \n ->

prop_show_read_id :: BV -> Bool
prop_show_read_id a = read (show a) ==. a

-- * clearBit

prop_clear_bit :: BV -> Property
prop_clear_bit a = forallIndexOf a $ \i ->
let cb = clearBit a i
in List.and
[ cb @. i' == ((i /= i') && a @. i')
| i' <- [0 .. size a ]
]

0 comments on commit 0aff564

Please sign in to comment.