jhorstmann commented on a change in pull request #8571: URL: https://github.com/apache/arrow/pull/8571#discussion_r518830136
########## File path: rust/arrow/src/util/bit_chunk_iterator.rs ########## @@ -55,48 +58,52 @@ impl<'a> BitChunks<'a> { pub struct BitChunkIterator<'a> { buffer: &'a Buffer, raw_data: *const u8, - offset: usize, + bit_offset: usize, chunk_len: usize, index: usize, } impl<'a> BitChunks<'a> { + /// Returns the number of remaining bits, guaranteed to be between 0 and 63 (inclusive) #[inline] pub const fn remainder_len(&self) -> usize { self.remainder_len } + /// Returns the bitmask of remaining bits #[inline] pub fn remainder_bits(&self) -> u64 { let bit_len = self.remainder_len; if bit_len == 0 { 0 } else { - let byte_len = ceil(bit_len, 8); Review comment: Exactly. If we have 7 remainder bits for example, if the offset starts at 0 it is enough to read `ceil(7, 8) = 1` byte, but if we start for example at offset 4 we need to combine bits from `ceil(4+7, 8) = 2` bytes. The further refactoring of the loop was then necessary because we may now need to combine bits from 9 bytes, and the previous code (which only shifted after the loop) would have overflowed an `u64`. ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org