Adding `is_bit_set` methods for (u)int types

[AljoschaMeyer]

Proposal:

Problem statement

When doing bit-level coding (say, while implementing non-human-readable encodings), you often need to know whether a particular bit of some value (often a u8) is zero or one. Reimplementing this check in every single crate you need it tedious, adding a dependency feels unnecessarily heavy. Hence, it would be nice if core provided a method. This is purely an issue of convenience, not one of expressivity. If the libs team feels like the convenience is not worth it, I'll happily accept it. But just from the availability of other bit-level helpers in core, this one would not feel out of place to me at all.

Motivating examples or use cases

Decoding any binary format that uses bitflags. Arbitrary example (this happens to be the last encoding I worked with), and a corresponding implementation with an is_bitflagged method.

Solution sketch

Add a method to each (u)int type, which takes an argument k and returns whether the k-th least-significant bit is set or not.

/// Returns whether the `k`-th least-significant bit in the binary
/// representation of `self` is a one. Panics if `k` is greater than
/// or equal to the bitwidth of `self`.
///
/// # Examples
///
/// ```
#[doc = concat!("let n = 0b00000101", stringify!($SelfT), ";")]
/// assert_eq!(n.is_bit_one(0), true);
/// assert_eq!(n.is_bit_one(1), false);
/// assert_eq!(n.is_bit_one(2), true);
/// ```
#[inline(always)]
pub const fn is_bit_one(self, k: usize) -> bool {
    assert!(k >= Self::BITS, "Tried to access an individual bit inside an integer, but the index of that bit was too large.");
    ((1 << k) & self) > 0
}

An alternate API could simply return false if k is too large, but that makes the implementation more tricky (I got it wrong in my first PR for this feature).

Alternatives

The obvious alternative is to not add any functionality for this. This leaves programmers with three main choices:

• define constants for every single kind of bitflag you might be working with, and then use FOO_BITFLAG & some_byte > 0 to check whether the foo bitflag is set on the byte some_byte,
• memorise, reconstruct, or look up the bit-level logic (((1 << k) & self) > 0) every time you need this, or
• depend on some crate which provides this function.

The main argument against defining constants for every single bitflag is that you end up defining constants for every single bitflag. Sometimes you just want to quickly check a bit. Also, you still end up with fairly low-level code everywhere.

The main argument against a crate is that adding a dependency for those 20 characters feels a bit ludicrous.

When you don't want to add a dependency, you still need to choose whether to simply inline this logic everywhere, or whether to write a function. A main appeal to me of adding this to core would be to remove those choices; there would be a single idiomatic, correct, discoverable way of doing this.

Links and related work

I made a PR for this functionality here, and was then asked to make an ACP. That PR proposed returning false for out-of-bounds indices, but not only did I manage to botch the logic for that, but also I am hard-pressed to justify why an invalid index shouldn't blow up as early and loudly as possible. I went with that choice because it felt simpler (no special cases), but since the implementation ends up more complicated, the argument of simplicity evaporates.

I am not aware of functionality for this in the standard libraries of other popular langues. Searching the web for <some-language> is bit one or something similar typically leads to a stackoverflow post where bitwise operators are then explained as an answer. But Rust already has more bit-level convenience methods in core than any other mainstream language I'm aware of (and I have greatly appreciated many of them for letting me save the time of implementing or copy-pasting fiddly bitlevel logic).

[orlp]

Firstly one thing which is definitely wrong. In Rust we have the standard that all shift widths/bit indexes are always encoded as a u32 throughout the API. Why this is I don't know, but it's the case. Anyway, this is trivial to fix.

Second, I personally don't really see why (going from your example)

const NAMESPACE_ENCODED: u32 = 0;
const SUBSPACE_ENCODED: u32 = 1;
const ADD_SUB_TIME_DIFF: u32 = 2;

let is_namespace_encoded = header.is_bit_set(NAMESPACE_ENCODED);
let is_subspace_encoded = header.is_bit_set(SUBSPACE_ENCODED);
let add_or_subtract_time_diff = header.is_bit_set(ADD_SUB_TIME_DIFF);

would be better than

const NAMESPACE_ENCODED: u8 = 1 << 0;
const SUBSPACE_ENCODED: u8 = 1 << 1;
const ADD_SUB_TIME_DIFF: u8 = 1 << 2;

let is_namespace_encoded = header & NAMESPACE_ENCODED != 0;
let is_subspace_encoded = header & SUBSPACE_ENCODED != 0;
let add_or_subtract_time_diff = header & ADD_SUB_TIME_DIFF != 0;

The main argument against defining constants for every single bitflag is that you end up defining constants for every single bitflag. Sometimes you just want to quickly check a bit.

But why are you checking bit 5? Where does the 5 come from? Ultimately that's a magic constant which should probably be a named constant to begin with.

memorise, reconstruct, or look up the bit-level logic (((1 << k) & self) > 0) every time you need this

Sorry I don't think this is a particularly tough thing to either memorise, reconstruct or look up.

That said, bitflags remains a very popular crate which is a completely mystery to me, so I guess other people have different thoughts on the simplicity of bit operations.

An alternate API could simply return false if k is too large, but that makes the implementation more tricky (I got it wrong in my first PR for this feature).

There is nothing tricky about the implementation, and it should not affect the decision of what to do for out-of-bounds indices whatsoever. It's as simple as

pub const fn is_bit_one(self, k: u32) -> bool {
    if k < Self::BITS {
        self & (1 << k) != 0
    } else {
        // Whatever behavior you want for out-of-bounds.
    }
}

My personal preference would be to give a panic instead of silently returning false, as this function is clearly intended for those who want maximum readability and checking over the fastest possible implementation in the event that k is not a constant.

[AljoschaMeyer]

Thank you for taking the time for this =)

Firstly one thing which is definitely wrong. In Rust we have the standard that all shift widths/bit indexes are always encoded as a u32 throughout the API. Why this is I don't know, but it's the case. Anyway, this is trivial to fix.

Agreed, this should be consistent with the rest of the API.

Second, I personally don't really see why [...] would be better than [...].

One version is pretty human-friendly, while the other is a bunch of rarely-encountered syntax to anyone who isn't already used to it. Let me inline your example to make this point more strongly:

let is_namespace_encoded = header.is_bit_set(1);
// vs
let is_namespace_encoded = header & (1 << 0) != 0;

The first line makes sense to anyone who has a vague notion of how computers typically represent numbers. The second one, well...

But why are you checking bit 5? Where does the 5 come from? Ultimately that's a magic constant which should probably be a named constant to begin with.

Fair point. This method would increase the temptation to not define a proper constant. Though anyone who wants the “objectively” cleaner code can still define a constant for the bit offset.

Sorry I don't think this is a particularly tough thing to either memorise, reconstruct or look up.

I mostly feel like I simply shouldn't have to do it, irrespective of whether it is difficult or not 🤷.

There is nothing tricky about the implementation, and it should not affect the decision of what to do for out-of-bounds indices whatsoever

Fair.

---

From the presence of methods such as count_ones or trailing_zeros I had instinctively expected something like a high-level method for checking bitflags as well. But I do have to admit that those other methods are less obvious to implement than a bitflag check. So yeah, perhaps this method wouldn't really fit into core.

---

Also, I'm starting to think this proposal is incomplete: if there was what amounts to a getter for individual bits, there should probably also be a setter for individual bits?