Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug?: BitVec's .as_bool() function returns negated value instead of the actual bit value #207

Open
shrugalic opened this issue Dec 31, 2022 · 2 comments

Comments

@shrugalic
Copy link
Contributor

shrugalic commented Dec 31, 2022

Because I didn't know any better, when I first used BitVec (version 1.0.1) yesterday I called .as_bool() on a particular bit of a BitVec, instead of just evaluating the bit.

Surprisingly, it returned the negated bit value instead of the expected value. Here's a test to demonstrate:

#[test]
fn bitvec_bool() {
    let bits: BitVec = [true, false].into_iter().collect();
    assert!(bits[0]); // works
    assert!(!bits[1]); // works
    assert!(bits[0].as_bool()); // fails
    assert!(!bits[1].as_bool()); // fails
}

The code that causes this is part of funty 2.0.0. Unfortunately, I couldn't find an appropriate branch of funty to create a PR.

It looks like this will be fixed in funty 3.0 (its main branch seems to be the 3.0 RC1), as the code for as_bool() for bool was replaced by this:

impl Fundamental for bool {
	#[inline(always)]
	fn as_bool(self) -> bool {
		self
	}

I do want to mention that the impl_for macro reads a bit strange to me, but then again I don't have much experience with macros. It reads like this:

macro_rules! impl_for {
	(Fundamental => $($t:ty => $is_zero:expr),+ $(,)?) => { $(
		impl Fundamental for $t {
			#[inline(always)]
			#[allow(clippy::redundant_closure_call)]
			fn as_bool(self) -> bool { ($is_zero)(self) }

Here's one example usage (of many more):

impl_for!(Fundamental =>
	i8 => |this| this != 0,

So the filter predicate closure in this case (and many other cases) is |this| this != 0, and the strange (to me) part about this is that it is called $is_zero in the macro. It would make sense to me if it were called either $is_NOT_zero or is_true or bit_is_set or anything along those lines.

Or maybe I completely mis-understand the purpose of as_bool()?

@myrrlyn
Copy link
Collaborator

myrrlyn commented Apr 12, 2023

Hi! I apologize for the long delay; I have had a very stressful winter and have not had the time or energy to check this repository in a timely manner. I'm trying to catch up now.


Okay, so, bitslice[index] already produces a bool; you shouldn't need to change its type. But this is definitely a logic error in funty.

I'll have to fix it in funty 2, since bitvec 1 is going to keep using that. Thank you for bringing it to my attention.

@ntc2
Copy link

ntc2 commented Dec 26, 2023

I just got bit by this, very confusing!

In my case, I was using as_bool() because I'm not familiar with BitVec and some API I'm calling returned BitVec. I wanted to iterate over the bits, so I did .iter(), and then I ended up with a BitRef, and I called as_bool() on that, causing the problem illustrated by the OP.

For completeness here's my minimal test case:

// use bitvec::prelude::*;
let interrogate_lsb = |x: BitVec<_, _>| {
    let lsb_array = x[0];
    let lsb_iter_as_bool = x.iter().next().unwrap().as_bool();
    let lsb_iter_as_u64 = x.iter().next().unwrap().as_u64();
    println!("x: {x:?}");
    println!("x[0]: {lsb_array:?}");
    println!("x.iter().next().unwrap().as_bool(): {lsb_iter_as_bool:?}");
    println!("x.iter().next().unwrap().as_u64(): {lsb_iter_as_u64:?}");
};
let mut bv = bitvec![u8, Lsb0;];
bv.push(false);
interrogate_lsb(bv);
println!();
let mut bv = bitvec![u8, Lsb0;];
bv.push(true);
interrogate_lsb(bv);

which prints

x: BitVec<u8, bitvec::order::Lsb0> { addr: 0x7fc434001f20, head: 000, bits: 1, capacity: 64 } [0]
x[0]: false
x.iter.next().unwrap(): BitRef<u8, bitvec::order::Lsb0> { addr: 0x7fc434001f20, head: 000, bits: 1, bit: false }
x.iter().next().unwrap().as_bool(): true
x.iter().next().unwrap().as_u64(): 0

x: BitVec<u8, bitvec::order::Lsb0> { addr: 0x7fc434001f20, head: 000, bits: 1, capacity: 64 } [1]
x[0]: true
x.iter.next().unwrap(): BitRef<u8, bitvec::order::Lsb0> { addr: 0x7fc434001f20, head: 000, bits: 1, bit: true }
x.iter().next().unwrap().as_bool(): false
x.iter().next().unwrap().as_u64(): 1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants